Don't contain resources in Bundle with only fullUrl (#5589)

* Don't contain resources in Bundle with only fullUrl

* Add changelog

* Spotless

* Fix RDF issue

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

@@ -49,6 +49,7 @@
 import org.apache.commons.io.output.StringBuilderWriter;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
+import org.apache.commons.lang3.tuple.Pair;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseBundle;
 import org.hl7.fhir.instance.model.api.IBaseCoding;
@@ -82,6 +83,7 @@
 
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.apache.commons.lang3.StringUtils.startsWith;
 
 @SuppressWarnings("WeakerAccess")
 public abstract class BaseParser implements IParser {
@@ -328,11 +330,7 @@ protected void encodeResourceToWriter(IBaseResource theResource, Writer theWrite
 		Validate.notNull(theWriter, "theWriter can not be null");
 		Validate.notNull(theEncodeContext, "theEncodeContext can not be null");
 
-		if (myContext.getVersion().getVersion() == FhirVersionEnum.R4B
-				&& theResource.getStructureFhirVersionEnum() == FhirVersionEnum.R5) {
-			// TODO: remove once we've bumped the core lib version
-		} else if (theResource.getStructureFhirVersionEnum()
-				!= myContext.getVersion().getVersion()) {
+		if (theResource.getStructureFhirVersionEnum() != myContext.getVersion().getVersion()) {
 			throw new IllegalArgumentException(Msg.code(1829) + "This parser is for FHIR version "
 					+ myContext.getVersion().getVersion() + " - Can not encode a structure for version "
 					+ theResource.getStructureFhirVersionEnum());
@@ -444,10 +442,6 @@ FhirTerser.ContainedResources getContainedResources() {
 		return myContainedResources;
 	}
 
-	void setContainedResources(FhirTerser.ContainedResources theContainedResources) {
-		myContainedResources = theContainedResources;
-	}
-
 	@Override
 	public Set<String> getDontStripVersionsFromReferencesAtPaths() {
 		return myDontStripVersionsFromReferencesAtPaths;
@@ -1010,6 +1004,32 @@ protected boolean isFhirVersionLessThanOrEqualTo(FhirVersionEnum theFhirVersionE
 		return theFhirVersionEnum == apiFhirVersion || apiFhirVersion.isOlderThan(theFhirVersionEnum);
 	}
 
+	protected void containResourcesInReferences(IBaseResource theResource) {
+
+		/*
+		 * If a UUID is present in Bundle.entry.fullUrl but no value is present
+		 * in Bundle.entry.resource.id, the resource has a discrete identity which
+		 * should be copied into the resource ID. It will not be serialized in
+		 * Resource.id because it's a placeholder/UUID value, but its presence there
+		 * informs the serializer that we don't need to contain this resource.
+		 */
+		if (theResource instanceof IBaseBundle) {
+			List<Pair<String, IBaseResource>> entries =
+					BundleUtil.getBundleEntryFullUrlsAndResources(getContext(), (IBaseBundle) theResource);
+			for (Pair<String, IBaseResource> nextEntry : entries) {
+				String fullUrl = nextEntry.getKey();
+				IBaseResource resource = nextEntry.getValue();
+				if (startsWith(fullUrl, "urn:")) {
+					if (resource != null && resource.getIdElement().getValue() == null) {
+						resource.getIdElement().setValue(fullUrl);
+					}
+				}
+			}
+		}
+
+		myContainedResources = getContext().newTerser().containResources(theResource);
+	}
+
 	class ChildNameAndDef {
 
 		private final BaseRuntimeElementDefinition<?> myChildDef;

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

@@ -892,7 +892,7 @@ private void encodeResourceToJsonStreamWriter(
 		}
 
 		if (!theContainedResource) {
-			setContainedResources(getContext().newTerser().containResources(theResource));
+			containResourcesInReferences(theResource);
 		}
 
 		RuntimeResourceDefinition resDef = getContext().getResourceDefinition(theResource);

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

@@ -191,7 +191,7 @@ private Resource encodeResourceToRDFStreamWriter(
 		}
 
 		if (!containedResource) {
-			setContainedResources(getContext().newTerser().containResources(resource));
+			containResourcesInReferences(resource);
 		}
 
 		if (!(resource instanceof IAnyResource)) {
@@ -238,7 +238,9 @@ private Resource encodeResourceToRDFStreamWriter(
 					rdfModel.createProperty(FHIR_NS + NODE_ROLE), rdfModel.createProperty(FHIR_NS + TREE_ROOT));
 		}
 
-		if (resourceId != null && resourceId.getIdPart() != null) {
+		if (resourceId != null
+				&& resourceId.getIdPart() != null
+				&& !resourceId.getValue().startsWith("urn:")) {
 			parentResource.addProperty(
 					rdfModel.createProperty(FHIR_NS + RESOURCE_ID),
 					createFhirValueBlankNode(rdfModel, resourceId.getIdPart()));

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

@@ -682,7 +682,7 @@ private void encodeResourceToXmlStreamWriter(
 		}
 
 		if (!theContainedResource) {
-			setContainedResources(getContext().newTerser().containResources(theResource));
+			containResourcesInReferences(theResource);
 		}
 
 		theEventWriter.writeStartElement(resDef.getName());

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

@@ -153,7 +153,42 @@ private static String getLinkNoCheck(FhirContext theContext, IBaseBundle theBund
 		return getLinkUrlOfType(theContext, theBundle, theLinkRelation, false);
 	}
 
+	/**
+	 * Returns a collection of Pairs, one for each entry in the bundle. Each pair will contain
+	 * the values of Bundle.entry.fullUrl, and Bundle.entry.resource respectively. Nulls
+	 * are possible in either or both values in the Pair.
+	 *
+	 * @since 7.0.0
+	 */
 	@SuppressWarnings("unchecked")
+	public static List<Pair<String, IBaseResource>> getBundleEntryFullUrlsAndResources(
+			FhirContext theContext, IBaseBundle theBundle) {
+		RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
+		BaseRuntimeChildDefinition entryChild = def.getChildByName("entry");
+		List<IBase> entries = entryChild.getAccessor().getValues(theBundle);
+
+		BaseRuntimeElementCompositeDefinition<?> entryChildElem =
+				(BaseRuntimeElementCompositeDefinition<?>) entryChild.getChildByName("entry");
+		BaseRuntimeChildDefinition resourceChild = entryChildElem.getChildByName("resource");
+
+		BaseRuntimeChildDefinition urlChild = entryChildElem.getChildByName("fullUrl");
+
+		List<Pair<String, IBaseResource>> retVal = new ArrayList<>(entries.size());
+		for (IBase nextEntry : entries) {
+
+			String fullUrl = urlChild.getAccessor()
+					.getFirstValueOrNull(nextEntry)
+					.map(t -> (((IPrimitiveType<?>) t).getValueAsString()))
+					.orElse(null);
+			IBaseResource resource = (IBaseResource)
+					resourceChild.getAccessor().getFirstValueOrNull(nextEntry).orElse(null);
+
+			retVal.add(Pair.of(fullUrl, resource));
+		}
+
+		return retVal;
+	}
+
 	public static List<Pair<String, IBaseResource>> getBundleEntryUrlsAndResources(
 			FhirContext theContext, IBaseBundle theBundle) {
 		RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
@@ -173,19 +208,15 @@ public static List<Pair<String, IBaseResource>> getBundleEntryUrlsAndResources(
 		List<Pair<String, IBaseResource>> retVal = new ArrayList<>(entries.size());
 		for (IBase nextEntry : entries) {
 
-			String url = null;
-			IBaseResource resource = null;
-
-			for (IBase nextEntryValue : requestChild.getAccessor().getValues(nextEntry)) {
-				for (IBase nextUrlValue : urlChild.getAccessor().getValues(nextEntryValue)) {
-					url = ((IPrimitiveType<String>) nextUrlValue).getValue();
-				}
-			}
+			String url = requestChild
+					.getAccessor()
+					.getFirstValueOrNull(nextEntry)
+					.flatMap(e -> urlChild.getAccessor().getFirstValueOrNull(e))
+					.map(t -> ((IPrimitiveType<?>) t).getValueAsString())
+					.orElse(null);
 
-			// Should return 0..1 only
-			for (IBase nextValue : resourceChild.getAccessor().getValues(nextEntry)) {
-				resource = (IBaseResource) nextValue;
-			}
+			IBaseResource resource = (IBaseResource)
+					resourceChild.getAccessor().getFirstValueOrNull(nextEntry).orElse(null);
 
 			retVal.add(Pair.of(url, resource));
 		}

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5589-dont-contain-resources-with-fullurl.yaml

@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 5589
+title: "When encoding a Bundle, if resources in bundle entries had a value in 
+   `Bundle.entry.fullUrl` but no value in `Bundle.entry.resource.id`, the parser
+   sometimes incorrectly moved these resources to be contained within other
+   resources when serializing the bundle. This has been corrected."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5589-dont-encode-placeholder-ids-in-rdf.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 5589
+title: "When encoding resources using the RDF parser, placeholder IDs (i.e. resource IDs
+   starting with `urn:`) were not omitted as they are in the XML and JSON parsers. This has
+   been corrected."

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java

@@ -682,12 +682,6 @@ public static IIdType fullyQualifyResourceIdOrReturnNull(
 	}
 
 	private static FhirContext getContextForVersion(FhirContext theContext, FhirVersionEnum theForVersion) {
-
-		// TODO: remove once we've bumped the core lib version
-		if (theContext.getVersion().getVersion() == FhirVersionEnum.R4B && theForVersion == FhirVersionEnum.R5) {
-			return theContext;
-		}
-
 		FhirContext context = theContext;
 		if (context.getVersion().getVersion() != theForVersion) {
 			context = myFhirContextMap.get(theForVersion);

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

@@ -27,7 +27,6 @@
 import org.hl7.fhir.r4.model.Encounter;
 import org.hl7.fhir.r4.model.Extension;
 import org.hl7.fhir.r4.model.HumanName;
-import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Identifier;
 import org.hl7.fhir.r4.model.Medication;
 import org.hl7.fhir.r4.model.MedicationDispense;
@@ -37,13 +36,13 @@
 import org.hl7.fhir.r4.model.Narrative;
 import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Organization;
+import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Practitioner;
 import org.hl7.fhir.r4.model.PrimitiveType;
 import org.hl7.fhir.r4.model.Quantity;
 import org.hl7.fhir.r4.model.QuestionnaireResponse;
 import org.hl7.fhir.r4.model.Reference;
-import org.hl7.fhir.r4.model.Resource;
 import org.hl7.fhir.r4.model.StringType;
 import org.hl7.fhir.r4.model.Type;
 import org.hl7.fhir.utilities.xhtml.XhtmlNode;
@@ -54,6 +53,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Date;
@@ -1200,6 +1200,87 @@ public void testEncodeToString_BackboneElement() {
 		assertEquals(expected, actual);
 	}
 
+	@Test
+	public void testEncodeBundleWithCrossReferenceFullUrlsAndNoIds() {
+		Bundle bundle = createBundleWithCrossReferenceFullUrlsAndNoIds();
+
+		String output = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle);
+		ourLog.info(output);
+
+		assertThat(output, not(containsString("\"contained\"")));
+		assertThat(output, not(containsString("\"id\"")));
+		assertThat(output, stringContainsInOrder(
+			 "\"fullUrl\": \"urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7\",",
+			 "\"resourceType\": \"Patient\"",
+			 "\"fullUrl\": \"urn:uuid:71d7ab79-a001-41dc-9a8e-b3e478ce1cbb\"",
+			 "\"resourceType\": \"Observation\"",
+			 "\"reference\": \"urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7\""
+		));
+
+	}
+
+	@Test
+	public void testEncodeBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters() {
+		Parameters parameters = createBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters();
+
+		String output = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(parameters);
+		ourLog.info(output);
+
+		assertThat(output, not(containsString("\"contained\"")));
+		assertThat(output, not(containsString("\"id\"")));
+		assertThat(output, stringContainsInOrder(
+			 "\"resourceType\": \"Parameters\"",
+			 "\"name\": \"resource\"",
+			 "\"fullUrl\": \"urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7\",",
+			 "\"resourceType\": \"Patient\"",
+			 "\"fullUrl\": \"urn:uuid:71d7ab79-a001-41dc-9a8e-b3e478ce1cbb\"",
+			 "\"resourceType\": \"Observation\"",
+			 "\"reference\": \"urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7\""
+		));
+
+	}
+
+	@Test
+	public void testParseBundleWithCrossReferenceFullUrlsAndNoIds() {
+		Bundle bundle = createBundleWithCrossReferenceFullUrlsAndNoIds();
+		String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle);
+
+		Bundle parsedBundle = ourCtx.newJsonParser().parseResource(Bundle.class, encoded);
+		assertEquals("urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7", parsedBundle.getEntry().get(0).getFullUrl());
+		assertEquals("urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7", parsedBundle.getEntry().get(0).getResource().getId());
+		assertEquals("urn:uuid:71d7ab79-a001-41dc-9a8e-b3e478ce1cbb", parsedBundle.getEntry().get(1).getFullUrl());
+		assertEquals("urn:uuid:71d7ab79-a001-41dc-9a8e-b3e478ce1cbb", parsedBundle.getEntry().get(1).getResource().getId());
+	}
+
+	@Nonnull
+	public static Bundle createBundleWithCrossReferenceFullUrlsAndNoIds() {
+		Bundle bundle = new Bundle();
+
+		Patient patient = new Patient();
+		patient.setActive(true);
+		bundle
+			 .addEntry()
+			 .setResource(patient)
+			 .setFullUrl("urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7");
+
+		Observation observation = new Observation();
+		observation.getSubject().setReference("urn:uuid:9e9187c1-db6d-4b6f-adc6-976153c65ed7").setResource(patient);
+		bundle
+			 .addEntry()
+			 .setResource(observation)
+			 .setFullUrl("urn:uuid:71d7ab79-a001-41dc-9a8e-b3e478ce1cbb");
+		return bundle;
+	}
+
+	@Nonnull
+	public static Parameters createBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters() {
+		Parameters retVal = new Parameters();
+		retVal
+			 .addParameter()
+			 .setName("resource")
+			 .setResource(createBundleWithCrossReferenceFullUrlsAndNoIds());
+		return retVal;
+	}
 
 	@Test
 	public void testPreCommentsToFhirComments() {

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

@@ -36,8 +36,11 @@
 import org.eclipse.rdf4j.rio.Rio;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Base;
+import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.Parameters;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -59,6 +62,12 @@
 import java.util.Optional;
 import java.util.stream.Stream;
 
+import static ca.uhn.fhir.parser.JsonParserR4Test.createBundleWithCrossReferenceFullUrlsAndNoIds;
+import static ca.uhn.fhir.parser.JsonParserR4Test.createBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.stringContainsInOrder;
+import static org.hamcrest.core.IsNot.not;
 import static org.junit.jupiter.api.Assertions.*;
 
 public class RDFParserTest extends BaseTest {
@@ -216,4 +225,27 @@ public String toString() {
 		}
 	}
 
+
+	@Test
+	public void testEncodeBundleWithCrossReferenceFullUrlsAndNoIds() {
+		Bundle bundle = createBundleWithCrossReferenceFullUrlsAndNoIds();
+
+		String output = ourCtx.newRDFParser().setPrettyPrint(true).encodeResourceToString(bundle);
+		ourLog.info(output);
+
+		assertThat(output, not(containsString("contained ")));
+		assertThat(output, not(containsString("id ")));
+	}
+
+	@Test
+	public void testEncodeBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters() {
+		Parameters parameters = createBundleWithCrossReferenceFullUrlsAndNoIds_NestedInParameters();
+
+		String output = ourCtx.newRDFParser().setPrettyPrint(true).encodeResourceToString(parameters);
+		ourLog.info(output);
+
+		assertThat(output, not(containsString("contained ")));
+		assertThat(output, not(containsString("id ")));
+	}
+
 }