FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
829 stars 345 forks source link

FhirPath: System.ArgumentException: 'Unknown symbol 'resolve'' #1968

Closed RodrigoTHD closed 1 year ago

RodrigoTHD commented 2 years ago

Hi there!

I'm receiving the error System.ArgumentException: 'Unknown symbol 'resolve'' when I'm trying to execute the FhirPath expression with resolve below:

var ctx = new FhirEvaluationContext(bundle.ToTypedElement());
var procedure = bundle.Entry.ByResourceType<Procedure>().FirstOrDefault();
var encounterReference = procedure.ToTypedElement().Scalar("$this.encounter.resolve()", ctx);

To Reproduce

  1. I'm using HL7.Fhir.R4 (3.8.0)
  2. The bundle has 80 resources: Encounter, Patient, Procedures, Observations etc.
  3. I'm using a FhirEvaluationContext with the bundle.
  4. The expression is being executed for one resource: procedure
  5. The resolution is local, must use the data provided in the bundle.

Expected behavior The resolve SHALL return the reference.

Screenshots image

Version used:

Additional context I didn't find details on how to use FhirPath with resolve in the documentation.

ewoutkramer commented 2 years ago

The issue is that resolve() is not a standard FhirPath function, but is specific to FHIR. I know that sounds silly, but you will not find this function in the FhirPath normative spec (https://hl7.org/fhirpath/). Fortunately, it is easy to add these functions. Try adding

   FhirPathCompiler.DefaultSymbolTable.AddFhirExtensions();

somewhere in your startup code. It should work.

@marcovisserFurore - Is this something we can help out with? Should we install the Fhir appendix dialect as the default?

marcovisserFurore commented 2 years ago

I think we can add the Fhir appendix. This is not the only question we get about the FhirPath function resolve(). I will add this issue on the backlog

RodrigoTHD commented 2 years ago

Hi, Thank you for the help!

It's working after calling the extension:

// Add Fhir extension
FhirPathCompiler.DefaultSymbolTable.AddFhirExtensions();

But, it was necessary to update the context by setting an ElementResolver to process the FhirPath to get the reference from the bundle, because I want to process it locally.

// Configuring elementResolver to process the reference from the bundle.
var ctx = new FhirEvaluationContext();
ctx.ElementResolver += (string fhirPath) =>
{
  return bundle.FindEntry($"{FhirClientUtils.FhirUrl}/{fhirPath}").FirstOrDefault().ToTypedElement();
};

So I have another question:

1) Example with rootElement

// Create context with rootElement
var ctx = new FhirEvaluationContext(bundle.ToTypedElement());
// Select single resource to execute FhirPath
var procedure = bundle.Entry.ByResourceType<Procedure>().FirstOrDefault();
// Execute FhirPath using the context
var encounterReference = procedure.Select("encounter.resolve()", ctx);

Result

image

2) Example with custom ElementResolver

// Create context
var ctx = new FhirEvaluationContext();
// Configure Element resolver function
ctx.ElementResolver += (string fhirPath) =>
{
  return bundle.FindEntry($"{FhirClientUtils.FhirUrl}/{fhirPath}").FirstOrDefault().ToTypedElement();
};
// Select single resource to execute FhirPath
var procedure = bundle.Entry.ByResourceType<Procedure>().FirstOrDefault();
// Execute FhirPath using the context
var encounterReference = procedure.Select("encounter.resolve()", ctx);

Result

image

ewoutkramer commented 2 years ago

But, it was necessary to update the context by setting an ElementResolver to process the FhirPath to get the reference from the bundle, because I want to process it locally.

Ah, yes, this is not enough - as you found out ;-) You can do it all by yourself (as you did), but you can also wrap your instance with a ScopedNode, this is an implementation of ITypedElement that keeps track of nested resources, and tracks back from child nodes to the root node, so it can actually resolve the resources. If you use this class, you should be fine without your additional code!

marcovisserFurore commented 2 years ago

Suggestion: use the ModuleInitializerAttribute. That can fire up a method and initialize for example the extra Fhir appendix dialect.

mmsmits commented 2 years ago

Can't use the ModelInitializer https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2255

brianpos commented 2 years ago

There are many cases where the thing being resolved is not within the scope too, and is expecting to reach outside the execution, i.e. resolving the subject reference to load in the patient that is connected (to perform some form of check on that) and depending on the context, that might be fine, or it might not be. (I'm not a fan of that type of usage) The place I've seen this used most of all is to check the type of a resource that is resolved (i.e. subject.resolve() is Patient for a patient search parameter on something) The server I formerly worked on, and also the Microsoft server, both do a fake resolving to a dummy resource of the type with the ID in it, which is enough for this to work for searching, but useless otherwise.

brianpos commented 2 years ago

Maybe a suggestion here is that we could put some hints in the exception message (or console/trace.log) to indicate that this work needs to be done.

marcovisserFurore commented 2 years ago

In the standup of 23rd of July we decided to add the FhirPath FHIR functions automatically only when the user is dealing with FHIR resources. So when the user uses the functions Base.Select, Base.Scalar, Base.Predicate, Base.IsBoolean and Base.IsTrue (in FhirPathExtensions.cs), we make sure those extra FhirPath functions (like resolve() are loaded.

Furthermore we add some more documentation about this.

ewoutkramer commented 2 years ago

I was wrong in my first reaction that you needed a ScopedNode to make this work - it's true, but our Select() already wraps the node with a ScopedNode, so that's not influencing what is going on here.

The real question (and sorry for not reading this not well enough initially) is: why did @RodrigoTHD need to write his own resolver. Why did we not find the encounter out-of-the-box?

The reason is obvious once you know how ScopedNode works in concert with resolve(): ScopedNode inspects all nodes it passes through while evaluating the FHIR path statement, so when it hits resolve(), it can find bundled and contained entries.

However, what @RodrigoTHD is doing here is jump right across the Bundle to te entries using C#:

   // Create context with rootElement
   var ctx = new FhirEvaluationContext(bundle.ToTypedElement());
   // Select single resource to execute FhirPath
   var procedure = bundle.Entry.ByResourceType<Procedure>().FirstOrDefault();
   // Execute FhirPath using the context
   var encounterReference = procedure.Select("encounter.resolve()", ctx);

In this last line, the Select will be the start of evaluation of the FhirPath, but we are already past the Bundle and its entries (and our POCO model does not have links back to the parent Bundle from contained/bundled resources). So there is no way for the evaluator to reach back. So,only by writing your own resolver, you can bring this knowledge back in.

What would have worked is this:

   // Create context with rootElement
   var ctx = new FhirEvaluationContext(bundle.ToTypedElement());
   // Select single resource to execute FhirPath
   var procedure = bundle.Select("entry.resource.ofType(Procedure).first()");
   // Execute FhirPath using the context
   var encounterReference = procedure.Select("encounter.resolve()", ctx);

Instead of using LINQ to navigate down the POCO, we use the FhirPath evaluator, which will stick all necessary context information in a ScopedNode that is returned from the first Select() and stored in the variable procedure. When you then continue to work your way from there (second FP statement), all works fine.

I can see how Rodrigo expects this to work if he passes in the root of the Bundle to the FhirEvaluationContext - you are kind of supplying the original root (= the Bundle) of the POCO you are resolving from, so in theory, resolve() could now work.

The question is: do we want to implement that? It's not trivial: even though you have the root Bundle context, you need to resolve from the reference itself "up" to its parents - ScopedNode won't help us here.

marcovisserFurore commented 1 year ago

Solved by PR #2168.