hapifhir / hapi-fhir

🔥 HAPI FHIR - Java API for HL7 FHIR Clients and Servers
http://hapifhir.io
Apache License 2.0
2.05k stars 1.33k forks source link

HAPI JSON parser is adding `#` to contained resource IDs. #6514

Open dotasek opened 13 hours ago

dotasek commented 13 hours ago

Describe the bug HAPI's JSON parser is adding # characters to contained resource IDs.

To Reproduce

I created a breaking test in a branch: https://github.com/hapifhir/hapi-fhir/blob/9e9f1e65e2534a7ece1574fa0529d813fbc6392b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java#L1383-L1389

This parses the following JSON:

{
    "resourceType": "Observation",
    "id": "O1",
    "contained": [
        {
            "resourceType": "Specimen",
            "id": "contained-id"
        }
    ],
    "status": "final",
    "specimen": {
        "reference": "#contained-id"
    }
}

Expected behavior The JSON parser should parse the contained specimen ID as is, without adding an additional #.

Otherwise, the parsed resource is not consistent with the FHIR specification for contained resources: https://hl7.org/fhir/r4/references.html#contained

Environment (please complete the following information):

dotasek commented 13 hours ago

The code that adds the # appears to be here: https://github.com/hapifhir/hapi-fhir/blob/77fa7f78191aa83b48a12bd29a7ff26fc574bb84/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java#L442

jamesagnew commented 13 hours ago

Is this causing an issue somewhere else?

This is pretty old behaviour, changing it could be a breaking change for people depending on it

dotasek commented 12 hours ago

@jamesagnew you may have broken a record for quick replies.

It is, in fact causing an issue. I ran into it while incrementing the core library in HAPI: https://github.com/hapifhir/hapi-fhir/pull/6510.

https://github.com/hapifhir/hapi-fhir/blob/59cf44c6066749cd4c7bee5eade30a6fd31ffc37/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/utils/FhirPathEngineR4Test.java#L43-L67

Grahame's last updates to FHIRPathEngine now expect resources to match the spec for contained resources:

https://github.com/hapifhir/org.hl7.fhir.core/blame/8869698a48a0638382d0c2523b502432459a1605/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/fhirpath/FHIRPathEngine.java#L5460-L5470

dotasek commented 5 hours ago

@tadgh and I looked into this after I mentioned it, and it's not just reading from JSON that displays odd behaviour.

FhirTerser appears to be mutating the contained resources in a multitude of irregular ways. For example:

https://github.com/hapifhir/hapi-fhir/blob/3b8569127e6ccb2a18a1ec769c6df45a216b8209/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java#L1814-L1816

I've outlined a few major ones here, with FIXME annotations where the tests are producing unexpected results.

https://github.com/hapifhir/hapi-fhir/pull/6518