diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3216-mdm-possible-match-to-match.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3216-mdm-possible-match-to-match.yaml new file mode 100644 index 00000000000..4483c10ab87 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3216-mdm-possible-match-to-match.yaml @@ -0,0 +1,3 @@ +--- +type: fix +title: "MDM was throwing a NullPointerException when upgrading a match from POSSIBLE_MATCH to MATCH. This has been corrected." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java index aedce78f880..1dbcf630c07 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java @@ -627,7 +627,7 @@ public class IdHelperService { } @Nonnull - public Long getPidOrThrowException(IAnyResource theResource) { + public Long getPidOrThrowException(@Nonnull IAnyResource theResource) { Long retVal = (Long) theResource.getUserData(RESOURCE_PID); if (retVal == null) { throw new IllegalStateException( diff --git a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvc.java b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvc.java index e8b5845d205..9c54828efd1 100644 --- a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvc.java +++ b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvc.java @@ -30,7 +30,9 @@ import ca.uhn.fhir.mdm.api.MdmMatchResultEnum; import ca.uhn.fhir.mdm.api.paging.MdmPageRequest; import ca.uhn.fhir.mdm.log.Logs; import ca.uhn.fhir.mdm.model.MdmTransactionContext; +import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Example; @@ -146,6 +148,15 @@ public class MdmLinkDaoSvc { * @return the {@link MdmLink} that contains the Match information for the source. */ public Optional getMatchedLinkForSource(IBaseResource theSourceResource) { + return getMdmLinkWithMatchResult(theSourceResource, MdmMatchResultEnum.MATCH); + } + + public Optional getPossibleMatchedLinkForSource(IBaseResource theSourceResource) { + return getMdmLinkWithMatchResult(theSourceResource, MdmMatchResultEnum.POSSIBLE_MATCH); + } + + @NotNull + private Optional getMdmLinkWithMatchResult(IBaseResource theSourceResource, MdmMatchResultEnum theMatchResult) { Long pid = myIdHelperService.getPidOrNull(theSourceResource); if (pid == null) { return Optional.empty(); @@ -153,7 +164,7 @@ public class MdmLinkDaoSvc { MdmLink exampleLink = myMdmLinkFactory.newMdmLink(); exampleLink.setSourcePid(pid); - exampleLink.setMatchResult(MdmMatchResultEnum.MATCH); + exampleLink.setMatchResult(theMatchResult); Example example = Example.of(exampleLink); return myMdmLinkDao.findOne(example); } @@ -298,4 +309,12 @@ public class MdmLinkDaoSvc { return myMdmLinkFactory.newMdmLink(); } + public Optional getMatchedOrPossibleMatchedLinkForSource(IAnyResource theResource) { + // TODO KHS instead of two queries, just do one query with an OR + Optional retval = getMatchedLinkForSource(theResource); + if (!retval.isPresent()) { + retval = getPossibleMatchedLinkForSource(theResource); + } + return retval; + } } diff --git a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmEidUpdateService.java b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmEidUpdateService.java index 94db6849073..f994685505d 100644 --- a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmEidUpdateService.java +++ b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmEidUpdateService.java @@ -80,7 +80,7 @@ public class MdmEidUpdateService { } else { //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); + linkToNewGoldenResourceAndFlagAsDuplicate(theTargetResource, theMatchedGoldenResourceCandidate.getMatchResult(), updateContext.getExistingGoldenResource(), updateContext.getMatchedGoldenResource(), theMdmTransactionContext); myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(theTargetResource, updateContext.getMatchedGoldenResource(), theMdmTransactionContext); myMdmResourceDaoSvc.upsertGoldenResource(updateContext.getMatchedGoldenResource(), theMdmTransactionContext.getResourceType()); @@ -121,10 +121,10 @@ public class MdmEidUpdateService { myMdmLinkSvc.updateLink(newGoldenResource, theOldGoldenResource, MdmMatchOutcome.POSSIBLE_DUPLICATE, MdmLinkSourceEnum.AUTO, theMdmTransactionContext); } - private void linkToNewGoldenResourceAndFlagAsDuplicate(IAnyResource theResource, IAnyResource theOldGoldenResource, IAnyResource theNewGoldenResource, MdmTransactionContext theMdmTransactionContext) { + private void linkToNewGoldenResourceAndFlagAsDuplicate(IAnyResource theResource, MdmMatchOutcome theMatchResult, IAnyResource theOldGoldenResource, IAnyResource theNewGoldenResource, MdmTransactionContext theMdmTransactionContext) { log(theMdmTransactionContext, "Changing a match link!"); myMdmLinkSvc.deleteLink(theOldGoldenResource, theResource, theMdmTransactionContext); - myMdmLinkSvc.updateLink(theNewGoldenResource, theResource, MdmMatchOutcome.NEW_GOLDEN_RESOURCE_MATCH, MdmLinkSourceEnum.AUTO, theMdmTransactionContext); + myMdmLinkSvc.updateLink(theNewGoldenResource, theResource, theMatchResult, MdmLinkSourceEnum.AUTO, theMdmTransactionContext); log(theMdmTransactionContext, "Duplicate detected based on the fact that both resources have different external EIDs."); myMdmLinkSvc.updateLink(theNewGoldenResource, theOldGoldenResource, MdmMatchOutcome.POSSIBLE_DUPLICATE, MdmLinkSourceEnum.AUTO, theMdmTransactionContext); } @@ -161,11 +161,11 @@ public class MdmEidUpdateService { myHasEidsInCommon = myEIDHelper.hasEidOverlap(myMatchedGoldenResource, theResource); myIncomingResourceHasAnEid = !myEIDHelper.getExternalEid(theResource).isEmpty(); - Optional theExistingMatchLink = myMdmLinkDaoSvc.getMatchedLinkForSource(theResource); + Optional theExistingMatchOrPossibleMatchLink = myMdmLinkDaoSvc.getMatchedOrPossibleMatchedLinkForSource(theResource); myExistingGoldenResource = null; - if (theExistingMatchLink.isPresent()) { - MdmLink mdmLink = theExistingMatchLink.get(); + if (theExistingMatchOrPossibleMatchLink.isPresent()) { + MdmLink mdmLink = theExistingMatchOrPossibleMatchLink.get(); Long existingGoldenResourcePid = mdmLink.getGoldenResourcePid(); myExistingGoldenResource = myMdmResourceDaoSvc.readGoldenResourceByPid(new ResourcePersistentId(existingGoldenResourcePid), resourceType); myRemainsMatchedToSameGoldenResource = candidateIsSameAsMdmLinkGoldenResource(mdmLink, theMatchedGoldenResourceCandidate); diff --git a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkSvcImpl.java b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkSvcImpl.java index 7e4b95dad0c..9af1eea89ef 100644 --- a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkSvcImpl.java +++ b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkSvcImpl.java @@ -36,6 +36,7 @@ import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import javax.annotation.Nonnull; import javax.transaction.Transactional; import java.util.Optional; @@ -85,6 +86,9 @@ public class MdmLinkSvcImpl implements IMdmLinkSvc { @Override public void deleteLink(IAnyResource theGoldenResource, IAnyResource theSourceResource, MdmTransactionContext theMdmTransactionContext) { + if (theGoldenResource == null) { + return; + } Optional optionalMdmLink = getMdmLinkForGoldenResourceSourceResourcePair(theGoldenResource, theSourceResource); if (optionalMdmLink.isPresent()) { MdmLink mdmLink = optionalMdmLink.get(); @@ -122,7 +126,7 @@ public class MdmLinkSvcImpl implements IMdmLinkSvc { return theIncomingSource == MdmLinkSourceEnum.AUTO && theExistingSource.isManual(); } - private Optional getMdmLinkForGoldenResourceSourceResourcePair(IAnyResource theGoldenResource, IAnyResource theCandidate) { + private Optional getMdmLinkForGoldenResourceSourceResourcePair(@Nonnull IAnyResource theGoldenResource, @Nonnull IAnyResource theCandidate) { if (theGoldenResource.getIdElement().getIdPart() == null || theCandidate.getIdElement().getIdPart() == null) { return Optional.empty(); } else { diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmMatchLinkSvcTest.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmMatchLinkSvcTest.java index e014e5b64ae..1a3eec54625 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmMatchLinkSvcTest.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmMatchLinkSvcTest.java @@ -435,6 +435,33 @@ public class MdmMatchLinkSvcTest extends BaseMdmR4Test { assertThat(patient3, is(sameGoldenResourceAs(patient))); } + + @Test + public void testPossibleMatchUpdatedToMatch() { + // setup + Patient patient = buildJanePatient(); + patient.getNameFirstRep().setFamily("familyone"); + patient = createPatientAndUpdateLinks(patient); + assertThat(patient, is(sameGoldenResourceAs(patient))); + + Patient patient2 = buildJanePatient(); + patient2.getNameFirstRep().setFamily("pleasedonotmatchatall"); + patient2 = createPatientAndUpdateLinks(patient2); + + assertThat(patient2, is(not(sameGoldenResourceAs(patient)))); + assertThat(patient2, is(not(linkedTo(patient)))); + assertThat(patient2, is(possibleMatchWith(patient))); + + patient2.getNameFirstRep().setFamily(patient.getNameFirstRep().getFamily()); + + // execute + updatePatientAndUpdateLinks(patient2); + + // validate + assertThat(patient2, is(linkedTo(patient))); + assertThat(patient2, is(sameGoldenResourceAs(patient))); + } + @Test public void testCreateGoldenResourceFromMdmTarget() { // Create Use Case #2 - adding patient with no EID