LinuxForHealth / FHIR

The LinuxForHealth FHIR® Server and related projects
https://linuxforhealth.github.io/FHIR
Apache License 2.0
331 stars 157 forks source link

Bundle transaction processing should replace all local references, not just ones that start with "urn:" #2512

Closed lmsurpre closed 2 years ago

lmsurpre commented 3 years ago

Is your feature request related to a problem? Please describe. From https://www.hl7.org/fhir/http.html#trules

A transaction may include references from one resource to another in the bundle, including circular references where resources refer to each other. When the server assigns a new id to any resource in the bundle which has a POST method as part of the processing rules above, it SHALL also update any references to that resource in the same bundle as they are processed (see about Ids in a bundle). References to resources that are not part of the bundle are left untouched. Version-specific references should remain as version-specific references after the references have been updated. Note that when building a transaction, a client can use arbitrarily chosen version references since they will all be re-assigned anyway. Servers SHALL replace all matching links in the bundle, whether they are found in the resource ids, resource references, elements of type uri, url, oid, uuid, and <a href=“” & <img src=“” in the narrative. Elements of type canonical are not replaced.

Today, the IBM FHIR Server only updates literal references when they start with the "urn:" prefix.

Describe the solution you'd like Update all literal references in the bundle when they match a fullUrl in the bundle that is associated with a resource being POSTed.

To determine whether a literal reference needs updating or not, we should follow the guidance at https://www.hl7.org/fhir/bundle.html#references and I believe that we already have code for this.

Describe alternatives you've considered

Acceptance Criteria

  1. GIVEN a transaction bundle with resources A and B AND resource A is a POST AND resource B is a POST or PUT with a literal relative reference to the fullUrl for resource A WHEN the bundle is posted to the server THEN the literal reference in resource B is updated to the assigned resource id of resource A

  2. GIVEN a transaction bundle with resources A and B AND resource A is a POST AND resource B is a POST or PUT with a literal absolute reference to the fullUrl for resource A WHEN the bundle is posted to the server THEN the literal reference in resource B is updated to the assigned resource id of resource A

Additional context

punktilious commented 3 years ago

This should be taken care of in #1869. Need to double-check the implementation.

lmsurpre commented 3 years ago

1869 was closed, but I don't think we got this one in there. Sound right, @punktilious ?

michaelwschroeder commented 3 years ago

The scope of this issue is to replace all local references (not just those starting with the urn: prefix) in elements of type Reference only. Issue #2993 has been opened to address replacing local references in the following element types, per the FHIR spec:

lmsurpre commented 2 years ago

I posted the following transaction bundle (adapted from the a smart health card example):

{
  "resourceType": "Bundle",
  "type": "transaction",
  "entry": [
    {
      "fullUrl": "resource:0",
            "request": {
        "method": "POST",
        "url": "Patient"
      },
      "resource": {
        "resourceType": "Patient",
        "name": [
          {
            "family": "Anyperson",
            "given": [
              "John",
              "B."
            ]
          }
        ],
        "birthDate": "1951-01-20"
      }
    },
    {
      "fullUrl": "resource:1",
            "request": {
                "method": "POST",
                "url": "Immunization"
            },
      "resource": {
        "resourceType": "Immunization",
        "status": "completed",
        "vaccineCode": {
          "coding": [
            {
              "system": "http://hl7.org/fhir/sid/cvx",
              "code": "207"
            }
          ]
        },
        "patient": {
          "reference": "resource:0"
        },
        "occurrenceDateTime": "2021-01-01",
        "performer": [
          {
            "actor": {
              "display": "ABC General Hospital"
            }
          }
        ],
        "lotNumber": "0000001"
      }
    },
    {
      "fullUrl": "resource:2",
            "request": {
                "method": "POST",
                "url": "Immunication"
            },
      "resource": {
        "resourceType": "Immunization",
        "status": "completed",
        "vaccineCode": {
          "coding": [
            {
              "system": "http://hl7.org/fhir/sid/cvx",
              "code": "207"
            }
          ]
        },
        "patient": {
          "reference": "resource:0"
        },
        "occurrenceDateTime": "2021-01-29",
        "performer": [
          {
            "actor": {
              "display": "ABC General Hospital"
            }
          }
        ],
        "lotNumber": "0000007"
      }
    }
  ]
}

I expected the transaction to be process and for the patient references in resource:1 and resource:2 to be replaced with the server-assigned id for resource:0.

Instead, I got a 400 Bad Request:

{
    "resourceType": "OperationOutcome",
    "issue": [
        {
            "severity": "fatal",
            "code": "invalid",
            "details": {
                "text": "FHIRProvider: Invalid reference value or resource type not found in reference value: &#39;resource:0&#39; for element: &#39;patient&#39; [Bundle.entry[1]]"
            },
            "expression": [
                "Bundle.entry[1]"
            ]
        }
    ]
}

I think what is happening is we're doing our reference type validation before rewriting the references. I think that reference type checking is actually performed during our parse validation, which makes this a lot trickier. We can verify this by configuring the server to skip reference type checking, but I don't think suggesting users to turn that off is a suitable solution. We might need to make a change on this one.

lmsurpre commented 2 years ago

If we agree that we should support transaction bundles like this one, here's the simplest solution I could think of:

  1. do the bundle parse without parse validation
  2. do the reference rewriting and ensure parse validation is ON when we reconstruct the objects with the new reference values
lmsurpre commented 2 years ago

Robin brought up a different option which would be to make our parse validation smarter about what is a valid bundle...probably by making it conditional on the type of bundle (e.g. special reference type checking for bundles of type batch or transaction)

lmsurpre commented 2 years ago

neither of those options are real simple/clean.

Bundle parse validation actually happens in the FHIRProvider.readFrom (a JAX-RS provider for the application/fhir media types). To skip parse validation there would require some special-case logic where we introspect the uriInfo and take special action on a post the base url. Bleh.

Making our parse validation smarter isn't a real simple win either because the ValidationSupport.checkReferenceType calls are in the individual resource types and backbone elements that have Reference data types and those methods are clueless about whether they happen to be getting parsed/validated as part of a larger batch/transaction bundle or not.

To be honest, the option I'm leaning most towards now is moving reference type checking from parse validaton to secondary validation (FHIRValidator)...then it should be easier to control the behavior using either of the options presented above. @JohnTimm feel free to weigh in on this one if you're interested :-)

lmsurpre commented 2 years ago

Before going much further with this, I decided to re-read the latest proposed wording on bundle reference resolution: https://jira.hl7.org/browse/FHIR-29271?focusedCommentId=183020&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-183020

To me, this wording makes it more clear that if you have a fullUrl that doesn't begin with http:, https:, or urn:, then there should be no expectation of reference replacement.

I decided to ask for further clarification at https://chat.fhir.org/#narrow/stream/179166-implementers/topic/Allowed.20uri.20schemes.20for.20Bundle.2Eentry.2EfullUrl/near/271621146

tbieste commented 2 years ago

Closing after successful QA testing with the following transaction bundle:

{ "resourceType": "Bundle", "type": "transaction", "entry": [ { "fullUrl": "resource:0", "request": { "method": "POST", "url": "Patient" }, "resource": { "resourceType": "Patient", "name": [ { "family": "Anyperson", "given": [ "John", "B." ] } ], "birthDate": "1951-01-20" } }, { "fullUrl": "resource:1", "request": { "method": "POST", "url": "Immunization" }, "resource": { "resourceType": "Immunization", "status": "completed", "vaccineCode": { "coding": [ { "system": "http://hl7.org/fhir/sid/cvx", "code": "207" } ] }, "patient": { "reference": "resource:0" }, "occurrenceDateTime": "2021-01-01", "performer": [ { "actor": { "display": "ABC General Hospital" } } ], "lotNumber": "0000001" } }, { "fullUrl": "resource:2", "request": { "method": "POST", "url": "Immunization" }, "resource": { "resourceType": "Immunization", "status": "completed", "vaccineCode": { "coding": [ { "system": "http://hl7.org/fhir/sid/cvx", "code": "207" } ] }, "patient": { "reference": "resource:0" }, "occurrenceDateTime": "2021-01-29", "performer": [ { "actor": { "display": "ABC General Hospital" } } ], "lotNumber": "0000007" } } ] }