Code review updates

hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmEidUpdateService.java

@@ -81,7 +81,9 @@ public class MdmEidUpdateService {
 			//This is a new linking scenario. we have to break the existing link and link to the new Golden Resource. For now, we create duplicate.
 			//updated patient has an EID that matches to a new candidate. Link them, and set the Golden Resources possible duplicates
 			linkToNewGoldenResourceAndFlagAsDuplicate(theTargetResource, updateContext.getExistingGoldenResource(), updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
-			// TODO NG - Do we need to merge fields from the old golden resource to the new golden resource?
+
+			myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(theTargetResource, updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
+			myMdmResourceDaoSvc.upsertGoldenResource(updateContext.getMatchedGoldenResource(), theMdmTransactionContext.getResourceType());
 		}
 	}
 

hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkUpdaterSvcImpl.java

@@ -94,7 +94,6 @@ public class MdmLinkUpdaterSvcImpl implements IMdmLinkUpdaterSvc {
 		mdmLink.setLinkSource(MdmLinkSourceEnum.MANUAL);
 		myMdmLinkDaoSvc.save(mdmLink);
 
-		// IAnyResource resource = (theManuallyMergedGoldenResource == null) ? theSourceResource : theManuallyMergedGoldenResource;
 		if (theMatchResult == MdmMatchResultEnum.MATCH) {
 			// only apply survivorship rules in case of a match
 			myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(theSourceResource, theGoldenResource, theMdmContext);

hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmSurvivorshipSvcImpl.java

@@ -34,12 +34,9 @@ public class MdmSurvivorshipSvcImpl implements IMdmSurvivorshipService {
 	private FhirContext myFhirContext;
 
 	/**
-	 * TODO NG Update docs to tell taht we also do that in non GR merge - like linking
+	 * Merges two golden resources by overwriting all field values on theGoldenResource param for CREATE_RESOURCE,
-	 *
+	 * UPDATE_RESOURCE, SUBMIT_RESOURCE_TO_MDM, UPDATE_LINK (when setting to MATCH) and MANUAL_MERGE_GOLDEN_RESOURCES.
-	 * Merges two golden resources by overwriting all field values on theGoldenResource param for all REST operation methods
+	 * PID, identifiers and meta values are not affected by this operation.
-	 * except MERGE_GOLDEN_RESOURCES. In case of MERGE_GOLDEN_RESOURCES, it will attempt to copy field values from
-	 * theTargetResource that do not exist in theGoldenResource. PID, indentifiers and meta values are not affected by
-	 * this operation.
 	 *
 	 * @param theTargetResource        Target resource to retrieve fields from
 	 * @param theGoldenResource        Golden resource to merge fields into

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmGoldenResourceMergerSvcTest.java

@@ -18,6 +18,7 @@ import org.hl7.fhir.r4.model.DateType;
 import org.hl7.fhir.r4.model.Enumerations;
 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.Patient;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -34,8 +35,7 @@ import java.util.stream.Collectors;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.*;
-import static org.junit.jupiter.api.Assertions.fail;
 
 public class MdmGoldenResourceMergerSvcTest extends BaseMdmR4Test {
 
@@ -409,6 +409,9 @@ public class MdmGoldenResourceMergerSvcTest extends BaseMdmR4Test {
 		assertThat(mergedSourcePatient.getName(), hasSize(2));
 		assertThat(mergedSourcePatient.getName().get(0).getGiven(), hasSize(2));
 		assertThat(mergedSourcePatient.getName().get(1).getGiven(), hasSize(2));
+
+		assertThat(mergedSourcePatient.getName().get(0).getNameAsSingleString(), is("Jeff George"));
+		assertThat(mergedSourcePatient.getName().get(1).getNameAsSingleString(), is("Jim George"));
 	}
 
 	@Test
@@ -426,6 +429,8 @@ public class MdmGoldenResourceMergerSvcTest extends BaseMdmR4Test {
 		mergeGoldenPatients();
 		assertThat(myToGoldenPatient.getName(), hasSize(1));
 		assertThat(myToGoldenPatient.getName().get(0).getGiven(), hasSize(2));
+
+		assertThat(myToGoldenPatient.getName().get(0).getNameAsSingleString(), is("Jim George"));
 	}
 
 	@Test
@@ -442,6 +447,11 @@ public class MdmGoldenResourceMergerSvcTest extends BaseMdmR4Test {
 		mergeGoldenPatients();
 
 		assertThat(myToGoldenPatient.getIdentifier(), hasSize(4));
+
+		assertTrue(myToGoldenPatient.getIdentifier().get(0).equalsDeep(new Identifier().setValue("aaa").setSystem("SYSTEM1")));
+		assertTrue(myToGoldenPatient.getIdentifier().get(1).equalsDeep(new Identifier().setValue("ccc").setSystem("SYSTEM1")));
+		assertTrue(myToGoldenPatient.getIdentifier().get(2).equalsDeep(new Identifier().setValue("bbb").setSystem("SYSTEM1")));
+		assertTrue(myToGoldenPatient.getIdentifier().get(3).equalsDeep(new Identifier().setValue("ccc").setSystem("SYSTEM2")));
 	}
 
 	private MdmLink createMdmLink(Patient theSourcePatient, Patient theTargetPatient) {

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmSurvivorshipSvcImplTest.java

@@ -1,7 +1,59 @@
 package ca.uhn.fhir.jpa.mdm.svc;
 
-import static org.junit.jupiter.api.Assertions.*;
+import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
+import ca.uhn.fhir.mdm.api.IMdmSurvivorshipService;
+import ca.uhn.fhir.mdm.model.MdmTransactionContext;
+import org.hl7.fhir.r4.model.Patient;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
 
-class MdmSurvivorshipSvcImplTest {
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class MdmSurvivorshipSvcImplTest extends BaseMdmR4Test {
+
+	@Autowired
+	private IMdmSurvivorshipService myMdmSurvivorshipService;
+
+	@Test
+	public void testRulesOnCreate() {
+		Patient p1 = buildFrankPatient();
+		Patient p2 = new Patient();
+
+		myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(p1, p2, new MdmTransactionContext(MdmTransactionContext.OperationType.CREATE_RESOURCE));
+
+		assertFalse(p2.hasIdElement());
+		assertTrue(p2.getIdentifier().isEmpty());
+		assertTrue(p2.getMeta().isEmpty());
+
+		assertTrue(p1.getNameFirstRep().equalsDeep(p2.getNameFirstRep()));
+		assertNull(p2.getBirthDate());
+		assertEquals(p1.getTelecom().size(), p2.getTelecom().size());
+		assertTrue(p2.getTelecomFirstRep().equalsDeep(p1.getTelecomFirstRep()));
+	}
+
+	@Test
+	public void testRulesOnMerge() {
+		Patient p1 = buildFrankPatient();
+		String p1Name = p1.getNameFirstRep().getNameAsSingleString();
+		Patient p2 = buildPaulPatient();
+		String p2Name = p2.getNameFirstRep().getNameAsSingleString();
+
+		myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(p1, p2, new MdmTransactionContext(MdmTransactionContext.OperationType.MERGE_GOLDEN_RESOURCES));
+
+		assertFalse(p2.hasIdElement());
+		assertFalse(p2.getIdentifier().isEmpty());
+		assertTrue(p2.getMeta().isEmpty());
+
+		assertEquals(2, p2.getName().size());
+		assertEquals(p2Name, p2.getName().get(0).getNameAsSingleString());
+		assertEquals(p1Name, p2.getName().get(1).getNameAsSingleString());
+		assertNull(p2.getBirthDate());
+
+		assertEquals(p1.getTelecom().size(), p1.getTelecom().size());
+		assertTrue(p2.getTelecomFirstRep().equalsDeep(p1.getTelecomFirstRep()));
+	}
 
 }

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

@@ -131,7 +131,11 @@ public final class TerserUtil {
 			IBase newFieldValue = childDefinition.getChildByName(field).newInstance();
 			terser.cloneInto(theFromFieldValue, newFieldValue, true);
 
+			try {
 				theToFieldValues.add(newFieldValue);
+			} catch (Exception e) {
+				childDefinition.getMutator().setValue(theTo, newFieldValue);
+			}
 		}
 	}