REST APIs documentation issues / questions
Description
PR is addressed in
Activity
Sebastian Iancu September 1, 2019 at 9:23 PM
1), 3) and 5) done
4) I don’t know…, this sounds more something for the server to do internally, rather than to report to the requested
6) and 8.) not sure for now if we need it
9) I think naming is ok, the idea was that we take a version from a versioned object
10) suggestion is reasonable, but this is a potential breaking-change - need to be discussed if is worth doing it
Sebastian Iancu September 1, 2019 at 9:02 PM
for 2.) the status code “409 Conflict” indicates that the request cannot be performed as it will end up in a (version or identity) conflict. These request does not have preconditions (i.e. If-Match headers), but contains either an id or the content is identified by id already existing on the server.
In order to prevent version lost upon updates or delete request we use IF-Match header for precondition, and for such request we return status code “412 Precondition failed”.
I know it might sound a bit confusing having two different status code for something apparently identical, but we did it like this in order be compliant with HTTP/REST style.
Details
Details
Reporter
Components
Affects versions
Priority
Minor
Various items in the documentation.
1. 'timestamp' is documented as the standard parameter name for times in the API. In the openEHR RM specs, we use 'time' or 'xxx_time' to mean this, and I don't think the terminology 'timestamp' is ever used.
2. Error code 409 is defined as
Conflict Indicates that the request could not be processed because it might generate a duplicate or a conflict
I think it would be clearer to have an error code that corresponds to creation of duplicates. What other kinds of conflicts are there? Note that creating a duplicate is also a pre-condition fail error condition.
3. In the documentation about date/time format, it probably should be made clear that TZ is only supplied when needed and that when not supplied, the local TZ is assumed. A secondary concern is that TZ probably should be added to all date/times committed to openEHR systems, to enable time information in EHRs shared across timezones to be computed properly.
4. In the abstract spec, I have documented pre-condition and post-conditions. Is there any way to show post-conditions in the REST API spec? Could it just point to the abstract spec, in order to show what should have occurred on the server? See e.g. 'Create EHR' => create_ehr() (https://www.openehr.org/releases/SM/latest/docs/openehr_platform/openehr_platform.html#_i_ehr_service_interface)
5. Typo in text for 409 error on 'Create EHR with id' call - 'already already'.
6. I created abstract definitions for create_ehr_for_subject() and create_ehr_for subject_with_id(); (see https://www.openehr.org/releases/SM/latest/docs/openehr_platform/openehr_platform.html#_i_ehr_service_interface) - do we not want these in the REST API - for many current systems, these will be the default form of the call I think. This seems specifically odd given that there are REST calls like 'Get EHR Summary by subject id'.
7. The call 'Get EHR summary by subject id' can result in >1 EHR Summary; is this handled properly?
8. In the abstract spec, I proposed calls for setting and unsetting is_queryable and is_modifiable - would these be useful in the REST API? (See https://www.openehr.org/releases/SM/latest/docs/openehr_platform/openehr_platform.html#_i_ehr_status_interface).
9. There is a call 'Get versioned EHR_STATUS version by time' which I think should be called 'Get EHR_STATUS version by time' - the result is a VERSION, not a VERSIONED_OBJECT. Any other similar calls should be checked as well.
10. After reading through various parts of the main definition, I think the parameter 'version_at_time' is confusingly named - it would be better to just name it 'time' or 'version_time'.