From 01b1074808a7ae8a218cef88892676a4b37390b7 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Wed, 14 Dec 2022 11:47:44 -0500 Subject: [PATCH] Fix MDM update link on resources with an old version. (#4362) * Add a failing unit test if we update a link after the MDM version had been bumped. * Fix bug with new MdmLinkFactory method: newMdmLinkVersionless(). Call this new method from MdmLinkDaoSvc.getLinkByGoldenResourceAndSourceResource() only. Think about other use cases for versionless MDM links. * Replace all read uses of MdmLinkFactory with newMdmLinkVersionless(). Add detailed documentation about why to do this. * Add changelog. Reverse wildcard change. --- ...ls-for-resources-previous-mdm-version.yaml | 5 +++ .../uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvc.java | 18 +++++------ .../ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java | 4 +-- .../mdm/svc/MdmLinkUpdaterSvcImplTest.java | 31 +++++++++++++++++++ .../ca/uhn/fhir/mdm/dao/MdmLinkFactory.java | 22 ++++++++++++- 5 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4364-mdm-update-link-fails-for-resources-previous-mdm-version.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4364-mdm-update-link-fails-for-resources-previous-mdm-version.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4364-mdm-update-link-fails-for-resources-previous-mdm-version.yaml new file mode 100644 index 00000000000..07efb90002a --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4364-mdm-update-link-fails-for-resources-previous-mdm-version.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 4364 +jira: HAPI-0738 +title: "Running MDM update link will fail if resources were created with a previous MDM rules version. This is now fixed with a query that retrieves and updates MDM links regardless of the MDM version of the resource links." 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 a8adb68293f..edc025faf4d 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 @@ -133,7 +133,7 @@ public class MdmLinkDaoSvc

getMdmLinksBySourcePidAndMatchResult(P theSourcePid, MdmMatchResultEnum theMatchResult) { - M exampleLink = myMdmLinkFactory.newMdmLink(); + M exampleLink = myMdmLinkFactory.newMdmLinkVersionless(); exampleLink.setSourcePersistenceId(theSourcePid); exampleLink.setMatchResult(theMatchResult); Example example = Example.of(exampleLink); @@ -193,7 +193,7 @@ public class MdmLinkDaoSvc

example = Example.of(exampleLink); @@ -215,7 +215,7 @@ public class MdmLinkDaoSvc

getMdmLinksByGoldenResourcePidSourcePidAndMatchResult(P theGoldenResourcePid, P theSourcePid, MdmMatchResultEnum theMatchResult) { - M exampleLink = myMdmLinkFactory.newMdmLink(); + M exampleLink = myMdmLinkFactory.newMdmLinkVersionless(); exampleLink.setGoldenResourcePersistenceId(theGoldenResourcePid); exampleLink.setSourcePersistenceId(theSourcePid); exampleLink.setMatchResult(theMatchResult); @@ -229,7 +229,7 @@ public class MdmLinkDaoSvc

getPossibleDuplicates() { - M exampleLink = myMdmLinkFactory.newMdmLink(); + M exampleLink = myMdmLinkFactory.newMdmLinkVersionless(); exampleLink.setMatchResult(MdmMatchResultEnum.POSSIBLE_DUPLICATE); Example example = Example.of(exampleLink); return myMdmLinkDao.findAll(example); @@ -241,7 +241,7 @@ public class MdmLinkDaoSvc

example = Example.of(exampleLink); return myMdmLinkDao.findOne(example); @@ -271,7 +271,7 @@ public class MdmLinkDaoSvc

example = Example.of(exampleLink); return myMdmLinkDao.findAll(example); @@ -321,7 +321,7 @@ public class MdmLinkDaoSvc

example = Example.of(exampleLink); return myMdmLinkDao.findAll(example); @@ -339,7 +339,7 @@ public class MdmLinkDaoSvc

example = Example.of(exampleLink); diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java index 888fccf2f49..4eb72e276be 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java @@ -27,12 +27,12 @@ import ca.uhn.fhir.jpa.searchparam.registry.SearchParamRegistryImpl; import ca.uhn.fhir.jpa.subscription.match.config.SubscriptionProcessorConfig; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.mdm.api.IMdmLink; -import ca.uhn.fhir.mdm.api.IMdmSettings; import ca.uhn.fhir.mdm.api.MdmConstants; import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum; import ca.uhn.fhir.mdm.api.MdmMatchResultEnum; import ca.uhn.fhir.mdm.dao.IMdmLinkDao; import ca.uhn.fhir.mdm.model.MdmTransactionContext; +import ca.uhn.fhir.mdm.rules.config.MdmSettings; import ca.uhn.fhir.mdm.rules.svc.MdmResourceMatcherSvc; import ca.uhn.fhir.mdm.util.EIDHelper; import ca.uhn.fhir.mdm.util.MdmResourceUtil; @@ -119,7 +119,7 @@ abstract public class BaseMdmR4Test extends BaseJpaR4Test { @Autowired protected IIdHelperService myIdHelperService; @Autowired - protected IMdmSettings myMdmSettings; + protected MdmSettings myMdmSettings; @Autowired protected MdmMatchLinkSvc myMdmMatchLinkSvc; @Autowired diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkUpdaterSvcImplTest.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkUpdaterSvcImplTest.java index 8be181e8262..951da376672 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkUpdaterSvcImplTest.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkUpdaterSvcImplTest.java @@ -1,14 +1,21 @@ package ca.uhn.fhir.jpa.mdm.svc; +import ca.uhn.fhir.jpa.entity.MdmLink; import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test; 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.model.MdmTransactionContext; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.util.List; + import static ca.uhn.fhir.mdm.api.MdmMatchResultEnum.MATCH; import static ca.uhn.fhir.mdm.api.MdmMatchResultEnum.NO_MATCH; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; class MdmLinkUpdaterSvcImplTest extends BaseMdmR4Test { @@ -35,4 +42,28 @@ class MdmLinkUpdaterSvcImplTest extends BaseMdmR4Test { assertLinksCreatedNewResource(true, true); assertLinksMatchedByEid(false, false); } + + @Test + public void testUpdateLinkMatchAfterVersionChange() { + myMdmSettings.getMdmRules().setVersion("1"); + + final Patient goldenPatient = createGoldenPatient(buildJanePatient()); + final Patient patient1 = createPatient(buildJanePatient()); + + final MdmTransactionContext mdmCtx = buildUpdateLinkMdmTransactionContext(); + + myMdmLinkDaoSvc.createOrUpdateLinkEntity(goldenPatient, patient1, MdmMatchOutcome.NO_MATCH, MdmLinkSourceEnum.MANUAL, createContextForCreate("Patient")); + + myMdmSettings.getMdmRules().setVersion("2"); + + myMdmLinkUpdaterSvc.updateLink(goldenPatient, patient1, MATCH, mdmCtx); + + final List targets = myMdmLinkDaoSvc.findMdmLinksByGoldenResource(goldenPatient); + assertFalse(targets.isEmpty()); + assertEquals(1, targets.size()); + + final MdmLink mdmLink = targets.get(0); + + assertEquals(patient1.getIdElement().toVersionless().getIdPart(), mdmLink.getSourcePersistenceId().getId().toString()); + } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/dao/MdmLinkFactory.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/dao/MdmLinkFactory.java index 2d23780debe..20194d5346d 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/dao/MdmLinkFactory.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/dao/MdmLinkFactory.java @@ -24,12 +24,21 @@ import ca.uhn.fhir.mdm.api.IMdmLink; import ca.uhn.fhir.mdm.api.IMdmSettings; import org.springframework.beans.factory.annotation.Autowired; +/** + * Creates a new {@link IMdmLink} either with the current {@link IMdmSettings#getRuleVersion()} or with a null version. + *
+ * **Use extreme caution**. The recommended practice is to only use (@link #newMdmLink()} when WRITING an MDM record. + *
+ * Otherwise, there is the risk that code that searches for an MDM record fill fail to locate it due to a version mismatch. + *
+ * Database code makes use of SpringData {@link org.springframework.data.domain.Example} queries. + */ public class MdmLinkFactory { private final IMdmSettings myMdmSettings; private final IMdmLinkImplFactory myMdmLinkImplFactory; @Autowired - public MdmLinkFactory(IMdmSettings theMdmSettings, IMdmLinkImplFactory theMdmLinkImplFactory) { + public MdmLinkFactory(IMdmSettings theMdmSettings, IMdmLinkImplFactory theMdmLinkImplFactory) { myMdmSettings = theMdmSettings; myMdmLinkImplFactory = theMdmLinkImplFactory; } @@ -37,6 +46,8 @@ public class MdmLinkFactory { /** * Create a new {@link IMdmLink}, populating it with the version of the ruleset used to create it. * + * Use this method **only** when writing a new MDM record. + * * @return the new {@link IMdmLink} */ public M newMdmLink() { @@ -44,4 +55,13 @@ public class MdmLinkFactory { retval.setVersion(myMdmSettings.getRuleVersion()); return retval; } + + /** + * Creating a new {@link IMdmLink} with the version deliberately omitted. It will return as null. + * + * This is the recommended use when querying for any MDM records + */ + public M newMdmLinkVersionless() { + return myMdmLinkImplFactory.newMdmLinkImpl(); + } }