mdm duplicate identifier (#5563)

* test and fix bug

* cleanup

* cleanup

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

@@ -97,21 +97,34 @@ public final class TerserUtil {
 	private TerserUtil() {}
 
 	/**
-	 * Given an Child Definition of `identifier`, a R4/DSTU3 EID Identifier, and a new resource, clone the EID into that resources' identifier list.
+	 * Given an Child Definition of `identifier`, a R4/DSTU3 Identifier, and a new resource, clone the identifier into that resources' identifier list if it is not already present.
 	 */
-	public static void cloneEidIntoResource(
+	public static void cloneIdentifierIntoResource(
 			FhirContext theFhirContext,
 			BaseRuntimeChildDefinition theIdentifierDefinition,
-			IBase theEid,
+			IBase theNewIdentifier,
-			IBase theResourceToCloneEidInto) {
+			IBaseResource theResourceToCloneInto) {
 		// FHIR choice types - fields within fhir where we have a choice of ids
-		BaseRuntimeElementCompositeDefinition<?> childIdentifier = (BaseRuntimeElementCompositeDefinition<?>)
+		BaseRuntimeElementCompositeDefinition<?> childIdentifierElementDefinition =
-				theIdentifierDefinition.getChildByName(FIELD_NAME_IDENTIFIER);
+				(BaseRuntimeElementCompositeDefinition<?>)
-		IBase resourceNewIdentifier = childIdentifier.newInstance();
+						theIdentifierDefinition.getChildByName(FIELD_NAME_IDENTIFIER);
+
+		List<IBase> existingIdentifiers = getValues(theFhirContext, theResourceToCloneInto, FIELD_NAME_IDENTIFIER);
+		if (existingIdentifiers != null) {
+			for (IBase existingIdentifier : existingIdentifiers) {
+				if (equals(existingIdentifier, theNewIdentifier)) {
+					ourLog.trace(
+							"Identifier {} already exists in resource {}", theNewIdentifier, theResourceToCloneInto);
+					return;
+				}
+			}
+		}
+
+		IBase newIdentifierBase = childIdentifierElementDefinition.newInstance();
 
 		FhirTerser terser = theFhirContext.newTerser();
-		terser.cloneInto(theEid, resourceNewIdentifier, true);
+		terser.cloneInto(theNewIdentifier, newIdentifierBase, true);
-		theIdentifierDefinition.getMutator().addValue(theResourceToCloneEidInto, resourceNewIdentifier);
+		theIdentifierDefinition.getMutator().addValue(theResourceToCloneInto, newIdentifierBase);
 	}
 
 	/**

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5563-mdm-dup-identifier.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 5563
+title: "Previously, certain mdm configuration could lead to duplicate eid identifier
+entries in golden resources.  This has been corrected"

hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/util/GoldenResourceHelper.java

@@ -159,7 +159,7 @@ public class GoldenResourceHelper {
 	private void cloneMDMEidsIntoNewGoldenResource(
 			BaseRuntimeChildDefinition theGoldenResourceIdentifier,
 			IAnyResource theIncomingResource,
-			IBase theNewGoldenResource) {
+			IBaseResource theNewGoldenResource) {
 		String incomingResourceType = myFhirContext.getResourceType(theIncomingResource);
 		String mdmEIDSystem = myMdmSettings.getMdmRules().getEnterpriseEIDSystemForResourceType(incomingResourceType);
 
@@ -182,7 +182,7 @@ public class GoldenResourceHelper {
 					ourLog.debug(
 							"Incoming resource EID System {} matches EID system in the MDM rules.  Copying to Golden Resource.",
 							incomingIdentifierSystemString);
-					ca.uhn.fhir.util.TerserUtil.cloneEidIntoResource(
+					ca.uhn.fhir.util.TerserUtil.cloneIdentifierIntoResource(
 							myFhirContext,
 							theGoldenResourceIdentifier,
 							incomingResourceIdentifier,
@@ -382,7 +382,7 @@ public class GoldenResourceHelper {
 		RuntimeResourceDefinition resourceDefinition = theFhirContext.getResourceDefinition(theResourceToCloneInto);
 		// hapi has 2 metamodels: for children and types
 		BaseRuntimeChildDefinition resourceIdentifier = resourceDefinition.getChildByName(FIELD_NAME_IDENTIFIER);
-		ca.uhn.fhir.util.TerserUtil.cloneEidIntoResource(
+		ca.uhn.fhir.util.TerserUtil.cloneIdentifierIntoResource(
 				theFhirContext,
 				resourceIdentifier,
 				IdentifierUtil.toId(theFhirContext, theEid),

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java

@@ -117,7 +117,7 @@ class TerserUtilTest {
 			  	""";
 
 	@Test
-	void testCloneEidIntoResource() {
+	void cloneIdentifierIntoResource() {
 		Identifier identifier = new Identifier().setSystem("http://org.com/sys").setValue("123");
 
 		Patient p1 = new Patient();
@@ -125,7 +125,25 @@ class TerserUtilTest {
 
 		Patient p2 = new Patient();
 		RuntimeResourceDefinition definition = ourFhirContext.getResourceDefinition(p1);
-		TerserUtil.cloneEidIntoResource(ourFhirContext, definition.getChildByName("identifier"), identifier, p2);
+		TerserUtil.cloneIdentifierIntoResource(ourFhirContext, definition.getChildByName("identifier"), identifier, p2);
+
+		assertEquals(1, p2.getIdentifier().size());
+		assertEquals(p1.getIdentifier().get(0).getSystem(), p2.getIdentifier().get(0).getSystem());
+		assertEquals(p1.getIdentifier().get(0).getValue(), p2.getIdentifier().get(0).getValue());
+	}
+
+	@Test
+	void cloneIdentifierIntoResourceNoDuplicates() {
+		Identifier identifier = new Identifier().setSystem("http://org.com/sys").setValue("123");
+
+		Patient p1 = new Patient();
+		p1.addIdentifier(identifier);
+
+		Patient p2 = new Patient();
+		Identifier dupIdentifier = new Identifier().setSystem("http://org.com/sys").setValue("123");
+		p2.addIdentifier(dupIdentifier);
+		RuntimeResourceDefinition definition = ourFhirContext.getResourceDefinition(p1);
+		TerserUtil.cloneIdentifierIntoResource(ourFhirContext, definition.getChildByName("identifier"), identifier, p2);
 
 		assertEquals(1, p2.getIdentifier().size());
 		assertEquals(p1.getIdentifier().get(0).getSystem(), p2.getIdentifier().get(0).getSystem());
@@ -171,7 +189,7 @@ class TerserUtilTest {
 	}
 
 	@Test
-	void testCloneEidIntoResourceViaHelper() {
+	void cloneIdentifierIntoResourceViaHelper() {
 		TerserUtilHelper p1Helper = TerserUtilHelper.newHelper(ourFhirContext, "Patient");
 		p1Helper.setField("identifier.system", "http://org.com/sys");
 		p1Helper.setField("identifier.value", "123");
@@ -182,7 +200,7 @@ class TerserUtilTest {
 		TerserUtilHelper p2Helper = TerserUtilHelper.newHelper(ourFhirContext, "Patient");
 		RuntimeResourceDefinition definition = p1Helper.getResourceDefinition();
 
-		TerserUtil.cloneEidIntoResource(ourFhirContext, definition.getChildByName("identifier"),
+		TerserUtil.cloneIdentifierIntoResource(ourFhirContext, definition.getChildByName("identifier"),
 			 p1.getIdentifier().get(0), p2Helper.getResource());
 
 		assertEquals(1, p2Helper.getFieldValues("identifier").size());