FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
816 stars 340 forks source link

Location on SourceNode differs from that on TypedElementToSourceNodeAdapter #2784

Closed cknaap closed 3 weeks ago

cknaap commented 2 months ago

Describe the bug If I construct a SourceNode directly, or from JSON or XML, the location for choice types includes the type name. E.g. Patient.extension[0].valueDateTime[0].

When I roundtrip it through ITypedElement back to ISourceNode using the TypedElementOnSourceNodeAdapter, the location of the same element no longer includes the type name.

In Firely Server we use the ITypedElement to run FHIRPath and find certain elements to patch. But the patching itself happens on ISourceNode. So somehow we need to know the location of an element in the ISourceNode based on information in the ITypedElement.

Up until SDK 5.7.0 this happened to work because TypedElementOnSourceNode retained its location verbatim from the underlying ISourceNode.

To Reproduce Steps to reproduce the behavior: see branch fix/2784-typedelement-sourcenode-location

Expected behavior When I roundtrip an ISourceNode -> ITypedElement -> ISourceNode, I should get the same locations for all elements. Likewise, POCO -> ISourceNode and a SourceNode constructed directly should yield the same locations. Mainly, the type name in choice type elements should be present.

Screenshots n/a

Version used:

Additional context n/a

ewoutkramer commented 2 months ago

Up until SDK 5.7.0 this happened to work because TypedElementOnSourceNode retained its location verbatim from the underlying ISourceNode.

Indeed, "happened" to work ;-) We fixed that bug (#2642) in 5.8.0, since it caused MaskingNode to malfunction. Now it seems this roundtrip feature was dependend on that bug.

When I roundtrip it through ITypedElement back to ISourceNode using the TypedElementOnSourceNodeAdapter

I think you mean "TypedElementToSourceNodeAdapter", since we're talking about getting a SourceNode back from a TypedElement, right? So ISourceNode --TypedElementOnSourceNode--> ITypedElement --TypedElementToSourceNodeAdapter--> ISourceNode ?

Indeed in that last adapter, on line 50 I see:

   public string Location => Current.Location;

This just "happened" to work because the TypedElementOnSourceNode passed it on 1-on-1 until 5.7.0.

ewoutkramer commented 2 months ago

BTW: since in many cases this is a wrapped source node on a wrapped itypedelement that wraps an isourcenode, we can save cycles here by polling the Annotations for ISourceNode - which will resurface the original ISourceNode, and then return those values. If we cannot find it in the annotations (since it got lost or transformed), we have to regenerate the path, since we cannot easily update ITypedElement's location to a SourceNode's locations. Which means tracking the path from parent to child, including array indices.