ahdis / fhir-mapping-tutorial

13 stars 12 forks source link

where condition can not specify source variable #21

Closed oliveregger closed 4 weeks ago

oliveregger commented 4 years ago

source.a2 as a where a2.length()<20 -> target.a2 = a "rule_a20b";

fails, need to change it to one of the following to run the map:

source.a2 as a where length()<20 -> target.a2 = a "rule_a20b"; source.a2 as a where $this.length()<20 -> target.a2 = a "rule_a20b"; source.a2 as a where source.a2.length()<20 -> target.a2 = a "rule_a20b"; source.a2 as a where %source.a2.length()<20 -> target.a2 = a "rule_a20b";

according to the spec a2 should be possible:

The context is an identifier which is either declared as a target for the map, a taret parameter or any named variable within the group (including the variables from the source content) in which this rule is nested.

cnayoung commented 3 years ago

The issue is not related to the definition context in the specification but appears to be a mis-implementation of the FHIRPath standard in HAPI. FHIRPath is a separate specification which is incorporated into the mapping language.

As an example, the rule for step 3b is: src.a2 as a where a2.length()<20 -> tgt.a2 = a "rule_a20b";

Output below:

Loading Load FHIR v4.0 from hl7.fhir.r4.core#4.0.1 - 4575 resources (00:03.0843) Load hl7.terminology#2.0.0 - 3749 resources (00:00.0879) Terminology server http://tx.fhir.org - Version 1.9.367 (00:01.0460) Load ./maptutorial/step3/logical - 2 resources (00:00.0028) Load ./maptutorial/step3/map - 6 resources (00:00.0043) Get set... go (00:00.0058) Start Transform http://hl7.org/fhir/StructureMap/tutorial-step3b Group : tutorial; vars = source variables [src: (TLeft)], target variables [tgt: (TRight)], shared variables [] rule : rule_a20b; vars = source variables [src: (TLeft)], target variables [tgt: (TRight)], shared variables [] condition [a2.length() < 20] for a2=string[012345678901234567890012345678901234567890012345678901234567890] : false ...success Done. Times: Loading: 00:06.0516

There is no error, but the result is an empty TRight element. The a2 element is not copied.

The quote above from the spec refers to the target transform - what comes after the '->'. However, the problem here is to do with the source content - what comes before the '->'. In the source content, it is 'src' (as per the version of the rule I quoted above) that is the context. Similar to context in the target transform, the "context is an identifier which is either declared as a source for the map or as a source parameter or any named variable within the group in which this rule is nested."

The trouble with the example rule for step 3b of the tutorial is that a2 is not qualified by a context when we use it to test the length of the value it contains. The HAPI validator evaluates length() on 'a2' which it appears to treat as undefined. Instead of reporting this as an error or warning the validator simply evaluates the condition as false. a2 is undefined, so it doesn't have a length. There is nothing for the length() function to evaluate so the condition cannot be true. It is therefore reported as false. This is a version of Tony Hoare's classic 'billion dollar mistake'.

The workarounds listed above work by qualifying a2. That even applies indirectly to the first example: source.a2 as a where length()<20 -> target.a2 = a "rule_a20b";

In this case, because we don't evaluate length() explicitly on anything, the validator evaluates the function on the source string which is, of course, obtained using src.a2 as the path.

So, what should happen in the broken version of the rule? The condition is a FHIRPath expression. In FHIRPath, if the expression contains an unqualified identifier, it should be assumed to be a child of the root node. In this case the root node is the context - i.e., src. So yes, the HAPI validator should have interpreted the condition as 'src.a2.length()<20'. There is even an example of this behaviour in the mapping language spec.

src.value : integer 0..* default 10 first as vs0 where value >= 10 check value <= 100 log value

brianpos commented 1 month ago

@oliveregger Following up on this one, it's the order of calling the where/check etc expressions. https://jira.hl7.org/browse/FHIR-46548 And I've done the equivalent change to apply this in the dotnet implementation. https://github.com/brianpos/fhir-net-mappinglanguage/commit/8bfd3e040f35f3d1d922d41d7e824911c120ba8e

oliveregger commented 1 month ago

the next java core library update should have it, in matchbox v3.9.3 it works now:

src.a2 as a where (a.length()<20) -> tgt.a2 = a "rule_a20b";

oliveregger commented 4 weeks ago

fixed in maps and from org.hl7.fhir.core 6.3.30 onwards it will work https://github.com/hapifhir/org.hl7.fhir.core/issues/1748