Disallow matching a resource to more than one golden record (#4344)

Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4156-mdm-one-source-resource-can-be-matched-to-multimple-golden-resources.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 4156
+title: "Before multiple MDM `POSSIBLE_MATCH` links pointing to different Golden Resources could be accepted (updated to `MATCH`). 
+   This has now been fixed, allowing only one MATCH."

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

@@ -21,14 +21,13 @@ package ca.uhn.fhir.jpa.mdm.svc;
  */
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.svc.IIdHelperService;
 import ca.uhn.fhir.jpa.mdm.dao.MdmLinkDaoSvc;
 import ca.uhn.fhir.jpa.mdm.util.MdmPartitionHelper;
 import ca.uhn.fhir.jpa.model.entity.PartitionablePartitionId;
-import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.mdm.api.IMdmLink;
-import ca.uhn.fhir.mdm.api.IMdmLinkSvc;
 import ca.uhn.fhir.mdm.api.IMdmLinkUpdaterSvc;
 import ca.uhn.fhir.mdm.api.IMdmSettings;
 import ca.uhn.fhir.mdm.api.IMdmSurvivorshipService;
@@ -47,6 +46,7 @@ import org.slf4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.annotation.Transactional;
 
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -61,8 +61,6 @@ public class MdmLinkUpdaterSvcImpl implements IMdmLinkUpdaterSvc {
 	@Autowired
 	MdmLinkDaoSvc myMdmLinkDaoSvc;
 	@Autowired
-	IMdmLinkSvc myMdmLinkSvc;
-	@Autowired
 	MdmResourceDaoSvc myMdmResourceDaoSvc;
 	@Autowired
 	MdmMatchLinkSvc myMdmMatchLinkSvc;
@@ -83,17 +81,20 @@ public class MdmLinkUpdaterSvcImpl implements IMdmLinkUpdaterSvc {
 		validateUpdateLinkRequest(theGoldenResource, theSourceResource, theMatchResult, sourceType);
 
 		ResourcePersistentId goldenResourceId = myIdHelperService.getPidOrThrowException(theGoldenResource);
-		ResourcePersistentId targetId = myIdHelperService.getPidOrThrowException(theSourceResource);
+		ResourcePersistentId sourceResourceId = myIdHelperService.getPidOrThrowException(theSourceResource);
 
 		// check if the golden resource and the source resource are in the same partition, throw error if not
 		myMdmPartitionHelper.validateResourcesInSamePartition(theGoldenResource, theSourceResource);
 
-		Optional<? extends IMdmLink> optionalMdmLink = myMdmLinkDaoSvc.getLinkByGoldenResourcePidAndSourceResourcePid(goldenResourceId, targetId);
+		Optional<? extends IMdmLink> optionalMdmLink = myMdmLinkDaoSvc.getLinkByGoldenResourcePidAndSourceResourcePid(goldenResourceId, sourceResourceId);
-		if (!optionalMdmLink.isPresent()) {
+		if (optionalMdmLink.isEmpty()) {
 			throw new InvalidRequestException(Msg.code(738) + myMessageHelper.getMessageForNoLink(theGoldenResource, theSourceResource));
 		}
 
 		IMdmLink mdmLink = optionalMdmLink.get();
+
+		validateNoMatchPresentWhenAcceptingPossibleMatch(theSourceResource, goldenResourceId, theMatchResult);
+
 		if (mdmLink.getMatchResult() == theMatchResult) {
 			ourLog.warn("MDM Link for " + theGoldenResource.getIdElement().toVersionless() + ", " + theSourceResource.getIdElement().toVersionless() + " already has value " + theMatchResult + ".  Nothing to do.");
 			return theGoldenResource;
@@ -124,6 +125,31 @@ public class MdmLinkUpdaterSvcImpl implements IMdmLinkUpdaterSvc {
 		return theGoldenResource;
 	}
 
+	/**
+	 * When updating POSSIBLE_MATCH link to a MATCH we need to validate that a MATCH to a different golden resource
+	 * doesn't exist, because a resource mustn't be a MATCH to more than one golden resource
+	 */
+	private void validateNoMatchPresentWhenAcceptingPossibleMatch(IAnyResource theSourceResource,
+					ResourcePersistentId theGoldenResourceId, MdmMatchResultEnum theMatchResult) {
+
+		// if theMatchResult != MATCH, we are not accepting POSSIBLE_MATCH so there is nothing to validate
+		if (theMatchResult != MdmMatchResultEnum.MATCH) { return; }
+
+		ResourcePersistentId sourceResourceId = myIdHelperService.getPidOrThrowException(theSourceResource);
+		List<? extends IMdmLink> mdmLinks = myMdmLinkDaoSvc
+			.getMdmLinksBySourcePidAndMatchResult(sourceResourceId, MdmMatchResultEnum.MATCH);
+
+		// if a link for a different golden resource exists, throw an exception
+		for (IMdmLink mdmLink : mdmLinks) {
+			if (mdmLink.getGoldenResourcePersistenceId() != theGoldenResourceId) {
+				IAnyResource existingGolden = myMdmResourceDaoSvc.readGoldenResourceByPid(mdmLink.getGoldenResourcePersistenceId(), mdmLink.getMdmSourceType());
+				throw new InvalidRequestException(Msg.code(2218) +
+					myMessageHelper.getMessageForAlreadyAcceptedLink(existingGolden, theSourceResource));
+			}
+		}
+	}
+
+
 	private void validateUpdateLinkRequest(IAnyResource theGoldenRecord, IAnyResource theSourceResource, MdmMatchResultEnum theMatchResult, String theSourceType) {
 		String goldenRecordType = myFhirContext.getResourceType(theGoldenRecord);
 
@@ -162,7 +188,7 @@ public class MdmLinkUpdaterSvcImpl implements IMdmLinkUpdaterSvc {
 		ResourcePersistentId targetId = myIdHelperService.getPidOrThrowException(theTargetGoldenResource);
 
 		Optional<? extends IMdmLink> oMdmLink = myMdmLinkDaoSvc.getLinkByGoldenResourcePidAndSourceResourcePid(goldenResourceId, targetId);
-		if (!oMdmLink.isPresent()) {
+		if (oMdmLink.isEmpty()) {
 			throw new InvalidRequestException(Msg.code(745) + "No link exists between " + theGoldenResource.getIdElement().toVersionless() + " and " + theTargetGoldenResource.getIdElement().toVersionless());
 		}
 

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

@@ -0,0 +1,119 @@
+package ca.uhn.fhir.jpa.mdm.svc;
+
+import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
+import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
+import ca.uhn.fhir.jpa.partition.SystemRequestDetails;
+import ca.uhn.fhir.mdm.api.IMdmLink;
+import ca.uhn.fhir.mdm.api.IMdmLinkUpdaterSvc;
+import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
+import ca.uhn.fhir.mdm.api.MdmMatchOutcome;
+import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
+import ca.uhn.fhir.mdm.model.MdmTransactionContext;
+import ca.uhn.fhir.mdm.util.MessageHelper;
+import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import org.hl7.fhir.r4.model.Patient;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.util.ResourceUtils;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class MdmLinkUpdaterSvcImplIT extends BaseMdmR4Test {
+
+	public static final String TEST_RSC_PATH = "mdm/mdm-link-update/";
+	public static final String Patient_A_JSON_PATH = TEST_RSC_PATH + "patient-A.json/";
+	public static final String Patient_B_JSON_PATH = TEST_RSC_PATH + "patient-B.json/";
+	public static final String Patient_C_JSON_PATH = TEST_RSC_PATH + "patient-C.json/";
+
+	@Autowired
+	private IMdmLinkUpdaterSvc myMdmLinkUpdaterSvc;
+
+	@Autowired
+	private MdmResourceDaoSvc myMdmResourceDaoSvc;
+
+	@Autowired
+	private MessageHelper myMessageHelper;
+
+
+
+	@Test
+	void testUpdateLinkToMatchWhenAnotherLinkToDifferentGoldenExistsMustFail() throws Exception {
+		// create Patient A -> MATCH GR A
+		Patient patientA = createPatientFromJsonInputFile(Patient_A_JSON_PATH);
+		// create Patient B -> MATCH GR B
+		Patient patientB = createPatientFromJsonInputFile(Patient_B_JSON_PATH);
+
+		Patient goldenA = getGoldenFor(patientA);
+		Patient goldenB = getGoldenFor(patientB);
+
+		// create Patient C -> no MATCH link. Only POSSIBLE_MATCH GR A and POSSIBLE_MATCH GR B and
+		Patient patientC = createPatientFromJsonInputFileWithPossibleMatches( List.of(goldenA, goldenB) );
+
+		MdmTransactionContext mdmTransactionContext = getPatientUpdateLinkContext();
+		// update POSSIBLE_MATCH Patient C -> GR A to MATCH (should work OK)
+		myMdmLinkUpdaterSvc.updateLink(goldenA, patientC, MdmMatchResultEnum.MATCH, mdmTransactionContext);
+
+		// update POSSIBLE_MATCH Patient C -> GR B to MATCH (should throw exception)
+		InvalidRequestException thrown = assertThrows(InvalidRequestException.class,
+			() -> myMdmLinkUpdaterSvc.updateLink(goldenB, patientC, MdmMatchResultEnum.MATCH, mdmTransactionContext));
+
+		String expectedExceptionMessage = Msg.code(2218) + myMessageHelper.getMessageForAlreadyAcceptedLink(goldenA, patientC);
+		assertEquals(expectedExceptionMessage, thrown.getMessage());
+	}
+
+
+
+	private Patient createPatientFromJsonInputFileWithPossibleMatches(List<Patient> theGoldens) throws Exception {
+		Patient patient = createPatientFromJsonInputFile(Patient_C_JSON_PATH, false);
+		for (Patient golden : theGoldens) {
+			myMdmLinkDaoSvc.createOrUpdateLinkEntity(golden, patient, MdmMatchOutcome.POSSIBLE_MATCH, MdmLinkSourceEnum.AUTO, new MdmTransactionContext());
+		}
+		return patient;
+	}
+
+
+	private MdmTransactionContext getPatientUpdateLinkContext() {
+		MdmTransactionContext ctx = new MdmTransactionContext();
+		ctx.setRestOperation(MdmTransactionContext.OperationType.UPDATE_LINK);
+		ctx.setResourceType("Patient");
+		return ctx;
+	}
+
+	private Patient getGoldenFor(Patient thePatient) {
+		Optional<? extends IMdmLink> patientALink = myMdmLinkDaoSvc.findMdmLinkBySource(thePatient);
+		assertTrue(patientALink.isPresent());
+		Patient golden = (Patient) myMdmResourceDaoSvc.readGoldenResourceByPid(patientALink.get().getGoldenResourcePersistenceId(), "Patient");
+		assertNotNull(golden);
+		return golden;
+	}
+
+
+	private Patient createPatientFromJsonInputFile(String thePath)  throws Exception {
+		return createPatientFromJsonInputFile(thePath, true);
+	}
+
+	private Patient createPatientFromJsonInputFile(String thePath, boolean theCreateGolden)  throws Exception {
+		File jsonInputUrl = ResourceUtils.getFile(ResourceUtils.CLASSPATH_URL_PREFIX + thePath);
+		String jsonPatient = Files.readString(Paths.get(jsonInputUrl.toURI()), StandardCharsets.UTF_8);
+
+		Patient patient = (Patient) myFhirContext.newJsonParser().parseResource(jsonPatient);
+		DaoMethodOutcome daoOutcome = myPatientDao.create(patient, new SystemRequestDetails());
+
+		if (theCreateGolden) {
+			myMdmMatchLinkSvc.updateMdmLinksForMdmSource(patient, createContextForCreate("Patient"));
+		}
+
+		return (Patient) daoOutcome.getResource();
+	}
+}

hapi-fhir-jpaserver-mdm/src/test/resources/mdm/mdm-link-update/patient-A.json

@@ -0,0 +1,33 @@
+{
+	"resourceType": "Patient",
+	"meta": {
+		"tag": [{
+			"system" : "http://acme.org/codes",
+			"code" : "tagA02"
+		}]
+	},
+	"identifier": [ {
+		"system": "urn:oid:1.2.36.146.595.217.0.1",
+		"value": "Pabc"
+	} ],
+	"name": [ {
+		"family": "Conde",
+		"given": [ "Carlos" ]
+	} ],
+	"gender": "male",
+	"birthDate": "1974-01-01",
+	"address": [ {
+		"line": [ "534 Erewhon St" ],
+		"city": "PleasantVille",
+		"state": "Vic",
+		"postalCode": "3999"
+	} ],
+	"telecom": [
+		{
+			"system": "phone",
+			"value": "(416) 555 1234",
+			"use": "home",
+			"rank": 1
+		}
+	]
+}

hapi-fhir-jpaserver-mdm/src/test/resources/mdm/mdm-link-update/patient-B.json

@@ -0,0 +1,33 @@
+{
+	"resourceType": "Patient",
+	"meta": {
+		"tag": [{
+			"system" : "http://acme.org/codes",
+			"code" : "tagA02"
+		}]
+	},
+	"identifier": [ {
+		"system": "urn:oid:1.2.36.146.595.217.0.1",
+		"value": "Pabc"
+	} ],
+	"name": [ {
+		"family": "Conde",
+		"given": [ "Carlos" ]
+	} ],
+	"gender": "male",
+	"birthDate": "1974-02-10",
+	"address": [ {
+		"line": [ "534 Erewhon St" ],
+		"city": "PleasantVille",
+		"state": "Vic",
+		"postalCode": "3999"
+	} ],
+	"telecom": [
+		{
+			"system": "phone",
+			"value": "(416) 666 5678",
+			"use": "home",
+			"rank": 1
+		}
+	]
+}

hapi-fhir-jpaserver-mdm/src/test/resources/mdm/mdm-link-update/patient-C.json

@@ -0,0 +1,33 @@
+{
+	"resourceType": "Patient",
+	"meta": {
+		"tag": [{
+			"system" : "http://acme.org/codes",
+			"code" : "tagA02"
+		}]
+	},
+	"identifier": [ {
+		"system": "urn:oid:1.2.36.146.595.217.0.1",
+		"value": "Pabc"
+	} ],
+	"name": [ {
+		"family": "Conde",
+		"given": [ "Carlos" ]
+	} ],
+	"gender": "male",
+	"birthDate": "1974-02-10",
+	"address": [ {
+		"line": [ "534 Erewhon St" ],
+		"city": "PleasantVille",
+		"state": "Vic",
+		"postalCode": "3999"
+	} ],
+	"telecom": [
+		{
+			"system": "phone",
+			"value": "(416) 555 1234",
+			"use": "home",
+			"rank": 1
+		}
+	]
+}

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

@@ -91,6 +91,15 @@ public class MessageHelper {
 		return "No link exists between " + theGoldenRecord + " and " + theSourceResource;
 	}
 
+	public String getMessageForAlreadyAcceptedLink(IAnyResource theGoldenRecord, IAnyResource theSourceResource) {
+		return getMessageForAlreadyAcceptedLink(theGoldenRecord.getIdElement().toVersionless().toString(),
+			theSourceResource.getIdElement().toVersionless().toString());
+	}
+
+	public String getMessageForAlreadyAcceptedLink(String theGoldenId, String theSourceId) {
+		return "A match with a different golden resource (" + theGoldenId + ") exists for resource " + theSourceId;
+	}
+
 	public String getMessageForPresentLink(IAnyResource theGoldenRecord, IAnyResource theSourceResource) {
 		return getMessageForPresentLink(theGoldenRecord.getIdElement().toVersionless().toString(),
 			theSourceResource.getIdElement().toVersionless().toString());