Resolve 5336 Mdm Link History Would Fail If Provided with Unknown Ids On Postgres (#5337)

* - Added checks in MdmLinkDaoJpaImpl to make sure no empty IN clauses are sent to database.
- Implemented tests for the above change.

* modified changelog

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5336-mdm-link-history-would-fail-if-provided-with-unknown-ids-on-postgres.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 5336
+jira: SMILE-7406
+title: "Previously, on PostgreSQL, the $mdm-link-history operation would fail if all ids provided to a parameter are 
+unknown, and the error will persist for all subsequent requests. This is now fixed."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkDaoJpaImpl.java

@@ -40,6 +40,7 @@ import ca.uhn.fhir.rest.api.SortOrderEnum;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.ListUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.Validate;
 import org.hibernate.envers.AuditReader;
 import org.hibernate.envers.RevisionType;
@@ -371,21 +372,39 @@ public class MdmLinkDaoJpaImpl implements IMdmLinkDao<JpaPid, MdmLink> {
 		final AuditQueryCreator auditQueryCreator = myAuditReader.createQuery();
 
 		try {
-			final AuditCriterion goldenResourceIdCriterion = AuditEntity.property(GOLDEN_RESOURCE_PID_NAME)
+			final AuditCriterion goldenResourceIdCriterion = buildAuditCriterionOrNull(
-					.in(convertToLongIds(theMdmHistorySearchParameters.getGoldenResourceIds()));
+					theMdmHistorySearchParameters.getGoldenResourceIds(), GOLDEN_RESOURCE_PID_NAME);
-			final AuditCriterion resourceIdCriterion = AuditEntity.property(SOURCE_PID_NAME)
+
-					.in(convertToLongIds(theMdmHistorySearchParameters.getSourceIds()));
+			final AuditCriterion resourceIdCriterion =
+					buildAuditCriterionOrNull(theMdmHistorySearchParameters.getSourceIds(), SOURCE_PID_NAME);
 
 			final AuditCriterion goldenResourceAndOrResourceIdCriterion;
 
 			if (!theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty()
 					&& !theMdmHistorySearchParameters.getSourceIds().isEmpty()) {
+
+				// Make sure the criterion does not contain empty IN clause, e.g. id IN (), which postgres (likely other
+				// sql servers) do not like. Directly return empty result instead.
+				if (ObjectUtils.anyNull(goldenResourceIdCriterion, resourceIdCriterion)) {
+					return new ArrayList<>();
+				}
 				goldenResourceAndOrResourceIdCriterion =
 						AuditEntity.and(goldenResourceIdCriterion, resourceIdCriterion);
+
 			} else if (!theMdmHistorySearchParameters.getGoldenResourceIds().isEmpty()) {
+
+				if (ObjectUtils.anyNull(goldenResourceIdCriterion)) {
+					return new ArrayList<>();
+				}
 				goldenResourceAndOrResourceIdCriterion = goldenResourceIdCriterion;
+
 			} else if (!theMdmHistorySearchParameters.getSourceIds().isEmpty()) {
+
+				if (ObjectUtils.anyNull(resourceIdCriterion)) {
+					return new ArrayList<>();
+				}
 				goldenResourceAndOrResourceIdCriterion = resourceIdCriterion;
+
 			} else {
 				throw new IllegalArgumentException(Msg.code(2298)
 						+ "$mdm-link-history Golden resource and source query IDs cannot both be empty.");
@@ -420,6 +439,12 @@ public class MdmLinkDaoJpaImpl implements IMdmLinkDao<JpaPid, MdmLink> {
 				.collect(Collectors.toUnmodifiableList());
 	}
 
+	private AuditCriterion buildAuditCriterionOrNull(
+			List<IIdType> theMdmHistorySearchParameterIds, String theProperty) {
+		List<Long> longIds = convertToLongIds(theMdmHistorySearchParameterIds);
+		return longIds.isEmpty() ? null : AuditEntity.property(theProperty).in(longIds);
+	}
+
 	private MdmLinkWithRevision<MdmLink> buildRevisionFromObjectArray(Object[] theArray) {
 		final Object mdmLinkUncast = theArray[0];
 		final Object revisionUncast = theArray[1];

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/dao/MdmLinkDaoSvcTest.java

@@ -20,6 +20,8 @@ import org.hl7.fhir.r4.model.Enumerations;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Practitioner;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -276,6 +278,179 @@ public class MdmLinkDaoSvcTest extends BaseMdmR4Test {
 		assertEquals(2, actualMdmLinkRevisions.size(), "Both Patient/p123 and Practitioner/p123 should be returned");
 	}
 
+	@ParameterizedTest
+	@ValueSource(strings = {"allUnknown", "someUnknown"})
+	public void testHistoryForUnknownIdsSourceIdOnly(String mode) {
+		// setup
+		final List<MdmLink> mdmLinksWithLinkedPatients1 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 3);
+		final List<MdmLink> mdmLinksWithLinkedPatients2 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 4);
+
+		MdmHistorySearchParameters mdmHistorySearchParameters = null;
+		List<MdmLinkWithRevision<MdmLink>> expectedMdLinkRevisions = null;
+		switch (mode) {
+			// $mdm-link-history?resourceId=Patient/unknown
+			case "allUnknown" -> {
+				mdmHistorySearchParameters = new MdmHistorySearchParameters().setSourceIds(List.of("unknown"));
+				expectedMdLinkRevisions = new ArrayList<>();
+			}
+			// $mdm-link-history?resourceId=Patient/1,Patient/2,Patient/unknown
+			case "someUnknown" -> {
+				List<String> resourceIdsWithSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				resourceIdsWithSomeUnknown.add("unknown");
+				mdmHistorySearchParameters = new MdmHistorySearchParameters().setSourceIds(resourceIdsWithSomeUnknown);
+
+				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();
+				expectedMdLinkRevisions = List.of(
+					buildMdmLinkWithRevision(1, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_1),
+					buildMdmLinkWithRevision(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1)
+				);
+			}
+		}
+
+		// execute
+		final List<MdmLinkWithRevision<MdmLink>> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters);
+
+		// verify
+		assert expectedMdLinkRevisions != null;
+		assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {"allUnknown", "someUnknown"})
+	public void testHistoryForUnknownIdsGoldenResourceIdOnly(String mode) {
+		// setup
+		final List<MdmLink> mdmLinksWithLinkedPatients1 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 3);
+		final List<MdmLink> mdmLinksWithLinkedPatients2 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 4);
+
+		MdmHistorySearchParameters mdmHistorySearchParameters = null;
+		List<MdmLinkWithRevision<MdmLink>> expectedMdLinkRevisions = null;
+		switch (mode) {
+			// $mdm-link-history?goldenResourceId=Patient/unknown
+			case "allUnknown" -> {
+				mdmHistorySearchParameters = new MdmHistorySearchParameters().setGoldenResourceIds(List.of("unknown"));
+				expectedMdLinkRevisions = new ArrayList<>();
+			}
+			// $mdm-link-history?goldenResourceId=Patient/1,Patient/2,Patient/unknown
+			case "someUnknown" -> {
+				List<String> resourceIdsWithSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				resourceIdsWithSomeUnknown.add("unknown");
+				mdmHistorySearchParameters = new MdmHistorySearchParameters().setGoldenResourceIds(resourceIdsWithSomeUnknown);
+
+				final JpaPid goldenResourceId1 = mdmLinksWithLinkedPatients1.get(0).getGoldenResourcePersistenceId();
+				final JpaPid goldenResourceId2 = mdmLinksWithLinkedPatients2.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 sourceId2_1 = mdmLinksWithLinkedPatients2.get(0).getSourcePersistenceId();
+				final JpaPid sourceId2_2 = mdmLinksWithLinkedPatients2.get(1).getSourcePersistenceId();
+				final JpaPid sourceId2_3 = mdmLinksWithLinkedPatients2.get(2).getSourcePersistenceId();
+				final JpaPid sourceId2_4 = mdmLinksWithLinkedPatients2.get(3).getSourcePersistenceId();
+				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(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1),
+					buildMdmLinkWithRevision(5, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_2),
+					buildMdmLinkWithRevision(6, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_3),
+					buildMdmLinkWithRevision(7, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_4)
+				);
+			}
+		}
+
+		// execute
+		final List<MdmLinkWithRevision<MdmLink>> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters);
+
+		// verify
+		assert expectedMdLinkRevisions != null;
+		assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {
+		"allUnknownSourceId",
+		"allUnknownGoldenId",
+		"allUnknownBoth",
+		"someUnknownSourceId",
+		"someUnknownGoldenId",
+		"someUnknownBoth"
+	})
+	public void testHistoryForUnknownIdsBothSourceAndGoldenResourceId(String mode) {
+		// setup
+		final List<MdmLink> mdmLinksWithLinkedPatients1 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 3);
+		final List<MdmLink> mdmLinksWithLinkedPatients2 = createMdmLinksWithLinkedPatients(MdmMatchResultEnum.MATCH, 4);
+
+		MdmHistorySearchParameters mdmHistorySearchParameters = null;
+		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();
+		List<MdmLinkWithRevision<MdmLink>> expectedMdLinkRevisions = List.of(
+			buildMdmLinkWithRevision(1, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId1, sourceId1_1),
+			buildMdmLinkWithRevision(4, RevisionType.ADD, MdmMatchResultEnum.MATCH, goldenResourceId2, sourceId2_1)
+		);
+		switch (mode) {
+			// $mdm-link-history?resourceId=Patient/unknown&goldenResourceId=Patient/1,Patient/2
+			case "allUnknownSourceId" -> {
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(List.of("unknown"))
+					.setGoldenResourceIds(new ArrayList<>(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0))));
+				expectedMdLinkRevisions = new ArrayList<>();
+			}
+			// $mdm-link-history?resourceId=Patient/1,Patient/2&goldenResourceId=Patient/unknown
+			case "allUnknownGoldenId" -> {
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(new ArrayList<>(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0))))
+					.setGoldenResourceIds(List.of("unknown"));
+				expectedMdLinkRevisions = new ArrayList<>();
+			}
+			// $mdm-link-history?resourceId=Patient/unknown&goldenResourceId=Patient/unknownGolden
+			case "allUnknownBoth" -> {
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(List.of("unknown"))
+					.setGoldenResourceIds(List.of("unknownGolden"));
+				expectedMdLinkRevisions = new ArrayList<>();
+			}
+			// $mdm-link-history?resourceId=Patient/1,Patient/2,Patient/unknown&goldenResourceId=Patient/3,Patient/4
+			case "someUnknownSourceId" -> {
+				List<String> sourceIdsWithSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				sourceIdsWithSomeUnknown.add("unknown");
+				List<String> goldenResourceIds = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(sourceIdsWithSomeUnknown)
+					.setGoldenResourceIds(goldenResourceIds);
+			}
+			// $mdm-link-history?resourceId=Patient/1,Patient/2&goldenResourceId=Patient/3,Patient/4,Patient/unknown
+			case "someUnknownGoldenId" -> {
+				List<String> sourceIds = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				List<String> goldenResourceIdsSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				goldenResourceIdsSomeUnknown.add("unknown");
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(sourceIds)
+					.setGoldenResourceIds(goldenResourceIdsSomeUnknown);
+			}
+			// $mdm-link-history?resourceId=Patient/1,Patient/2,Patient/unknown&goldenResourceId=Patient/3,Patient/4,Patient/unknownGolden
+			case "someUnknownBoth" -> {
+				List<String> sourceIdsSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getSourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				sourceIdsSomeUnknown.add("unknown");
+				List<String> goldenResourceIdsSomeUnknown = new ArrayList<>(getIdsFromMdmLinks(MdmLink::getGoldenResourcePersistenceId, mdmLinksWithLinkedPatients1.get(0), mdmLinksWithLinkedPatients2.get(0)));
+				goldenResourceIdsSomeUnknown.add("unknownGolden");
+				mdmHistorySearchParameters = new MdmHistorySearchParameters()
+					.setSourceIds(sourceIdsSomeUnknown)
+					.setGoldenResourceIds(goldenResourceIdsSomeUnknown);
+			}
+		}
+
+		// execute
+		final List<MdmLinkWithRevision<MdmLink>> actualMdmLinkRevisions = myMdmLinkDaoSvc.findMdmLinkHistory(mdmHistorySearchParameters);
+
+		// verify
+		assert expectedMdLinkRevisions != null;
+		assertMdmRevisionsEqual(expectedMdLinkRevisions, actualMdmLinkRevisions);
+	}
+
 	@Nonnull
 	private static List<String> getIdsFromMdmLinks(Function<MdmLink, JpaPid> getIdFunction, MdmLink... mdmLinks) {
 		return Arrays.stream(mdmLinks)