Fix #814 - Don't create duplicate contained resources

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

@@ -9,9 +9,9 @@ package ca.uhn.fhir.parser;
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -38,6 +38,7 @@ import java.util.*;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
+@SuppressWarnings("WeakerAccess")
 public abstract class BaseParser implements IParser {
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseParser.class);
@@ -156,7 +157,6 @@ public abstract class BaseParser implements IParser {
 	}
 
 	private void containResourcesForEncoding(ContainedResources theContained, IBaseResource theResource, IBaseResource theTarget) {
-		Set<String> allIds = new HashSet<String>();
 		Map<String, IBaseResource> existingIdToContainedResource = null;
 
 		if (theTarget instanceof IResource) {
@@ -167,9 +167,8 @@ public abstract class BaseParser implements IParser {
 					if (!nextId.startsWith("#")) {
 						nextId = '#' + nextId;
 					}
-					allIds.add(nextId);
 					if (existingIdToContainedResource == null) {
-						existingIdToContainedResource = new HashMap<String, IBaseResource>();
+						existingIdToContainedResource = new HashMap<>();
 					}
 					existingIdToContainedResource.put(nextId, next);
 				}
@@ -182,9 +181,8 @@ public abstract class BaseParser implements IParser {
 					if (!nextId.startsWith("#")) {
 						nextId = '#' + nextId;
 					}
-					allIds.add(nextId);
 					if (existingIdToContainedResource == null) {
-						existingIdToContainedResource = new HashMap<String, IBaseResource>();
+						existingIdToContainedResource = new HashMap<>();
 					}
 					existingIdToContainedResource.put(nextId, next);
 				}
@@ -480,7 +478,7 @@ public abstract class BaseParser implements IParser {
 	@Override
 	public void setPreferTypes(List<Class<? extends IBaseResource>> thePreferTypes) {
 		if (thePreferTypes != null) {
-			ArrayList<Class<? extends IBaseResource>> types = new ArrayList<Class<? extends IBaseResource>>();
+			ArrayList<Class<? extends IBaseResource>> types = new ArrayList<>();
 			for (Class<? extends IBaseResource> next : thePreferTypes) {
 				if (Modifier.isAbstract(next.getModifiers()) == false) {
 					types.add(next);
@@ -516,8 +514,7 @@ public abstract class BaseParser implements IParser {
 				}
 			}
 
-			List<T> newList = new ArrayList<T>();
+			List<T> newList = new ArrayList<>(theProfiles);
-			newList.addAll(theProfiles);
 
 			BaseRuntimeElementDefinition<?> idElement = myContext.getElementDefinition("id");
 			@SuppressWarnings("unchecked")
@@ -1183,8 +1180,10 @@ public abstract class BaseParser implements IParser {
 		}
 
 		public void addContained(IIdType theId, IBaseResource theResource) {
-			myResourceToId.put(theResource, theId);
+			if (!hasId(theId)) {
-			myResources.add(theResource);
+				myResourceToId.put(theResource, theId);
+				myResources.add(theResource);
+			}
 		}
 
 		public List<IBaseResource> getContainedResources() {
@@ -1195,6 +1194,15 @@ public abstract class BaseParser implements IParser {
 			return myResourceToId.get(theNext);
 		}
 
+		public boolean hasId(IIdType theId) {
+			for (IIdType next : myResourceToId.values()) {
+				if (Objects.equals(next.getValue(), theId.getValue())) {
+					return true;
+				}
+			}
+			return false;
+		}
+
 		public boolean isEmpty() {
 			return myResourceToId.isEmpty();
 		}

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

@@ -88,6 +88,39 @@ public class JsonParserR4Test {
 
 	}
 
+	/*
+	 * See #814
+	 */
+	@Test
+	public void testDuplicateContainedResourcesNotOutputtedTwiceWithManualIdsAndManualAddition() {
+		MedicationDispense md = new MedicationDispense();
+
+		MedicationRequest mr = new MedicationRequest();
+		mr.setId("#MR");
+		md.addAuthorizingPrescription().setResource(mr);
+
+		Medication med = new Medication();
+		med.setId("#MED");
+
+		Reference medRef = new Reference();
+		medRef.setReference("#MED");
+		md.setMedication(medRef);
+		mr.setMedication(medRef);
+
+		md.getContained().add(mr);
+		md.getContained().add(med);
+
+		String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(md);
+		ourLog.info(encoded);
+
+		int idx = encoded.indexOf("\"Medication\"");
+		assertNotEquals(-1, idx);
+
+		idx = encoded.indexOf("\"Medication\"", idx + 1);
+		assertEquals(-1, idx);
+
+	}
+
 	@Test
 	public void testExcludeNothing() {
 		IParser parser = ourCtx.newJsonParser().setPrettyPrint(true);

src/changes/changes.xml

@@ -189,6 +189,11 @@
 				DateParam class now has equals() and hashCode() implementations. Thanks
 				to Gaetano Gallo for the pull request!
 			</action>
+			<action type="fix" issue="814">
+				Fix a bug where under certain circumstances, duplicate contained resources
+				could be output by the parser's encode methods. Thanks to
+				Frank Tao for supplying a test case!
+			</action>
 		</release>
 		<release version="3.2.0" date="2018-01-13">
 			<action type="add">