From 8d4f07c56e550464f1e47860d930786c3aa494dd Mon Sep 17 00:00:00 2001 From: TynerGjs <132295567+TynerGjs@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:15:47 -0400 Subject: [PATCH] Resolve 5262 adding new parameter strictmatch to the mdm history link operation 1 (#5282) * implemented the strictMatch parameter, added corresponding test * implemented the strictMatch parameter, added corresponding test * updated mdm link history documentation created changelog * reverted previous changes. Made the default behaviour of history link as AND instead of OR * optimized imports * modified changelog * modified tests --- ...tch-to-the-mdm-history-link-operation.yaml | 5 ++ .../fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java | 11 +--- .../fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java | 59 +++++++++++++++++-- .../params/MdmHistorySearchParameters.java | 24 -------- .../MdmLinkHistoryProviderDstu3Plus.java | 7 ++- 5 files changed, 65 insertions(+), 41 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5262-adding-new-parameter-strictmatch-to-the-mdm-history-link-operation.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5262-adding-new-parameter-strictmatch-to-the-mdm-history-link-operation.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5262-adding-new-parameter-strictmatch-to-the-mdm-history-link-operation.yaml new file mode 100644 index 00000000000..e727ec08d83 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5262-adding-new-parameter-strictmatch-to-the-mdm-history-link-operation.yaml @@ -0,0 +1,5 @@ +--- +type: change +issue: 5262 +title: "Previously, when both resourceId and goldenResourceId are provided to the mdm link history operation, they will +be treated as an OR. This is now changed to AND in order to comply with REST conventions." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java index 013c06efee5..6e87d443f1b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java @@ -380,15 +380,8 @@ public class MdmLinkDaoJpaImpl implements IMdmLinkDao { if (!theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty() && !theMdmHistorySearchParameters.getSourceIds().isEmpty()) { - if (theMdmHistorySearchParameters.getParameterJoinType() == MdmHistorySearchParameters.JoinType.AND) { - // 'and' the source and golden ids - goldenResourceAndOrResourceIdCriterion = - AuditEntity.and(goldenResourceIdCriterion, resourceIdCriterion); - } else { - // default is 'or' - goldenResourceAndOrResourceIdCriterion = - AuditEntity.or(goldenResourceIdCriterion, resourceIdCriterion); - } + goldenResourceAndOrResourceIdCriterion = + AuditEntity.and(goldenResourceIdCriterion, resourceIdCriterion); } else if (!theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty()) { goldenResourceAndOrResourceIdCriterion = goldenResourceIdCriterion; } else if (!theMdmHistorySearchParameters.getSourceIds().isEmpty()) { diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java index 27abb67ece0..3078a06e0c0 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.mdm.api.MdmLinkWithRevision; import ca.uhn.fhir.mdm.api.MdmMatchResultEnum; import ca.uhn.fhir.mdm.model.MdmPidTuple; import ca.uhn.fhir.mdm.rules.json.MdmRulesJson; +import ca.uhn.fhir.model.primitive.BooleanDt; import org.hibernate.envers.RevisionType; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.Enumerations; @@ -26,7 +27,10 @@ import javax.annotation.Nonnull; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -114,12 +118,19 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test { final List mdmLinksWithLinkedPatients3 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 2); flipLinksTo(mdmLinksWithLinkedPatients3, MdmMatchResultEnum.NO_MATCH); - final MdmHistorySearchParameters mdmHistorySearchParameters = + final MdmHistorySearchParameters mdmHistorySearchParametersResourceIds = + new MdmHistorySearchParameters() + .setGoldenResourceIds(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients3.get(0))); + + final List> actualMdmLinkRevisionsResourceIds = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParametersResourceIds); + + final MdmHistorySearchParameters mdmHistorySearchParametersGoldenResourceIds = new MdmHistorySearchParameters() - .setGoldenResourceIds(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients3.get(0))) .setSourceIds(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0))); - final List> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters); + final List> actualMdmLinkRevisionsGoldenResourceIds = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParametersGoldenResourceIds); + + final List> actualMdmLinkRevisionsJoined = joinAndHandleRepetitiveLinks(actualMdmLinkRevisionsResourceIds, actualMdmLinkRevisionsGoldenResourceIds); final JpaPid goldenResourceId1 = mdmLinksWithLinkedPatients1.get(0).getGoldenResourcePersistenceId(); final JpaPid goldenResourceId2 = mdmLinksWithLinkedPatients2.get(0).getGoldenResourcePersistenceId(); @@ -138,14 +149,14 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test { buildMdmLinkWithRevision(1, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_1), buildMdmLinkWithRevision(2, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_2), buildMdmLinkWithRevision(3, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_3), - buildMdmLinkWithRevision(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1), buildMdmLinkWithRevision(10, RevisionType.MOD, MdmMatchResultEnum.NO_MATCH, goldenResourceId3, sourceId3_1), buildMdmLinkWithRevision(8, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId3, sourceId3_1), buildMdmLinkWithRevision(11, RevisionType.MOD, MdmMatchResultEnum.NO_MATCH, goldenResourceId3, sourceId3_2), - buildMdmLinkWithRevision(9, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId3, sourceId3_2) + buildMdmLinkWithRevision(9, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId3, sourceId3_2), + buildMdmLinkWithRevision(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1) ); - assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions); + assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisionsJoined); } @Test @@ -212,6 +223,29 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test { assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions); } + @Test + public void testHistoryForBothSourceAndGoldenResourceIds(){ + // setup + MdmLink targetMdmLink = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 2).get(0); + + // link both patient to the GR + String goldenPatientId = targetMdmLink.getGoldenResourcePersistenceId().getId().toString(); + String sourcePatientId = targetMdmLink.getSourcePersistenceId().getId().toString(); + + // execute + MdmHistorySearchParameters mdmHistorySearchParameters = new MdmHistorySearchParameters() + .setSourceIds(List.of(sourcePatientId)) + .setGoldenResourceIds(List.of(goldenPatientId)); + + List> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters); + + // verify + assertEquals(1, actualMdmLinkRevisions.size()); + MdmLink actualMdmLink = actualMdmLinkRevisions.get(0).getMdmLink(); + assertEquals(goldenPatientId, actualMdmLink.getGoldenResourcePersistenceId().getId().toString()); + assertEquals(sourcePatientId, actualMdmLink.getSourcePersistenceId().getId().toString()); + } + @Test public void testHistoryForNoIdsOnly() { assertThrows(IllegalArgumentException.class, () -> myMdmLinkDaoSvc.findMdmLinkHistory(new MdmHistorySearchParameters())); @@ -313,4 +347,17 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test { mdmLink.setSourcePersistenceId(runInTransaction(() -> myIdHelperService.getPidOrNull(RequestPartitionId.allPartitions(), theTargetResource))); return myMdmLinkDao.save(mdmLink); } + + private List> joinAndHandleRepetitiveLinks(List> toLinks, List> fromLinks){ + List> joinedLinks = new ArrayList<>(toLinks); + Set joinedLinkIds = new HashSet<>(); + toLinks.forEach(link -> joinedLinkIds.add(link.getMdmLink().getId().toString())); + for (MdmLinkWithRevision link : fromLinks){ + if (!joinedLinkIds.contains(link.getMdmLink().getId().toString())){ + joinedLinks.add(link); + } + } + + return joinedLinks; + } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/params/MdmHistorySearchParameters.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/params/MdmHistorySearchParameters.java index 43f4f59113e..c16c3ecdeea 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/params/MdmHistorySearchParameters.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/params/MdmHistorySearchParameters.java @@ -33,24 +33,9 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; public class MdmHistorySearchParameters { - - public enum JoinType { - AND, - OR - } - private List myGoldenResourceIds = new ArrayList<>(); private List mySourceIds = new ArrayList<>(); - /** - * When both myGoldenResourceIds and mySourceIds are provided, - * this parameter determines whether to 'and' or 'or' the query - * that fetches links by these values. - * - * Default (legacy) functionality is 'or'. - */ - private JoinType myParameterJoinType = JoinType.OR; - public MdmHistorySearchParameters() {} public List getGoldenResourceIds() { @@ -61,10 +46,6 @@ public class MdmHistorySearchParameters { return mySourceIds; } - public JoinType getParameterJoinType() { - return myParameterJoinType; - } - public MdmHistorySearchParameters setGoldenResourceIds(List theGoldenResourceIds) { myGoldenResourceIds = extractId(theGoldenResourceIds); return this; @@ -75,11 +56,6 @@ public class MdmHistorySearchParameters { return this; } - public MdmHistorySearchParameters setJoinType(JoinType theJoinType) { - myParameterJoinType = theJoinType; - return this; - } - @Override public boolean equals(Object theO) { if (this == theO) return true; diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmLinkHistoryProviderDstu3Plus.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmLinkHistoryProviderDstu3Plus.java index 6670e1334a5..dd7ae09c710 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmLinkHistoryProviderDstu3Plus.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmLinkHistoryProviderDstu3Plus.java @@ -28,6 +28,7 @@ import ca.uhn.fhir.mdm.api.IMdmControllerSvc; import ca.uhn.fhir.mdm.api.params.MdmHistorySearchParameters; import ca.uhn.fhir.mdm.model.mdmevents.MdmHistoryEvent; import ca.uhn.fhir.mdm.model.mdmevents.MdmLinkWithRevisionJson; +import ca.uhn.fhir.model.api.annotation.Description; import ca.uhn.fhir.rest.annotation.Operation; import ca.uhn.fhir.rest.annotation.OperationParam; import ca.uhn.fhir.rest.api.server.RequestDetails; @@ -62,13 +63,15 @@ public class MdmLinkHistoryProviderDstu3Plus extends BaseMdmProvider { @Operation(name = ProviderConstants.MDM_LINK_HISTORY, idempotent = true) public IBaseParameters historyLinks( - @OperationParam( + @Description(value = "The id of the Golden Resource (e.g. Golden Patient Resource).") + @OperationParam( name = ProviderConstants.MDM_QUERY_LINKS_GOLDEN_RESOURCE_ID, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List> theMdmGoldenResourceIds, - @OperationParam( + @Description(value = "The id of the source resource (e.g. Patient resource).") + @OperationParam( name = ProviderConstants.MDM_QUERY_LINKS_RESOURCE_ID, min = 0, max = OperationParam.MAX_UNLIMITED,