4968 mdm throws nonsensical error when updating resources after mdm clear (#4975)

* log specific error message + test

* changelog

* remove draft tests

* remove another comment i left behind

* update issue number in changelog

* update error location

* update error location

* update test

* change to exception

* address review comments: add test with submit and also update change log

* format test nicer

---------

Co-authored-by: justindar <justin.dar@smilecdr.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4968-fix-MDM-throws-nonsensical-error-when-updating-resources-after-mdm-clear.yaml

@@ -0,0 +1,5 @@
+
+---
+type: fix
+issue: 4968
+title: "Previously, when performing a `$mdm-clear` followed by an update to an existing resource without doing the required `$mdm-submit` first, a non-descriptive NullPointerException was thrown. To fix this, an exception that includes a more descriptive/suggestive message will be thrown when this condition is detected."

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

@@ -19,6 +19,7 @@
  */
 package ca.uhn.fhir.jpa.mdm.svc;
 
+import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.mdm.dao.MdmLinkDaoSvc;
 import ca.uhn.fhir.jpa.mdm.svc.candidate.MatchedGoldenResourceCandidate;
 import ca.uhn.fhir.jpa.mdm.svc.candidate.MdmGoldenResourceFindingSvc;
@@ -34,6 +35,7 @@ import ca.uhn.fhir.mdm.model.MdmTransactionContext;
 import ca.uhn.fhir.mdm.util.EIDHelper;
 import ca.uhn.fhir.mdm.util.GoldenResourceHelper;
 import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.slf4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -80,10 +82,15 @@ public class MdmEidUpdateService {
 		} else {
 			//This is a new linking scenario. we have to break the existing link and link to the new Golden Resource. For now, we create duplicate.
 			//updated patient has an EID that matches to a new candidate. Link them, and set the Golden Resources possible duplicates
-			linkToNewGoldenResourceAndFlagAsDuplicate(theTargetResource, theMatchedGoldenResourceCandidate.getMatchResult(), updateContext.getExistingGoldenResource(), updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
+			IAnyResource theOldGoldenResource = updateContext.getExistingGoldenResource();
+			if (theOldGoldenResource == null) {
+				throw new InternalErrorException(Msg.code(2362) + "Old golden resource was null while updating MDM links with new golden resource. It is likely that a $mdm-clear was performed without a $mdm-submit. Link will not be updated.");
+			} else {
+				linkToNewGoldenResourceAndFlagAsDuplicate(theTargetResource, theMatchedGoldenResourceCandidate.getMatchResult(), theOldGoldenResource, updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
 
-			myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(theTargetResource, updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
+				myMdmSurvivorshipService.applySurvivorshipRulesToGoldenResource(theTargetResource, updateContext.getMatchedGoldenResource(), theMdmTransactionContext);
-			myMdmResourceDaoSvc.upsertGoldenResource(updateContext.getMatchedGoldenResource(), theMdmTransactionContext.getResourceType());
+				myMdmResourceDaoSvc.upsertGoldenResource(updateContext.getMatchedGoldenResource(), theMdmTransactionContext.getResourceType());
+			}
 		}
 	}
 

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderBatchR4Test.java

@@ -3,21 +3,27 @@ package ca.uhn.fhir.jpa.mdm.provider;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.interceptor.api.IInterceptorService;
 import ca.uhn.fhir.interceptor.api.Pointcut;
+import ca.uhn.fhir.mdm.log.Logs;
 import ca.uhn.fhir.mdm.rules.config.MdmSettings;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import ca.uhn.test.concurrency.PointcutLatch;
+import ca.uhn.test.util.LogbackCaptureTestExtension;
+import ch.qos.logback.classic.Logger;
 import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Medication;
 import org.hl7.fhir.r4.model.Organization;
+import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Practitioner;
 import org.hl7.fhir.r4.model.StringType;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -29,7 +35,7 @@ import java.util.stream.Stream;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.either;
-import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -45,6 +51,9 @@ public class MdmProviderBatchR4Test extends BaseLinkR4Test {
 	protected IAnyResource myGoldenMedication;
 	protected StringType myGoldenMedicationId;
 
+	@RegisterExtension
+	LogbackCaptureTestExtension myLogCapture = new LogbackCaptureTestExtension((Logger) Logs.getMdmTroubleshootingLog());
+
 	@Autowired
 	IInterceptorService myInterceptorService;
 	@Autowired
@@ -186,4 +195,45 @@ public class MdmProviderBatchR4Test extends BaseLinkR4Test {
 				.or(containsString(Msg.code(488) + "Failed to parse match URL")));// Sync case
 		}
 	}
+
+	@Test
+	public void testClearAndUpdateResource_WithoutMdmSubmit_LogsError() {
+		// Given
+		Patient janePatient = createPatientAndUpdateLinks(buildJanePatient());
+		Patient janePatient2 = createPatientAndUpdateLinks(buildJanePatient());
+		assertLinkCount(5);
+
+		// When
+		clearMdmLinks();
+
+		updatePatientAndUpdateLinks(janePatient);
+		try {
+			updatePatientAndUpdateLinks(janePatient2);
+		} catch (InternalErrorException e) {
+			// Then
+			assertLinkCount(1);
+			String expectedMsg = Msg.code(2362) + "Old golden resource was null while updating MDM links with new golden resource. It is likely that a $mdm-clear was performed without a $mdm-submit. Link will not be updated.";
+			assertEquals(expectedMsg, e.getMessage());
+		}
+	}
+
+	@ParameterizedTest
+	@MethodSource("requestTypes")
+	public void testUpdateResource_WithClearAndSubmit_Succeeds(ServletRequestDetails theSyncOrAsyncRequest) throws InterruptedException {
+		// Given
+		Patient janePatient = createPatientAndUpdateLinks(buildJanePatient());
+		Patient janePatient2 = createPatientAndUpdateLinks(buildJanePatient());
+		assertLinkCount(5);
+
+		// When
+		clearMdmLinks();
+		afterMdmLatch.runWithExpectedCount(3, () -> {
+			myMdmProvider.mdmBatchPatientType(null , null, theSyncOrAsyncRequest);
+		});
+
+		// Then
+		updatePatientAndUpdateLinks(janePatient);
+		updatePatientAndUpdateLinks(janePatient2);
+		assertLinkCount(3);
+	}
 }