Ensure a non-numeric FHIR ID doesn't result in a NumberFormatException when processing survivorship rules (#5883)

* Add failing test as well as commented out potential solution.

* Fix for NumberFormatException.

* Add conditional test for survivorship rules.

* Spotless.

* Add changelog.

* Code review feedback.

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/5886-mdm-apply-survivorship-number-format-exception.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 5886
+title: "Previously, either updating links on, or deleting one of two patients with non-numeric IDs linked to a golden 
+        patient would result in a HAPI-0389 if there were survivorship rules.
+        This issue has been fixed for both the update links and delete cases."

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

@@ -2,11 +2,16 @@ package ca.uhn.fhir.jpa.mdm.svc;
 
 import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
 import ca.uhn.fhir.mdm.api.IMdmSurvivorshipService;
+import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
+import ca.uhn.fhir.mdm.api.MdmMatchOutcome;
 import ca.uhn.fhir.mdm.model.MdmTransactionContext;
+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.List;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -55,4 +60,21 @@ class MdmSurvivorshipSvcImplIT extends BaseMdmR4Test {
 		assertEquals(p1.getTelecom().size(), p1.getTelecom().size());
 		assertTrue(p2.getTelecomFirstRep().equalsDeep(p1.getTelecomFirstRep()));
 	}
+
+	@Test
+	public void matchingPatientsWith_NON_Numeric_Ids_matches_doesNotThrow_NumberFormatException() {
+		final Patient frankPatient1 = buildFrankPatient();
+		frankPatient1.setId("patA");
+		myPatientDao.update(frankPatient1, new SystemRequestDetails());
+		final Patient frankPatient2 = buildFrankPatient();
+		frankPatient2.setId("patB");
+		myPatientDao.update(frankPatient2, new SystemRequestDetails());
+		final Patient goldenPatient = buildFrankPatient();
+		myPatientDao.create(goldenPatient, new SystemRequestDetails());
+
+		myMdmLinkDaoSvc.createOrUpdateLinkEntity(goldenPatient, frankPatient1, MdmMatchOutcome.NEW_GOLDEN_RESOURCE_MATCH, MdmLinkSourceEnum.MANUAL, createContextForCreate("Patient"));
+		myMdmLinkDaoSvc.createOrUpdateLinkEntity(goldenPatient, frankPatient2, MdmMatchOutcome.NEW_GOLDEN_RESOURCE_MATCH, MdmLinkSourceEnum.MANUAL, createContextForCreate("Patient"));
+
+		myMdmSurvivorshipService.rebuildGoldenResourceWithSurvivorshipRules(goldenPatient, new MdmTransactionContext(MdmTransactionContext.OperationType.UPDATE_LINK));
+	}
 }

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

@@ -25,8 +25,9 @@ import ca.uhn.fhir.rest.api.server.RequestDetails;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Patient;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -96,8 +97,9 @@ public class MdmSurvivorshipSvcImplTest {
 	}
 
 	@SuppressWarnings({"rawtypes", "unchecked"})
-	@Test
-	public void rebuildGoldenResourceCurrentLinksUsingSurvivorshipRules_withManyLinks_rebuildsInUpdateOrder() {
+	@ParameterizedTest
+	@ValueSource(booleans = {true,false})
+	public void rebuildGoldenResourceCurrentLinksUsingSurvivorshipRules_withManyLinks_rebuildsInUpdateOrder(boolean theIsUseNonNumericId) {
 		// setup
 		// create resources
 		Patient goldenPatient = new Patient();
@@ -126,7 +128,7 @@ public class MdmSurvivorshipSvcImplTest {
 			patient.addIdentifier()
 				.setSystem("http://example.com")
 				.setValue("Value" + i);
-			patient.setId("Patient/" + i);
+			patient.setId("Patient/" + (theIsUseNonNumericId ? "pat"+i : Integer.toString(i)));
 			resources.add(patient);
 
 			MdmLinkJson link = createLinkJson(
@@ -149,8 +151,13 @@ public class MdmSurvivorshipSvcImplTest {
 		when(myDaoRegistry.getResourceDao(eq("Patient")))
 			.thenReturn(resourceDao);
 		AtomicInteger counter = new AtomicInteger();
-		when(resourceDao.readByPid(any()))
-			.thenAnswer(params -> resources.get(counter.getAndIncrement()));
+		if (theIsUseNonNumericId) {
+			when(resourceDao.read(any(), any()))
+				.thenAnswer(params -> resources.get(counter.getAndIncrement()));
+		} else {
+			when(resourceDao.readByPid(any()))
+				.thenAnswer(params -> resources.get(counter.getAndIncrement()));
+		}
 		Page<MdmLinkJson> linkPage = mock(Page.class);
 		when(myMdmLinkQuerySvc.queryLinks(any(), any()))
 			.thenReturn(linkPage);

hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/svc/MdmSurvivorshipSvcImpl.java

@@ -31,17 +31,22 @@ import ca.uhn.fhir.mdm.api.params.MdmQuerySearchParameters;
 import ca.uhn.fhir.mdm.model.MdmTransactionContext;
 import ca.uhn.fhir.mdm.model.mdmevents.MdmLinkJson;
 import ca.uhn.fhir.mdm.util.GoldenResourceHelper;
+import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;
 import ca.uhn.fhir.util.TerserUtil;
+import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.springframework.data.domain.Page;
 
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
 public class MdmSurvivorshipSvcImpl implements IMdmSurvivorshipService {
+	private static final Pattern IS_UUID =
+			Pattern.compile("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}");
 
 	protected final FhirContext myFhirContext;
 
@@ -133,16 +138,30 @@ public class MdmSurvivorshipSvcImpl implements IMdmSurvivorshipService {
 			String sourceId = link.getSourceId();
 
 			// +1 because of "/" in id: "ResourceType/Id"
-			IResourcePersistentId<?> pid = getResourcePID(sourceId.substring(resourceType.length() + 1), resourceType);
+			final String sourceIdUnqualified = sourceId.substring(resourceType.length() + 1);
 
-			// this might be a bit unperformant
-			// but it depends how many links there are
-			// per golden resource (unlikely to be thousands)
-			return dao.readByPid(pid);
+			// myMdmLinkQuerySvc.queryLinks populates sourceId with the FHIR_ID, not the RES_ID, so if we don't
+			// add this conditional logic, on JPA, myIIdHelperService.newPidFromStringIdAndResourceName will fail with
+			// NumberFormatException
+			if (isNumericOrUuid(sourceIdUnqualified)) {
+				IResourcePersistentId<?> pid = getResourcePID(sourceIdUnqualified, resourceType);
+
+				// this might be a bit unperformant
+				// but it depends how many links there are
+				// per golden resource (unlikely to be thousands)
+				return dao.readByPid(pid);
+			} else {
+				return dao.read(new IdDt(sourceId), new SystemRequestDetails());
+			}
 		});
 	}
 
 	private IResourcePersistentId<?> getResourcePID(String theId, String theResourceType) {
 		return myIIdHelperService.newPidFromStringIdAndResourceName(theId, theResourceType);
 	}
+
+	private boolean isNumericOrUuid(String theLongCandidate) {
+		return StringUtils.isNumeric(theLongCandidate)
+				|| IS_UUID.matcher(theLongCandidate).matches();
+	}
 }