From e29ad5179ac2b9ff9ea5190e28dae439b27aeb50 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 4 Nov 2021 18:09:46 -0400 Subject: [PATCH] Head off potential MDM bug (#3140) * wip * fix up error message * wip * Fix all tests * Update old tests for failure messages * Add another test case * Remove dead file * Fix log message --- .../ca/uhn/fhir/jpa/mdm/svc/MdmResourceFilteringSvc.java | 2 +- .../ca/uhn/fhir/jpa/mdm/provider/BaseProviderR4Test.java | 3 +++ .../fhir/jpa/mdm/provider/MdmProviderCreateLinkR4Test.java | 4 ++-- .../provider/MdmProviderMergeGoldenResourcesR4Test.java | 3 ++- .../fhir/jpa/mdm/provider/MdmProviderUpdateLinkR4Test.java | 4 ++-- .../java/ca/uhn/fhir/mdm/provider/MdmControllerHelper.java | 7 ++++++- .../java/ca/uhn/fhir/mdm/provider/MdmControllerUtil.java | 1 - .../src/main/java/ca/uhn/fhir/mdm/util/MessageHelper.java | 4 ++++ 8 files changed, 20 insertions(+), 8 deletions(-) diff --git a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmResourceFilteringSvc.java b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmResourceFilteringSvc.java index 4a64ef6f72e..3f9ad7ced95 100644 --- a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmResourceFilteringSvc.java +++ b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/MdmResourceFilteringSvc.java @@ -57,7 +57,7 @@ public class MdmResourceFilteringSvc { */ public boolean shouldBeProcessed(IAnyResource theResource) { if (MdmResourceUtil.isMdmManaged(theResource)) { - ourLog.debug("MDM Message handler is dropping [{}] as it is MDM-managed.", theResource); + ourLog.debug("MDM Message handler is dropping [{}] as it is MDM-managed.", theResource.getId()); return false; } diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/BaseProviderR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/BaseProviderR4Test.java index 09fc013195c..86cd94f783a 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/BaseProviderR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/BaseProviderR4Test.java @@ -7,6 +7,7 @@ import ca.uhn.fhir.mdm.api.IMdmMatchFinderSvc; import ca.uhn.fhir.mdm.api.IMdmSubmitSvc; import ca.uhn.fhir.mdm.provider.MdmProviderDstu3Plus; import ca.uhn.fhir.mdm.rules.config.MdmSettings; +import ca.uhn.fhir.mdm.util.MessageHelper; import ca.uhn.fhir.test.utilities.BatchJobHelper; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; @@ -39,6 +40,8 @@ public abstract class BaseProviderR4Test extends BaseMdmR4Test { private MdmSettings myMdmSettings; @Autowired BatchJobHelper myBatchJobHelper; + @Autowired + MessageHelper myMessageHelper; private String defaultScript; diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderCreateLinkR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderCreateLinkR4Test.java index 4b19d53f2e9..085947e3527 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderCreateLinkR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderCreateLinkR4Test.java @@ -141,7 +141,7 @@ public class MdmProviderCreateLinkR4Test extends BaseLinkR4Test { @Test public void testCreateIllegalSecondArg() { try { - myMdmProvider.createLink(myPatientId, new StringType(""), MATCH_RESULT, myRequestDetails); + myMdmProvider.createLink(myVersionlessGodlenResourceId, new StringType(""), MATCH_RESULT, myRequestDetails); fail(); } catch (InvalidRequestException e) { assertThat(e.getMessage(), endsWith(" must have form / where is the id of the resource and is the type of the resource")); @@ -155,7 +155,7 @@ public class MdmProviderCreateLinkR4Test extends BaseLinkR4Test { myMdmProvider.createLink(new StringType(patient.getIdElement().getValue()), myPatientId, MATCH_RESULT, myRequestDetails); fail(); } catch (InvalidRequestException e) { - String expectedMessage = myMessageHelper.getMessageForUnmanagedResource(); + String expectedMessage = myMessageHelper.getMessageForFailedGoldenResourceLoad("goldenResourceId", patient.getId()); assertEquals(expectedMessage, e.getMessage()); } } diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderMergeGoldenResourcesR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderMergeGoldenResourcesR4Test.java index 39b5101c93e..e6f86b493ae 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderMergeGoldenResourcesR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderMergeGoldenResourcesR4Test.java @@ -129,7 +129,8 @@ public class MdmProviderMergeGoldenResourcesR4Test extends BaseProviderR4Test { myMdmProvider.mergeGoldenResources(fromGoldenResourceId, toGoldenResourceId, null, myRequestDetails); fail(); } catch (InvalidRequestException e) { - assertEquals("Only MDM managed resources can be merged. MDM managed resources must have the HAPI-MDM tag.", e.getMessage()); + String message = myMessageHelper.getMessageForFailedGoldenResourceLoad("fromGoldenResourceId", fromGoldenResourceId.getValue()); + assertEquals(e.getMessage(), message); } } diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderUpdateLinkR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderUpdateLinkR4Test.java index 9f6c7d3f76c..f4eecadfe6e 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderUpdateLinkR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/provider/MdmProviderUpdateLinkR4Test.java @@ -117,7 +117,7 @@ public class MdmProviderUpdateLinkR4Test extends BaseLinkR4Test { @Test public void testUpdateIllegalSecondArg() { try { - myMdmProvider.updateLink(myPatientId, new StringType(""), NO_MATCH_RESULT, myRequestDetails); + myMdmProvider.updateLink(myVersionlessGodlenResourceId, new StringType(""), NO_MATCH_RESULT, myRequestDetails); fail(); } catch (InvalidRequestException e) { assertThat(e.getMessage(), endsWith(" must have form / where is the id of the resource and is the type of the resource")); @@ -151,7 +151,7 @@ public class MdmProviderUpdateLinkR4Test extends BaseLinkR4Test { myMdmProvider.updateLink(new StringType(patient.getIdElement().getValue()), myPatientId, NO_MATCH_RESULT, myRequestDetails); fail(); } catch (InvalidRequestException e) { - String expectedMessage = myMessageHelper.getMessageForUnmanagedResource(); + String expectedMessage = myMessageHelper.getMessageForFailedGoldenResourceLoad("goldenResourceId", patient.getIdElement().getValue()); assertEquals(expectedMessage, e.getMessage()); } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerHelper.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerHelper.java index cf1c301dd7e..b5d79b22115 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerHelper.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerHelper.java @@ -64,7 +64,12 @@ public class MdmControllerHelper { public IAnyResource getLatestGoldenResourceFromIdOrThrowException(String theParamName, String theGoldenResourceId) { IdDt resourceId = MdmControllerUtil.getGoldenIdDtOrThrowException(theParamName, theGoldenResourceId); - return loadResource(resourceId.toUnqualifiedVersionless()); + IAnyResource iAnyResource = loadResource(resourceId.toUnqualifiedVersionless()); + if (MdmResourceUtil.isGoldenRecord(iAnyResource)) { + return iAnyResource; + } else { + throw new InvalidRequestException(myMessageHelper.getMessageForFailedGoldenResourceLoad(theParamName, theGoldenResourceId)); + } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerUtil.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerUtil.java index 42660ccc35c..4eb1c97f2c6 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerUtil.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/provider/MdmControllerUtil.java @@ -57,7 +57,6 @@ public class MdmControllerUtil { static IdDt getGoldenIdDtOrThrowException(String theParamName, String theId) { IdDt goldenResourceId = new IdDt(theId); - //TODO GGG MDM: maybe add a gate here to only consider resources that can possibly be EMPI'ed? if (goldenResourceId.getIdPart() == null) { throw new InvalidRequestException(theParamName + " is '" + theId + "'. must have form / where is the id of the resource"); } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/util/MessageHelper.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/util/MessageHelper.java index 37168596d08..ef648fcc85a 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/util/MessageHelper.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/util/MessageHelper.java @@ -107,4 +107,8 @@ public class MessageHelper { public String getMessageForMultipleGoldenRecords(String theSourceResource) { return theSourceResource + " already has matched golden resource. Use $mdm-query-links to see more details."; } + + public String getMessageForFailedGoldenResourceLoad(String theParamName, String theGoldenResourceId) { + return theGoldenResourceId + " used as parameter [" + theParamName + "] could not be loaded as a golden resource, as it appears to be lacking the golden resource meta tags."; + } }