FirelyTeam / firely-validator-api

Firely's official FHIR validator API for validating HL7 FHIR resources against profiles.
Other
8 stars 2 forks source link

Invariant rule ref-1 fails when resource that would succeed normally is inside a Parameters resource #327

Closed almostchristian closed 1 month ago

almostchristian commented 3 months ago

The following input fails for both the legacy validator and the new validator. We do profile validation on Parameters resources as inputs to custom operations using the profile set in OperationDefinition.inputProfile.

{
  "resourceType": "Parameters",
  "parameter": [
    {
      "name": "patient",
      "resource": {
        "resourceType": "Patient",
        "contained": [
          {
            "resourceType": "Organization",
            "id": "acme",
            "name": "Acme Trading corp"
          }
        ],
        "identifier": [
          {
            "use": "usual",
            "value": "12345",
            "assigner": {
              "reference": "#acme",
              "display": "Acme"
            }
          }
        ],
        "active": true
      }
    }
  ]
}

Validation response:

{
  "resourceType": "OperationOutcome",
  "issue": [
    {
      "severity": "error",
      "code": "invariant",
      "details": {
        "coding": [
          {
            "system": "http://hl7.org/fhir/dotnet-api-operation-outcome",
            "code": "1012"
          },
          {
            "system": "http://fire.ly/dotnet-sdk-operation-outcome-structdef-reference",
            "code": "Parameters.parameter.resource->Patient.identifier->Identifier.assigner->Reference"
          }
        ],
        "text": "Instance failed constraint ref-1 \"SHALL have a contained resource if a local reference is provided\""
      },
      "location": [
        "Parameters.parameter[0].resource[0].identifier[0].assigner[0]"
      ],
      "expression": [
        "Parameters.parameter[0].resource[0].identifier[0].assigner[0]"
      ]
    }
  ]
}

The FhirPath expression is reference.exists() implies (reference.startsWith('#').not() or (reference.substring(1).trace('url') in %rootResource.contained.id.trace('ids')) or (reference='#' and %rootResource!=%resource)) which means the %rootResource in this context is the Parameters resource which would be impossible since Parameters does not have contained resources. The resource validates successfully in https://validator.fhir.org/, which I think means it doesn't use the Parameters as the rootResource.

For now, we just disable the ref-1 rule.

ewoutkramer commented 3 months ago

I decided to make the Parameters resource the %rootResource here because of this:

https://www.hl7.org/fhir/fhirpath.html#variables

FHIR defines two specific variables that are always in scope when FHIRPath is used in any of the contexts above:

%resource // the resource that contains the original node that is in %context %rootResource // the container resource for the resource identified by %resource

This does not tell us much, but the text under it says:

When a DomainResource contains another resource, and that contained resource is the focus (%resource) then %rootResource refers to the container resource. Note that in most cases, the resource is not contained by another resource, and then %rootResource is the same as %resource.

Clearly, the contained resource (Patient) is the focus here, as we are validating the reference in Patient.identifier. The text says "then %rootResource refers to the container resource", which clearly is the Parameters resource here.

Now, the only interesting bit is that the spec says: "When a DomainResource contains another resource" - and Parameters is not a DomainResource. Will that be the difference? Maybe the %rootResource is to be read in the context of containment, and non-domain resources do not have a contained. This might be because the definition of %rootResource says "the container resource for the resource(...)". And non-domain resources cannot be container resources. Well, actually, I have interpreted they can be, since Bundles and Parameters are containers of other resources, just not through contained....

almostchristian commented 3 months ago

Found this interesting discussion https://chat.fhir.org/#narrow/stream/179166-implementers/topic/Contained.20resources/near/398280761

DomainResource.contained isn't a boundary that resets rootResource, but resources in Bundle and Parameters do reset rootResource

mmsmits commented 2 months ago

Also solve #FirelyTeam/firely-net-sdk#2177

almostchristian commented 2 weeks ago

The latest version of the the validator (2.5.0) fixes ref-1 for R5 but seems to fail for R4/R4B. I think it's because of the difference of the expression. I got it to work in R4B by modifying the Expression to be the same as the one in R5. It's a bit of a hack for now.

[System.Runtime.CompilerServices.UnsafeAccessor(System.Runtime.CompilerServices.UnsafeAccessorKind.Field, Name = "<Expression>k__BackingField")]
private static extern ref string GetExpression(FhirPathValidator counter);

// ...

validationSettings.ExcludeFilters.Add(x =>
{
    if (x is FhirPathValidator fpv && fpv.Key == "ref-1")
    {
        GetExpression(fpv) = "reference.exists() implies (reference.startsWith('#').not() or (reference.substring(1).trace('url') in %rootResource.contained.id.trace('ids')) or (reference='#' and %rootResource!=%resource))";
    }

    return false;
});