Stream: fhirpath
Topic: Handling choice elements
Ewout Kramer (Jul 14 2021 at 09:30):
We recently discovered a bug in our .NET implementation where some parts of our infrastructure accept Condition.onsetPeriod = .... where other parts don't. The spec is very clear about this (https://hl7.org/fhir/fhirpath.html#polymorphism): it should be Condition.onset. While we were planning to fix this, we noticed the Java implementation also accepts Condition.onsetPeriod, so now we are torn: stick to the normative spec (and become incompatible with the Java implementation and thus quite some FP statements in the wild) or submit a tracker item to document this in the FP spec (or FHIR appendix) to allow this as some kind of backwards compat behaviour?
Lloyd McKenzie (Jul 14 2021 at 13:53):
@Grahame Grieve
Brian Postlethwaite (Jul 14 2021 at 20:19):
And would also be interested in the results from the fhirpathjs implementation too
Brian Postlethwaite (Jul 14 2021 at 20:20):
(and Grahame's Delphi version)
Paul Lynch (Jul 27 2021 at 16:01):
Brian Postlethwaite said:
And would also be interested in the results from the fhirpathjs implementation too
fhirpath.js would accept onsetPeriod or just onset. There are some cases when fhirpath.js cannot figure out that the field is a choice type, in which case only the name with the type would work.
I have never seen the advantage of removing the type from the field name (though I concede that is per spec).
Ewout Kramer (Jul 28 2021 at 06:39):
I have never seen the advantage of removing the type from the field name (though I concede that is per spec).
Because the name of the element in the datamodel (and thus for many, the property name of classes in generated code in C#, Java, Delphi...) is different than the specific serialization of such FHIR choice elements in json and xml. If FhirPath was going to be used for FHIR only, we could argue they are practically the same, but FhirPath can be (and is) used for other models too, where there is no such mapping to a serialization. Hence it made more sense to us to keep this "clean" (early version of FhirPath did use the suffixed names, before we had ofType()).
Grahame Grieve (Aug 17 2021 at 04:43):
@Ewout Kramer condition.onsetPeriod should not be supported. That's a bug and surely there's a test case for that?
Ewout Kramer (Aug 18 2021 at 08:27):
You would think so... What we'll do is that we will fix our engine to be completely strict here, and when we are fixing that bug, we'll make sure there is a testcase for it in the suite.
Lee Surprenant (Aug 19 2021 at 18:43):
Late to this party, but I recently noticed that the spec has a few search parameter expression that get this wrong as well.
image.png
Will open a jira ticket to request those to be instantiates as Canonical and instantiates as Uri.
Lee Surprenant (Aug 19 2021 at 18:54):
Grahame Grieve (Aug 19 2021 at 20:11):
those are separate elements in CarePlan
Lee Surprenant (Aug 25 2021 at 02:02):
wow, you are totally right. i'm not sure why I was thinking this was a choice type. i will retract that tracker
Ewout Kramer (Aug 25 2021 at 07:00):
Related, this is particularly nasty: http://hl7.org/fhir/2021May/ingredient.html
There is both concentration[x] and concentrationHighLimit[x] - I wonder how many "naive" implementations for parsers this will break....
Grahame Grieve (Aug 25 2021 at 07:01):
I'm surprised that's OK with the tools. i don't think it's OK and it won't make R5
Lee Surprenant (Aug 25 2021 at 14:51):
just had a quick peak and R4B has concentration[x] and concentrationText... is that one ok?
Ewout Kramer (Aug 25 2021 at 16:18):
In R4B Ingredient, I see "concentration", but not "concentration[x]", so these 3 elements starting with "concentration" should be OK.
Lee Surprenant (Aug 25 2021 at 19:29):
am I in the wrong spot? https://build.fhir.org/branches/R4B/ingredient.html
Chris Moesel (Aug 25 2021 at 20:03):
I suspect that @Ewout Kramer was looking at the official R4B ballot: http://hl7.org/fhir/2021Mar/ingredient.html
Ingredient-FHIR-v4.1.0-20210825160217.png
Chris Moesel (Aug 25 2021 at 20:08):
Looks like there was a ballot comment complaining about the separate low/high properties and the resolution was to add a choice between Range and RatioRange -- resulting in the current build that @Lee Surprenant noticed. (JIRA issue w/ resolution to add the choice: https://jira.hl7.org/browse/FHIR-31821).
Grahame Grieve (Aug 26 2021 at 02:37):
clearly, the name detection clash is failing. @Rik Smithies this is a big deal; the names must be different, and I'll have to figure out why, but please start changing them now.
Grahame Grieve (Aug 26 2021 at 02:38):
you cannot have an element name nnn[x] and then any other element that starts with nnn
Maximilian Reith (Aug 26 2021 at 07:46):
Grahame Grieve said:
Ewout Kramer condition.onsetPeriod should not be supported. That's a bug and surely there's a test case for that?
Ewout Kramer said:
You would think so... What we'll do is that we will fix our engine to be completely strict here, and when we are fixing that bug, we'll make sure there is a testcase for it in the suite.
Which effects could i expect in hl7-Validator and Simplifier-Validator at existing constraints with e.g. condition.onsetPeriod in it? The constraints won't fire or will always fire?
Grahame Grieve (Aug 26 2021 at 07:57):
It depends on how you write them. onsertPeriod will be null, and that will propagate according to the rules here http://hl7.org/fhirpath/N1/#null-and-empty. A constraint that returns null is a constraint that is not known to be true, and so is considered to have failed
Grahame Grieve (Aug 26 2021 at 21:30):
I got around to looking at the tests, and I don't think that this can be true:
While we were planning to fix this, we noticed the Java implementation also accepts Condition.onsetPeriod
Grahame Grieve (Aug 26 2021 at 21:30):
because there is a test case for that
Grahame Grieve (Aug 26 2021 at 21:30):
<group name="polymorphics">
<!--
The way FHIRpath works, it treats polymorphic values e.g. Observation.value[x] as
Observation.value. This catches people out - they often write Observation.valueString.
For this reason, some engines have a non-strict mode where this is allowed, but it's not
technical conformant. While this might change in the future, it's not legal in strict mode.
these tests test this out.
-->
<test name="testPolymorphicsA" inputfile="observation-example.xml"><expression>Observation.value.exists()</expression><output type="boolean">true</output></test>
<test name="testPolymorphicsB" inputfile="observation-example.xml"><expression invalid="semantic">Observation.valueQuantity.exists()</expression><output type="boolean">false</output></test>
<modeTest mode="lenient/polymorphics" name="testPolymorphicsC" inputfile="observation-example.xml"><expression>Observation.valueQuantity.exists()</expression><output type="boolean">true</output></modeTest>
<modeTest mode="lenient/polymorphics" name="testPolymorphicsD" inputfile="observation-example.xml"><expression>Observation.valueString.exists()</expression><output type="boolean">false</output></modeTest>
</group>
Grahame Grieve (Aug 26 2021 at 21:31):
the java FHIRPath implementation acccepts onsetPeriod only if set to lenient mode, and this is only set for the second pair of tests here.
Grahame Grieve (Aug 26 2021 at 21:31):
So I don't think that the java implementation should accept onsetPeriod @Ewout Kramer
Ewout Kramer (Aug 27 2021 at 09:34):
Ah - that's good news. I know we accepted in one of the very first drafts of FhirPath, but that must be years ago.
Maximilian Reith (Aug 31 2021 at 07:09):
Grahame Grieve said:
It depends on how you write them. onsertPeriod will be null, and that will propagate according to the rules here http://hl7.org/fhirpath/N1/#null-and-empty. A constraint that returns null is a constraint that is not known to be true, and so is considered to have failed
Hmm that's bad for IGs which have relied on the output of the validator. There are a lot of profils which can not be changed in a short time and that change immediately will make them faulty . What about a transition phase in which both approaches will work?
Grahame Grieve (Aug 31 2021 at 07:11):
what's an example?
Maximilian Reith (Aug 31 2021 at 07:18):
I will ask them if i could use there example...guess i can reply today
Maximilian Reith (Aug 31 2021 at 12:09):
Grahame Grieve said:
what's an example?
okay we have serveral interfaces in which this problem takes place. All of these interfaces are wide spread and cannot be updated shortly.
e.g. An interface for a certificate of incapacity for work
https://simplifier.net/eau/kbvpreaubundle
<constraint>
<key value="angabeAUseitErstbescheinigung" />
...
<expression value=".........**onsetPeriod**.start.exists() )" />
<source value="https://fhir.kbv.de/StructureDefinition/KBV_PR_EAU_Bundle" />
</constraint>
e.g. An interface for prescriptons
https://simplifier.net/erezept/kbvprerpprescription
<constraint>
<key value="-erp-angabeUnfallbetrieb" />
...
<expression value="extension('unfallkennzeichen').**valueCoding**.code = '2' implies extension('unfallbetrieb').exists()" />
<source value="https://fhir.kbv.de/StructureDefinition/KBV_PR_ERP_Prescription" />
</constraint>
e.g. An interface for vaccinations
https://simplifier.net/im1x0/kbvprmiovaccinationrecordprime
<constraint>
<key value="Occurrence" />
....
<expression value="**occurrenceDateTime**.toString().length()>=10" />
<source value="https://fhir.kbv.de/StructureDefinition/KBV_PR_MIO_Vaccination_Record_Prime" />
</constraint>
e.g. An interface for medical records for children
https://simplifier.net/uh1x0/kbvprmiocmrobservationu1u3datetimeofbirth
<constraint>
<key value="date-1" />
...
<expression value="***valueDateTime***.toString().length()>=16" />
<source value="https://fhir.kbv.de/StructureDefinition/KBV_PR_MIO_CMR_Observation_U1_U3_Date_Time_of_Birth|1.0.1" />
</constraint>
a lot more examples are available....And a lot more interfaces will probably face such problems.
In this case a transition phase up to 01.07.2022 is necessary
Maximilian Reith (Sep 01 2021 at 11:35):
@Grahame Grieve
i added your reference afterwards, so it may not have worked. This is an important topic for us.
Maximilian Reith (Sep 03 2021 at 07:30):
By the way: The first bracket looks a bit weird or not?
is https://www.hl7.org/fhir/fhirpath.html#polymorphism :
(Observation.value as Quantity).unit
should be?
Observation.(value as Quantity).unit
Lee Surprenant (Sep 03 2021 at 11:21):
Observation.(value as Quantity).unit
I think not. as (and in) are both functions and operators in fhirpath. you could do (Observation.value as Quantity).unit or Observation.value.as(Quantity).unit but Observation.(value as Quantity).unit doesn't look right to me
Maximilian Reith (Sep 06 2021 at 06:48):
okay thanks
Maximilian Reith (Sep 07 2021 at 06:28):
the validator might not work properly in context of an extension
We have a chance to update one of our interfaces, so i tried to correct this constraint:
FHIR-Profile_V1.0.2\KBV_PR_ERP_Prescription_ConstraintsNeu.xml (in attached zip)
In line 599 to 612 two variants of this constraint are available:
This part is working properly, but this my be outdated in future:
(extension('https://fhir.kbv.de/StructureDefinition/KBV_EX_ERP_DosageFlag').empty() or extension('https://fhir.kbv.de/StructureDefinition/KBV_EX_ERP_DosageFlag').value.as(Boolean)=false) implies text.empty()
The second is not working:
(extension('https://fhir.kbv.de/StructureDefinition/KBV_EX_ERP_DosageFlag').empty() or (extension('https://fhir.kbv.de/StructureDefinition/KBV_EX_ERP_DosageFlag').value as Boolean)=false) implies text.empty()
Version 5.5.1
java -jar validator_cli.jar -ig FHIR-Profile_V1.0.2 -recurse -version 4.0.1 tset1.xml
FHIR-Profile_V1.0.2.zip
Maximilian Reith (Sep 08 2021 at 16:45):
Could somebody confirm this?
Grahame Grieve (Sep 14 2021 at 10:57):
can you provide a whole test case - an instance and a profile?
Grahame Grieve (Sep 14 2021 at 10:57):
@Maximilian Reith
Maximilian Reith (Sep 15 2021 at 06:11):
@Grahame Grieve
yes of course. it is already in the attached zip (3 posts up). tset1.xml is the instance and the profile is in the zip too in directory FHIR-Profile_V1.0.2\KBV_PR_ERP_Prescription_ConstraintsNeu.
Maximilian Reith (Sep 15 2021 at 06:12):
just use the mentioned command: java -jar validator_cli.jar -ig FHIR-Profile_V1.0.2 -recurse -version 4.0.1 tset1.xml
Maximilian Reith (Sep 16 2021 at 04:54):
i will provide a test case here: https://github.com/FHIR/fhir-test-cases/tree/master/validator
Maximilian Reith (Sep 27 2021 at 14:45):
I talked to @Patrick Werner and @Alexander Zautke . They told me a "lenient"-mode is available in HAPI. That would be a solution for us if it supports all type[x] (e.g. value[x], effected[x]) elements. But the lenient-mode needs to be integrated into the cli-validator, before the "hard" type[x]-check is implemented. Even HL7-Germany is affected by this issue.
Brian Postlethwaite (Mar 08 2022 at 19:37):
(and this one too @Lloyd)
image.png
Last updated: Apr 12 2022 at 19:14 UTC
