HAPI JSON parser is adding # to contained resource IDs.

[dotasek]

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

To Reproduce

I created a breaking test in a branch:

hapi-fhir/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java

Lines 1383 to 1389 in 9e9f1e6

	public void testContainedResourceIdIsReadWithoutAddingHash() throws IOException {
	String text = loadResource("/observation-with-contained-specimen.json");
	Observation observation = ourCtx.newJsonParser().parseResource(Observation.class, text);
	assertThat(observation.getSpecimen().getReference()).isEqualTo("#contained-id");
	assertThat(observation.getContained().get(0).getId()).isEqualTo("contained-id");
	}

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):

• HAPI FHIR Version: master branch
• OS: mac

[dotasek]

The code that adds the # appears to be here:

hapi-fhir/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java

Line 442 in 77fa7f7

res.getIdElement().setValue('#' + res.getIdElement().getIdPart());

[jamesagnew]

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]

@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: #6510.

hapi-fhir/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/utils/FhirPathEngineR4Test.java

Lines 43 to 67 in 59cf44c

	public void testCrossResourceBoundaries() throws FHIRException {
	Specimen specimen = new Specimen();
	specimen.setReceivedTimeElement(new DateTimeType("2011-01-01"));
	Observation o = new Observation();
	o.setId("O1");
	o.setStatus(Observation.ObservationStatus.FINAL);
	o.setSpecimen(new Reference(specimen));
	IParser p = ourCtx.newJsonParser();
	o = (Observation) p.parseResource(p.encodeResourceToString(o));
	List<Base> value;
	value = ourCtx.newFhirPath().evaluate(o, "Observation.specimen", Base.class);
	assertThat(value).hasSize(1);
	value = ourCtx.newFhirPath().evaluate(o, "Observation.specimen.resolve()", Base.class);
	assertThat(value).hasSize(1);
	value = ourCtx.newFhirPath().evaluate(o, "Observation.specimen.resolve().receivedTime", Base.class);
	assertThat(value).hasSize(1);
	assertEquals("2011-01-01", ((DateTimeType) value.get(0)).getValueAsString());
	}

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]

@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:

hapi-fhir/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java

Lines 1814 to 1816 in 3b85691

	if (isBlank(newId.getValue())) {
	newId.setValue("#" + UUID.randomUUID());
	}

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

#6518

[tadgh]

Having discussed with james, here's what we will do:

1. Fix the bug with the # being added to the contained ID during Deserialization.
2. Do not fix the bug with the resources being mutated by the encoder.
3. Add a note to upgrade.md indicating that this breaking API change in the JSON parser has occurred.

[dotasek]

@tadgh For clarity:

2. Do not fix the bug with the resources being mutated by the encoder.

2a. We are still expecting the resources to be mutated, in that IDs are to be generated
2b. The generated IDs should be consistent with the spec (contained resource ID doesn't start with #, but reference ID does)