Issue 5102 mdm link history ordering is incorrect when client assigned id and pid are different (#5105)

* Failing test

* Add some self-assigned Patient IDs

* Sort after client-selected ID is interpolated into the result

* spotless

* changelog

* Keep DAO level ordering enforced by tests

* Filter out non-client-provider ids from test as they are unpredictable in build pipeline

---------

Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5102-mdm-link-history-ordering-wrong-for-client-assigned-id.yaml

@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 5102
+title: "Consider user-assigned IDs, instead of system assigned IDs, when sorting MDM links history."

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

@@ -411,7 +411,6 @@ public class MdmLinkDaoJpaImpl implements IMdmLinkDao<JpaPid, MdmLink> {
 				.collect(Collectors.toUnmodifiableList());
 	}
 
-	@SuppressWarnings("unchecked")
 	private MdmLinkWithRevision<MdmLink> buildRevisionFromObjectArray(Object[] theArray) {
 		final Object mdmLinkUncast = theArray[0];
 		final Object revisionUncast = theArray[1];

hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkQuerySvcImplSvc.java

@@ -38,6 +38,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.transaction.annotation.Transactional;
 
+import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -96,7 +97,7 @@ public class MdmLinkQuerySvcImplSvc implements IMdmLinkQuerySvc {
 	public Page<MdmLinkJson> queryLinks(
 			MdmQuerySearchParameters theMdmQuerySearchParameters, MdmTransactionContext theMdmContext) {
 		@SuppressWarnings("unchecked")
-		Page<? extends IMdmLink> mdmLinks = myMdmLinkDaoSvc.executeTypedQuery(theMdmQuerySearchParameters);
+		Page<? extends IMdmLink<?>> mdmLinks = myMdmLinkDaoSvc.executeTypedQuery(theMdmQuerySearchParameters);
 		return mdmLinks.map(myMdmModelConverterSvc::toJson);
 	}
 
@@ -120,17 +121,26 @@ public class MdmLinkQuerySvcImplSvc implements IMdmLinkQuerySvc {
 				.setResourceType(theRequestResourceType);
 
 		@SuppressWarnings("unchecked")
-		Page<? extends IMdmLink> mdmLinkPage = myMdmLinkDaoSvc.executeTypedQuery(mdmQuerySearchParameters);
+		Page<? extends IMdmLink<?>> mdmLinkPage = myMdmLinkDaoSvc.executeTypedQuery(mdmQuerySearchParameters);
 		return mdmLinkPage.map(myMdmModelConverterSvc::toJson);
 	}
 
 	@Override
 	public List<MdmLinkWithRevisionJson> queryLinkHistory(MdmHistorySearchParameters theMdmHistorySearchParameters) {
+		@SuppressWarnings("unchecked")
 		final List<MdmLinkWithRevision<? extends IMdmLink<?>>> mdmLinkHistoryFromDao =
 				myMdmLinkDaoSvc.findMdmLinkHistory(theMdmHistorySearchParameters);
 
+		Comparator<MdmLinkWithRevisionJson> linkHistoryComparator =
+				Comparator.<MdmLinkWithRevisionJson, String>comparing(
+								l -> l.getMdmLink().getGoldenResourceId())
+						.thenComparing(l -> l.getMdmLink().getSourceId())
+						.thenComparing(Comparator.comparingLong(MdmLinkWithRevisionJson::getRevisionNumber)
+								.reversed());
+
 		return mdmLinkHistoryFromDao.stream()
 				.map(myMdmModelConverterSvc::toJson)
+				.sorted(linkHistoryComparator)
 				.collect(Collectors.toUnmodifiableList());
 	}
 }

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmLinkQuerySvcImplSvcTest.java

@@ -0,0 +1,73 @@
+package ca.uhn.fhir.jpa.mdm.svc;
+
+import ca.uhn.fhir.interceptor.model.RequestPartitionId;
+import ca.uhn.fhir.jpa.entity.MdmLink;
+import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
+import ca.uhn.fhir.mdm.api.IMdmLinkQuerySvc;
+import ca.uhn.fhir.mdm.api.MdmHistorySearchParameters;
+import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
+import ca.uhn.fhir.mdm.api.MdmLinkWithRevisionJson;
+import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
+import org.hl7.fhir.r4.model.Patient;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class MdmLinkQuerySvcImplSvcTest extends BaseMdmR4Test {
+
+	@Autowired
+	private IMdmLinkQuerySvc myMdmLinkQuerySvc;
+
+
+	@Test
+	public void testHistoryForGoldenResourceIds_withUserProvidedIds_sortsByUserProviderIds() {
+		String goldenResourceId  = createMdmLinksWithLinkedPatientsWithId(List.of("456a", "", "789a", "", "123a"));
+
+		final MdmHistorySearchParameters mdmHistorySearchParameters =
+			new MdmHistorySearchParameters().setGoldenResourceIds(Collections.singletonList(goldenResourceId));
+
+		List<MdmLinkWithRevisionJson> linksWithRevisionJson = myMdmLinkQuerySvc.queryLinkHistory(mdmHistorySearchParameters);
+
+		// links should be ordered by sourceId ascending
+		List<String> patientIdsFormLinks = linksWithRevisionJson.stream().map(l -> l.getMdmLink().getSourceId()).collect(Collectors.toList());
+
+		// Patients with blank client IDs should have been assigned sequential PID, which range we don;t know, but we want to make sure
+		// that "123a", "456a" and "789a" are in this order
+		List<String> orderedClientIdsFromLinks = patientIdsFormLinks.stream().filter(id -> id.endsWith("a")).collect(Collectors.toList());
+		assertEquals(List.of("Patient/123a", "Patient/456a", "Patient/789a"), orderedClientIdsFromLinks);
+	}
+
+	private String createMdmLinksWithLinkedPatientsWithId(List<String> thePatientIds) {
+		final Patient goldenPatient = createPatient();
+
+		for (String patientId : thePatientIds) {
+			final Patient patient = new Patient();
+			if (isNotBlank(patientId)) {
+				patient.setId(patientId);
+				myPatientDao.update(patient, new SystemRequestDetails());
+			} else {
+				myPatientDao.create(patient, new SystemRequestDetails());
+			}
+
+			MdmLink mdmLink = (MdmLink) myMdmLinkDaoSvc.newMdmLink();
+			mdmLink.setLinkSource(MdmLinkSourceEnum.MANUAL);
+			mdmLink.setMatchResult(MdmMatchResultEnum.MATCH);
+			mdmLink.setCreated(new Date());
+			mdmLink.setUpdated(new Date());
+			mdmLink.setGoldenResourcePersistenceId(runInTransaction(() -> myIdHelperService.getPidOrNull(RequestPartitionId.allPartitions(), goldenPatient)));
+			mdmLink.setSourcePersistenceId(runInTransaction(() -> myIdHelperService.getPidOrNull(RequestPartitionId.allPartitions(), patient)));
+			myMdmLinkDao.save(mdmLink);
+		}
+
+		return goldenPatient.getIdPart();
+	}
+
+}