-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spec-compliant whole-system history #2026
Comments
Signed-off-by: Robin Arnold <[email protected]>
Signed-off-by: Robin Arnold <[email protected]>
Signed-off-by: Robin Arnold <[email protected]>
Reopening this issue, with verification of the header set Prefer: return=representation. on a delete the full resource is included, but there is no way to read that versioned resource. or otherwise see what it used to be. so it should not be included if it is a Delete method. |
this is definitely the bigger issue of the two things mentioned. the spec is fairly clear that the resource content should be omitted for the history of a delete.
I think its fine to continue including the Entry.response.location for the deleted version though (my 2 cents) |
Resource will not be added to the bundle entry when the change type == DELETED, and we'll also fetch the resource values now when the return preference is OPERATION_OUTCOME, not just REPRESENTATION. |
Delete entry now looks like this (test data):
|
Signed-off-by: Robin Arnold <[email protected]>
The two noted reasons to reopen this issue are now fixed. Closing the issue. |
Is your feature request related to a problem? Please describe.
For #1955 we added support for a high-performance
changes
feed. Originally, Robin was going to implement that feature as a custom operation, but I convinced him that it was so close to the whole-system history interaction that we should just serve it from that endpoint.From my analysis, the implementation is super close to a compliant whole-system history interaction. However, there are just a couple aspects where we're out of compliance:
bdl-8 | Rule | Bundle.entry | fullUrl cannot be a version specific reference | fullUrl.contains('/_history/').not()
http://www.hl7.org/fhir/bundle.html#history says:
I think that using _history in the fullUrl was a conscious decision to avoid duplicating this long string in the response.header, but I think we should revisit that decision (and we need to if we want to be spec-compliant).
From https://www.hl7.org/fhir/R4/http.html#history
GET [base]/_history
might be expected to return the latest set of changes; ours returns the oldest set of changesnext
link should point to the next oldest set of changes (going backwards in time), but ournext
link actually goes to the next newest size of changesDescribe the solution you'd like
To bring our implementation into compliance, while still supporting this high-performance variant for consumers that need it, I propose that we allow clients to opt-in to the behavior that they want.
For number 1 (bdl-8), start including
Bundle.entry.response.location
with the value that we currently put in fullUrl. In the fullUrl, drop the/_history/:vid
suffix.For number 2 (inclusion of the resource bodies), they can opt-in by passing us a return preference via the HTTP Prefer header.
This builds on the use of "Prefer: return" as documented for create/update/patch/transaction, which lists the following allowed values:
In the case of History, I don't think the OperationOutcome return preference makes much sense, but just allowing minimal vs representation seems like a way to let the client to decide whether they are interested in a summary of the changes or the specific contents of the resources that changed.
My proposal is to make
return=representation
the default for all history interactions (for both whole-system or resource-instance level), but to use the high-performance implementation (with no return resource) in the case they passPrefer: return=minimal
.For number 3 (ordering of the results), I'd like to introduce a new
_sort
parameter._sort=forward
.(which is also the default)_sort=backward
(which allows the server to sort the resources in whatever order it wants)_sort=system
UPDATE: FHIR has agreed to adopt the following variants, so we should use these instead:
Describe alternatives you've considered
Move our "changes" feed to a custom operation...like Robin had originally planned to do.
Acceptance Criteria
At least one acceptance criteria is included.
GIVEN a
_history
requestWHEN no Prefer header is passed
THEN the response returns the resource contents in the response entries (spec-compliant)
GIVEN a
_history
requestWHEN
Prefer: return=minimal
is passedTHEN the response returns no resource bodies in the response entries
GIVEN a
_history
requestWHEN
Prefer: return=representation
is passedTHEN the response returns the resource contents in the response entries
Additional context
We raised the sort ordering stuff at https://chat.fhir.org/#narrow/stream/179166-implementers/topic/Ordering.20for.20the.20whole-system.20history.20interaction
We may want to open a corresponding issue.
Similarly, we may want to share our idea for excluding the resource types with the rest of the FHIR community.
The text was updated successfully, but these errors were encountered: