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.
This commit is contained in:
Luke deGruchy 2022-12-14 11:47:44 -05:00 committed by GitHub
parent c03129f545
commit 01b1074808
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 68 additions and 12 deletions

View File

@ -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."

View File

@ -133,7 +133,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
if (theSourceResourcePid == null || theGoldenResourcePid == null) {
return Optional.empty();
}
M link = myMdmLinkFactory.newMdmLink();
M link = myMdmLinkFactory.newMdmLinkVersionless();
link.setSourcePersistenceId(theSourceResourcePid);
link.setGoldenResourcePersistenceId(theGoldenResourcePid);
@ -151,7 +151,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
* @return a list of {@link IMdmLink} entities matching these criteria.
*/
public List<M> getMdmLinksBySourcePidAndMatchResult(P theSourcePid, MdmMatchResultEnum theMatchResult) {
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setSourcePersistenceId(theSourcePid);
exampleLink.setMatchResult(theMatchResult);
Example<M> example = Example.of(exampleLink);
@ -193,7 +193,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
return Optional.empty();
}
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setSourcePersistenceId(pid);
exampleLink.setMatchResult(theMatchResult);
Example<M> example = Example.of(exampleLink);
@ -215,7 +215,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
public Optional<M> 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<P extends IResourcePersistentId, M extends IMdmLink<P
* @return A list of {@link IMdmLink} that hold potential duplicate golden resources.
*/
public List<M> getPossibleDuplicates() {
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setMatchResult(MdmMatchResultEnum.POSSIBLE_DUPLICATE);
Example<M> example = Example.of(exampleLink);
return myMdmLinkDao.findAll(example);
@ -241,7 +241,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
if (pid == null) {
return Optional.empty();
}
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setSourcePersistenceId(pid);
Example<M> example = Example.of(exampleLink);
return myMdmLinkDao.findOne(example);
@ -271,7 +271,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
if (pid == null) {
return Collections.emptyList();
}
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setGoldenResourcePersistenceId(pid);
Example<M> example = Example.of(exampleLink);
return myMdmLinkDao.findAll(example);
@ -321,7 +321,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
if (pid == null) {
return Collections.emptyList();
}
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setSourcePersistenceId(pid);
Example<M> example = Example.of(exampleLink);
return myMdmLinkDao.findAll(example);
@ -339,7 +339,7 @@ public class MdmLinkDaoSvc<P extends IResourcePersistentId, M extends IMdmLink<P
if (pid == null) {
return Collections.emptyList();
}
M exampleLink = myMdmLinkFactory.newMdmLink();
M exampleLink = myMdmLinkFactory.newMdmLinkVersionless();
exampleLink.setGoldenResourcePersistenceId(pid);
exampleLink.setMatchResult(MdmMatchResultEnum.MATCH);
Example<M> example = Example.of(exampleLink);

View File

@ -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<JpaPid> myIdHelperService;
@Autowired
protected IMdmSettings myMdmSettings;
protected MdmSettings myMdmSettings;
@Autowired
protected MdmMatchLinkSvc myMdmMatchLinkSvc;
@Autowired

View File

@ -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<MdmLink> 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());
}
}

View File

@ -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.
* <br>
* **Use extreme caution**. The recommended practice is to only use (@link #newMdmLink()} when WRITING an MDM record.
* <br>
* Otherwise, there is the risk that code that searches for an MDM record fill fail to locate it due to a version mismatch.
* <br>
* Database code makes use of SpringData {@link org.springframework.data.domain.Example} queries.
*/
public class MdmLinkFactory<M extends IMdmLink> {
private final IMdmSettings myMdmSettings;
private final IMdmLinkImplFactory<M> myMdmLinkImplFactory;
@Autowired
public MdmLinkFactory(IMdmSettings theMdmSettings, IMdmLinkImplFactory theMdmLinkImplFactory) {
public MdmLinkFactory(IMdmSettings theMdmSettings, IMdmLinkImplFactory<M> theMdmLinkImplFactory) {
myMdmSettings = theMdmSettings;
myMdmLinkImplFactory = theMdmLinkImplFactory;
}
@ -37,6 +46,8 @@ public class MdmLinkFactory<M extends IMdmLink> {
/**
* 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<M extends IMdmLink> {
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();
}
}