Ensure querying $mdm-link-history on only golden resource or resource only does not result in an error (#4681)

* Ensure querying $mdm-link-history on only golden resource or resource ID does not result in an error.

* Change MdmHistorySearchParameters semantics back to set**** so we don't need to a hapi-fhir to cdr bump.

* Remove envers disabling from HibernatePropertiesProvider.getDialect as by then it's too late.

* Update hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4680-mdm-link-history-only-one-of-each-param-no-error.yaml

Co-authored-by: Ken Stevens <khstevens@gmail.com>

---------

Co-authored-by: Ken Stevens <khstevens@gmail.com>
This commit is contained in:
Luke deGruchy 2023-03-28 12:25:59 -04:00 committed by GitHub
parent 479e412786
commit 734bb3cbff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 95 additions and 12 deletions

View File

@ -0,0 +1,4 @@
---
type: fix
issue: 4680
title: "Previously, invoking $mdm-link-history on only the golden resource ID or source ID would fail due to faulty validation. This has been corrected."

View File

@ -57,13 +57,6 @@ public class HibernatePropertiesProvider {
myDialect = dialect;
}
if (myEntityManagerFactory != null) {
final Map<String, Object> jpaPropertyMap = myEntityManagerFactory.getJpaPropertyMap();
if (! jpaPropertyMap.containsKey(Constants.HIBERNATE_INTEGRATION_ENVERS_ENABLED)) {
jpaPropertyMap.put(Constants.HIBERNATE_INTEGRATION_ENVERS_ENABLED, myStorageSettings.isNonResourceDbHistoryEnabled());
}
}
return dialect;
}

View File

@ -45,6 +45,7 @@ import org.hibernate.envers.AuditReader;
import org.hibernate.envers.RevisionType;
import org.hibernate.envers.query.AuditEntity;
import org.hibernate.envers.query.AuditQueryCreator;
import org.hibernate.envers.query.criteria.AuditCriterion;
import org.hl7.fhir.instance.model.api.IIdType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -332,10 +333,24 @@ public class MdmLinkDaoJpaImpl implements IMdmLinkDao<JpaPid, MdmLink> {
final AuditQueryCreator auditQueryCreator = myAuditReader.createQuery();
try {
final AuditCriterion goldenResourceIdCriterion = AuditEntity.property(GOLDEN_RESOURCE_PID_NAME).in(convertToLongIds(theMdmHistorySearchParameters.getGoldenResourceIds()));
final AuditCriterion resourceIdCriterion = AuditEntity.property(SOURCE_PID_NAME).in(convertToLongIds(theMdmHistorySearchParameters.getSourceIds()));
final AuditCriterion goldenResourceAndOrResourceIdCriterion;
if (! theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty() && ! theMdmHistorySearchParameters.getSourceIds().isEmpty()) {
goldenResourceAndOrResourceIdCriterion = AuditEntity.or(goldenResourceIdCriterion, resourceIdCriterion);
} else if (! theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty()) {
goldenResourceAndOrResourceIdCriterion = goldenResourceIdCriterion;
} else if (! theMdmHistorySearchParameters.getSourceIds().isEmpty()) {
goldenResourceAndOrResourceIdCriterion = resourceIdCriterion;
} else {
throw new IllegalArgumentException(Msg.code(2298) + "$mdm-link-history Golden resource and source query IDs cannot both be empty.");
}
@SuppressWarnings("unchecked")
final List<Object[]> mdmLinksWithRevisions = auditQueryCreator.forRevisionsOfEntity(MdmLink.class, false, false)
.add(AuditEntity.or(AuditEntity.property(GOLDEN_RESOURCE_PID_NAME).in(convertToLongIds(theMdmHistorySearchParameters.getGoldenResourceIds())),
AuditEntity.property(SOURCE_PID_NAME).in(convertToLongIds(theMdmHistorySearchParameters.getSourceIds()))))
.add(goldenResourceAndOrResourceIdCriterion)
.addOrder(AuditEntity.property(GOLDEN_RESOURCE_PID_NAME).asc())
.addOrder(AuditEntity.property(SOURCE_PID_NAME).asc())
.addOrder(AuditEntity.revisionNumber().desc())

View File

@ -38,6 +38,7 @@ import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class MdmLinkDaoSvcTest extends BaseMdmR4Test {
@ -144,6 +145,75 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test {
assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
}
@Test
public void testHistoryForGoldenResourceIdsOnly() {
final List<MdmLink> mdmLinksWithLinkedPatients1 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 3);
createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 4);
final List<MdmLink> mdmLinksWithLinkedPatients3 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 2);
flipLinksTo(mdmLinksWithLinkedPatients3, MdmMatchResultEnum.NO_MATCH);
final MdmHistorySearchParameters mdmHistorySearchParameters =
new MdmHistorySearchParameters()
.setGoldenResourceIds(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients3.get(0)));
final List<MdmLinkWithRevision<MdmLink>> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters);
final JpaPid goldenResourceId1 = mdmLinksWithLinkedPatients1.get(0).getGoldenResourcePersistenceId();
final JpaPid goldenResourceId3 = mdmLinksWithLinkedPatients3.get(0).getGoldenResourcePersistenceId();
final JpaPid sourceId1_1 = mdmLinksWithLinkedPatients1.get(0).getSourcePersistenceId();
final JpaPid sourceId1_2 = mdmLinksWithLinkedPatients1.get(1).getSourcePersistenceId();
final JpaPid sourceId1_3 = mdmLinksWithLinkedPatients1.get(2).getSourcePersistenceId();
final JpaPid sourceId3_1 = mdmLinksWithLinkedPatients3.get(0).getSourcePersistenceId();
final JpaPid sourceId3_2 = mdmLinksWithLinkedPatients3.get(1).getSourcePersistenceId();
final List<MdmLinkWithRevision<MdmLink>> expectedMdLinkRevisions = List.of(
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(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)
);
assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
}
@Test
public void testHistoryForSourceIdsOnly() {
final List<MdmLink> mdmLinksWithLinkedPatients1 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 3);
final List<MdmLink> mdmLinksWithLinkedPatients2 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 4);
final List<MdmLink> mdmLinksWithLinkedPatients3 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 2);
flipLinksTo(mdmLinksWithLinkedPatients3, MdmMatchResultEnum.NO_MATCH);
final MdmHistorySearchParameters mdmHistorySearchParameters =
new MdmHistorySearchParameters()
.setSourceIds(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
final List<MdmLinkWithRevision<MdmLink>> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters);
final JpaPid goldenResourceId1 = mdmLinksWithLinkedPatients1.get(0).getGoldenResourcePersistenceId();
final JpaPid goldenResourceId2 = mdmLinksWithLinkedPatients2.get(0).getGoldenResourcePersistenceId();
final JpaPid sourceId1_1 = mdmLinksWithLinkedPatients1.get(0).getSourcePersistenceId();
final JpaPid sourceId2_1 = mdmLinksWithLinkedPatients2.get(0).getSourcePersistenceId();
final List<MdmLinkWithRevision<MdmLink>> expectedMdLinkRevisions = List.of(
buildMdmLinkWithRevision(1, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_1),
buildMdmLinkWithRevision(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1)
);
assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
}
@Test
public void testHistoryForNoIdsOnly() {
assertThrows(IllegalArgumentException.class, () -> myMdmLinkDaoSvc.findMdmLinkHistory(new MdmHistorySearchParameters()));
}
@Nonnull
private static List<String> getIdsFromMdmLinks(Function<MdmLink, JpaPid> getIdFunction, MdmLink... mdmLinks) {
return Arrays.stream(mdmLinks)

View File

@ -27,13 +27,14 @@ import org.hl7.fhir.instance.model.api.IIdType;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
public class MdmHistorySearchParameters {
private List<IIdType> myGoldenResourceIds;
private List<IIdType> mySourceIds;
private List<IIdType> myGoldenResourceIds = new ArrayList<>();
private List<IIdType> mySourceIds = new ArrayList<>();
public MdmHistorySearchParameters() {}

View File

@ -73,7 +73,7 @@ public abstract class BaseMdmProvider {
}
private void validateBothCannotBeNullOrEmpty(String theFirstName, List<IPrimitiveType<String>> theFirstList, String theSecondName, List<IPrimitiveType<String>> theSecondList) {
if ((theFirstList == null || theFirstList.isEmpty()) || (theSecondList == null || theSecondList.isEmpty())) {
if ((theFirstList == null || theFirstList.isEmpty()) && (theSecondList == null || theSecondList.isEmpty())) {
throw new InvalidRequestException(Msg.code(2292) + "both ["+theFirstName+"] and ["+theSecondName+"] cannot be null or empty");
}
}