Stream: implementers
Topic: Crucible fails to validate a response
Florian Apolloner (Mar 14 2019 at 15:22):
Hi, I am trying to understand why crucible thinks that the test https://projectcrucible.org/servers/5c89107d04ebd024be000000#5c8a6caf04ebd0675a000008/history001/HI08 did not return a bundle. If I look at the response: https://projectcrucible.org/servers/5c89107d04ebd024be000000#0-response I see the xml bundle just fine. Any hints?
Lloyd McKenzie (Mar 14 2019 at 15:58):
@Jason Walonoski ?
Jason Walonoski (Mar 14 2019 at 20:19):
@Robert Scanlon Similar issue to this? https://chat.fhir.org/#narrow/stream/179204-crucible/topic/Can't.20figure.20out.20why.20test.20fails/near/157278207
Robert Scanlon (Mar 14 2019 at 20:36):
Hmmm, looking into it now.
Robert Scanlon (Mar 14 2019 at 21:31):
This appears to be Crucible's fault. I put in a candidate fix but will be a little while before it gets deployed. @Jason Walonoski : https://github.com/fhir-crucible/fhir_client/pull/110
Florian Apolloner (Mar 15 2019 at 08:49):
Ah perfect. I have no issue with waiting, I am glad it is not an issue in my code :) Thank you!
Robert Scanlon (Mar 15 2019 at 13:04):
No problem, thanks for the report! One thing I am noticing is that the order of the entries in the bundle don't always appear to be in the right order, but that issue is being masked by this problem.
Robert Scanlon (Mar 15 2019 at 13:06):
PASS hi01_history_for_specific_resource_test:
PASS hi01.1_history_request_entries_test:
PASS hi02_full_history_of_a_resource_by_id_with_since_test:
PASS hi03_individual_history_versions_test:
PASS hi04_history_for_missing_resource_test:
**FAIL hi06_all_history_for_resource_with_since_test: Result contains entries in the wrong order.**
PASS hi08_all_history_whole_system_with_since_test:
PASS hi09_resource_history_page_forward_test:
PASS hi10_resource_history_page_backwards_test:
PASS hi11_first_page_full_history_test:
Execute HistoryTest completed in 19.866114000004018 seconds.
Robert Scanlon (Mar 15 2019 at 13:09):
Sometimes it works, sometimes it doesn't, which makes me think that your DB tends to return the data in the right order based on how the data happens to be accessed, but there isn't an explicit sort in there
Robert Scanlon (Mar 15 2019 at 13:10):
(that is total speculation though)
Florian Apolloner (Mar 15 2019 at 13:11):
@Robert Scanlon Yes I also saw that one, but couldn't figure out what the expected order is. I am currently sorting strictly by "creation_timestamp DESC" which should mean that newer entries should show up first (every operation creates a new entry and as such also a new version). Also to get the crucible tests passing I have to truncate my database before the testrun because it otherwise fetches old entries marked as deleted (not yet sure if I implement something against the spec here or of the tests in crucible are having issues here)
Robert Scanlon (Mar 18 2019 at 19:34):
@Florian Apolloner -- We deployed a fix to the issue where the bundle wasn't recognized, so that should be all set.
Robert Scanlon (Mar 18 2019 at 19:38):
I'm curious about the other issues though. I assume what is being returned now is that issue you need to truncate your database to pass?
Florian Apolloner (Mar 18 2019 at 22:29):
@Robert Scanlon Yeah, I made a mistake there when removing the 'resource' objects for deleted entries. Fixed that and now I get invalid order again: https://projectcrucible.org/servers/5c89107d04ebd024be000000#5c901b6804ebd01d60000002/history001/HI06 -- having a hard time figuring out why though :/
Florian Apolloner (Mar 18 2019 at 22:31):
Ah, I think I failed in incrementing my version ID when recreating a deleted entry.
EDIT:// Nope, something else seems to be going on there. I noticed that crucible does a PUT with a versionId in the test-setup for that test. Is that allowed? I've left the data as it is currently in the broken state, maybe you have an idea.
Do I really need to order by version_id instead of just a time based history?
Florian Apolloner (Mar 18 2019 at 22:42):
ie https://bap-fhir.herokuapp.com/fhir/STU3/Patient/_history?_since=2019-03-18T22%3A26%3A53Z currently returns vids 4,3,2,1,4,3,2,1 (since it returns 2 resources), should that return 4,4,3,3,2,2,1,1 instead?
Robert Scanlon (Mar 19 2019 at 14:21):
Doing a create and an update with a meta.versionId is valid, but the server should ignore them and put in their own values: "If the request body includes a meta, the server SHALL ignore the existing versionId and lastUpdated values. The server SHALL populate the id, meta.versionId and meta.lastUpdated with the new correct values." http://hl7.org/fhir/stu3/http.html#create
Robert Scanlon (Mar 19 2019 at 14:23):
Similar language is in there for updates http://hl7.org/fhir/stu3/http.html#update
Robert Scanlon (Mar 19 2019 at 14:24):
Now, it appears that Crucible is wrong in that it assumes there is a global incrementing versionId: https://github.com/fhir-crucible/plan_executor/blob/master/lib/tests/suites/history_test.rb#L307
Robert Scanlon (Mar 19 2019 at 14:27):
"The version can be globally unique, or scoped by the Logical Id of the resource. Version identifiers are generally either a serially incrementing id scoped by the logical id, or a uuid, though neither of these approaches is required. There is no fixed order for version ids - clients cannot assume that a versionId that comes after another one either numerically or alphabetically represents a later version. The same versionId can never be used for more than one version of the same resource." http://hl7.org/fhir/stu3/resource.html#metadata
Robert Scanlon (Mar 19 2019 at 14:29):
So i think we need to remove that line, and perhaps add some logic to make sure that versionIds are not reused within a single resource.
Robert Scanlon (Mar 19 2019 at 14:33):
FYI @Jason Walonoski, let me know if there is something I'm not considering here. I think this part of the test might have been from an early version of FHIR that did not have concrete guidance on version ids.
Jason Walonoski (Mar 19 2019 at 15:41):
Yes, it looks like incrementing version ids are not required... which is sad, because it means they are totally opaque and meaningless. We need to change the logic that you linked to.
Florian Apolloner (Mar 19 2019 at 16:21):
@Robert Scanlon Ah yes, I have version numbers per resource_id, not something globally unique, even though that would be easily doable. Thank you for your help!
Robert Scanlon (Mar 19 2019 at 17:01):
@Florian Apolloner -- To be clear, version ids don't have to be globally unique, just unique on a per resource basis, according to the spec. But you can make them globally unique if you'd like (uuids would do that). We'll update our tests to remove the logic that assumes they are incrementing integers and will update here when that is deployed. Thanks again!
Robert Scanlon (Mar 22 2019 at 04:58):
@Florian Apolloner -- fixed the issue on our side with requiring incrementing global versioning and now it looks good. https://projectcrucible.org/servers/5c89107d04ebd024be000000#5c946aeb04ebd0266c000002/history001/HI01
Florian Apolloner (Mar 22 2019 at 08:38):
Ah lovely. Many thanks!
Last updated: Apr 12 2022 at 19:14 UTC
