From f7c47f911bd8006690206f25a9e5caf803413c6b Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Thu, 12 Nov 2020 15:59:00 -0500 Subject: [PATCH 1/2] Merge Service --- .../jpa/empi/svc/EmpiPersonMergerSvcImpl.java | 3 +- .../ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java | 50 +++-- .../EmpiProviderMergePersonsR4Test.java | 8 +- .../provider/EmpiProviderQueryLinkR4Test.java | 4 +- .../fhir/jpa/empi/svc/EmpiLinkSvcTest.java | 20 +- .../jpa/empi/svc/EmpiPersonMergerSvcTest.java | 207 +++++++++--------- .../jpa/empi/svc/EmpiResourceDaoSvcTest.java | 8 +- .../ca/uhn/fhir/empi/util/PersonHelper.java | 77 +++++-- .../util/PrimitiveTypeComparingPredicate.java | 70 ++++++ .../PrimitiveTypeComparingPredicateTest.java | 11 + 10 files changed, 295 insertions(+), 163 deletions(-) create mode 100644 hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicate.java create mode 100644 hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcImpl.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcImpl.java index 66292439a07..39276443b57 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcImpl.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcImpl.java @@ -60,8 +60,7 @@ public class EmpiPersonMergerSvcImpl implements IEmpiPersonMergerSvc { public IAnyResource mergePersons(IAnyResource theFrom, IAnyResource theTo, EmpiTransactionContext theEmpiTransactionContext) { Long toPid = myIdHelperService.getPidOrThrowException(theTo); -// TODO NG - Revisit when merge rules are defined -// myPersonHelper.mergeFields(theFrom, theTo); + myPersonHelper.mergeFields(theFrom, theTo); mergeSourceResourceLinks(theFrom, theTo, toPid, theEmpiTransactionContext); removeTargetLinks(theFrom, theTo, theEmpiTransactionContext); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java index 36c20dd83c3..294883597a6 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java @@ -36,9 +36,9 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import org.apache.commons.lang3.StringUtils; import org.hamcrest.Matcher; 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.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.ContactPoint; @@ -139,12 +139,12 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { @Nonnull protected Patient createUnmanagedSourceResource() { - return createSourceResourcePatient(new Patient(), false); + return createGoldenPatient(new Patient(), false); } @Nonnull - protected Patient createSourceResourcePatient() { - return createSourceResourcePatient(new Patient(), true); + protected Patient createGoldenPatient() { + return createGoldenPatient(new Patient(), true); } @Nonnull @@ -153,17 +153,17 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { } @Nonnull - protected Patient createSourceResourcePatient(Patient theSourceResourcePatient) { - return createSourceResourcePatient(theSourceResourcePatient, true); + protected Patient createGoldenPatient(Patient thePatient) { + return createGoldenPatient(thePatient, true); } @Nonnull - protected Patient createSourceResourcePatient(Patient theSourceResourcePatient, boolean theEmpiManaged) { + protected Patient createGoldenPatient(Patient thePatient, boolean theEmpiManaged) { if (theEmpiManaged) { - theSourceResourcePatient.getMeta().addTag().setSystem(EmpiConstants.SYSTEM_EMPI_MANAGED).setCode(EmpiConstants.CODE_HAPI_EMPI_MANAGED); - theSourceResourcePatient.setActive(true); + thePatient.getMeta().addTag().setSystem(EmpiConstants.SYSTEM_EMPI_MANAGED).setCode(EmpiConstants.CODE_HAPI_EMPI_MANAGED); + thePatient.setActive(true); } - DaoMethodOutcome outcome = myPatientDao.create(theSourceResourcePatient); + DaoMethodOutcome outcome = myPatientDao.create(thePatient); Patient patient = (Patient) outcome.getResource(); patient.setId(outcome.getId()); return patient; @@ -393,23 +393,23 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { } protected Patient getOnlyActiveSourcePatient() { - List resources = getAllActiveSourcePatients(); + List resources = getAllActiveGoldenPatients(); assertEquals(1, resources.size()); return (Patient) resources.get(0); } @Nonnull - protected List getAllActiveSourcePatients() { - return getAllSourcePatients(true); + protected List getAllActiveGoldenPatients() { + return getAllGoldenPatients(true); } @Nonnull - protected List getAllSourcePatients() { - return getAllSourcePatients(false); + protected List getAllGoldenPatients() { + return getAllGoldenPatients(false); } @Nonnull - private List getAllSourcePatients(boolean theOnlyActive) { + private List getAllGoldenPatients(boolean theOnlyActive) { SearchParameterMap map = new SearchParameterMap(); map.setLoadSynchronous(true); //TODO GGG ensure that this tag search works effectively. @@ -423,7 +423,7 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { @Nonnull protected EmpiLink createResourcesAndBuildTestEmpiLink() { - Patient sourcePatient = createSourceResourcePatient(); + Patient sourcePatient = createGoldenPatient(); Patient patient = createPatient(); EmpiLink empiLink = myEmpiLinkDaoSvc.newEmpiLink(); @@ -468,10 +468,18 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { } - protected void print(IBaseResource ... theResource) { - for (IBaseResource r : theResource) { - System.out.println(myFhirContext.newJsonParser().encodeResourceToString(r)); + protected void print(String message, IBaseResource ... theResource) { + if (StringUtils.isNotEmpty(message)) { + ourLog.info(message); } + + for (IBaseResource r : theResource) { + ourLog.info(myFhirContext.newJsonParser().encodeResourceToString(r)); + } + } + + protected void print(IBaseResource ... theResource) { + print(null, theResource); } @@ -487,7 +495,7 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { protected void printLinks() { myEmpiLinkDao.findAll().forEach(empiLink -> { - System.out.println(empiLink); + ourLog.info(String.valueOf(empiLink)); }); } diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMergePersonsR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMergePersonsR4Test.java index 76cc230e1b6..f09d7d39dca 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMergePersonsR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMergePersonsR4Test.java @@ -31,9 +31,9 @@ public class EmpiProviderMergePersonsR4Test extends BaseProviderR4Test { super.before(); super.loadEmpiSearchParameters(); - myFromSourcePatient = createSourceResourcePatient(); + myFromSourcePatient = createGoldenPatient(); myFromSourcePatientId = new StringType(myFromSourcePatient.getIdElement().getValue()); - myToSourcePatient = createSourceResourcePatient(); + myToSourcePatient = createGoldenPatient(); myToSourcePatientId = new StringType(myToSourcePatient.getIdElement().getValue()); } @@ -44,8 +44,8 @@ public class EmpiProviderMergePersonsR4Test extends BaseProviderR4Test { assertEquals(myToSourcePatient.getIdElement(), mergedSourcePatient.getIdElement()); // TODO GGG RP FIX //assertThat(mergedSourcePatient, is(sameSourceResourceAs(myToSourcePatient))); - assertEquals(2, getAllSourcePatients().size()); - assertEquals(1, getAllActiveSourcePatients().size()); + assertEquals(2, getAllGoldenPatients().size()); + assertEquals(1, getAllActiveGoldenPatients().size()); Patient fromSourcePatient = myPatientDao.read(myFromSourcePatient.getIdElement().toUnqualifiedVersionless()); assertThat(fromSourcePatient.getActive(), is(false)); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderQueryLinkR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderQueryLinkR4Test.java index 92c019dc943..06aa70d321e 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderQueryLinkR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderQueryLinkR4Test.java @@ -42,10 +42,10 @@ public class EmpiProviderQueryLinkR4Test extends BaseLinkR4Test { // Add a possible duplicate myLinkSource = new StringType(EmpiLinkSourceEnum.AUTO.name()); - Patient sourcePatient1 = createSourceResourcePatient(); + Patient sourcePatient1 = createGoldenPatient(); myPerson1Id = new StringType(sourcePatient1.getIdElement().toVersionless().getValue()); Long sourcePatient1Pid = myIdHelperService.getPidOrNull(sourcePatient1); - Patient sourcePatient2 = createSourceResourcePatient(); + Patient sourcePatient2 = createGoldenPatient(); myPerson2Id = new StringType(sourcePatient2.getIdElement().toVersionless().getValue()); Long sourcePatient2Pid = myIdHelperService.getPidOrNull(sourcePatient2); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiLinkSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiLinkSvcTest.java index 7dc7687acb3..f584219eb49 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiLinkSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiLinkSvcTest.java @@ -46,7 +46,7 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testCreateRemoveLink() { assertLinkCount(0); - Patient sourcePatient = createSourceResourcePatient(); + Patient sourcePatient = createGoldenPatient(); IdType sourcePatientId = sourcePatient.getIdElement().toUnqualifiedVersionless(); assertEquals(0, sourcePatient.getLink().size()); Patient patient = createPatient(); @@ -70,8 +70,8 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testPossibleDuplicate() { assertLinkCount(0); - Patient sourcePatient1 = createSourceResourcePatient(); - Patient sourcePatient2 = createSourceResourcePatient(); + Patient sourcePatient1 = createGoldenPatient(); + Patient sourcePatient2 = createGoldenPatient(); // TODO GGG MDM NOT VALID myEmpiLinkSvc.updateLink(sourcePatient1, sourcePatient2, EmpiMatchOutcome.POSSIBLE_DUPLICATE, EmpiLinkSourceEnum.AUTO, createContextForCreate("Patient")); assertLinkCount(1); @@ -80,8 +80,8 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testNoMatchBlocksPossibleDuplicate() { assertLinkCount(0); - Patient sourcePatient1 = createSourceResourcePatient(); - Patient sourcePatient2 = createSourceResourcePatient(); + Patient sourcePatient1 = createGoldenPatient(); + Patient sourcePatient2 = createGoldenPatient(); Long sourcePatient1Pid = myIdHelperService.getPidOrNull(sourcePatient1); Long sourcePatient2Pid = myIdHelperService.getPidOrNull(sourcePatient2); @@ -98,8 +98,8 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testNoMatchBlocksPossibleDuplicateReversed() { assertLinkCount(0); - Patient sourcePatient1 = createSourceResourcePatient(); - Patient sourcePatient2 = createSourceResourcePatient(); + Patient sourcePatient1 = createGoldenPatient(); + Patient sourcePatient2 = createGoldenPatient(); Long sourcePatient1Pid = myIdHelperService.getPidOrNull(sourcePatient1); Long sourcePatient2Pid = myIdHelperService.getPidOrNull(sourcePatient2); @@ -124,7 +124,7 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testManualEmpiLinksCannotBeModifiedBySystem() { - Patient sourcePatient = createSourceResourcePatient(buildJaneSourcePatient()); + Patient sourcePatient = createGoldenPatient(buildJaneSourcePatient()); Patient patient = createPatient(buildJanePatient()); myEmpiLinkSvc.updateLink(sourcePatient, patient, EmpiMatchOutcome.NO_MATCH, EmpiLinkSourceEnum.MANUAL, createContextForCreate("Patient")); @@ -138,7 +138,7 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testAutomaticallyAddedNO_MATCHEmpiLinksAreNotAllowed() { - Patient sourcePatient = createSourceResourcePatient(buildJaneSourcePatient()); + Patient sourcePatient = createGoldenPatient(buildJaneSourcePatient()); Patient patient = createPatient(buildJanePatient()); // Test: it should be impossible to have a AUTO NO_MATCH record. The only NO_MATCH records in the system must be MANUAL. @@ -152,7 +152,7 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Test public void testSyncDoesNotSyncNoMatchLinks() { - Patient sourcePatient = createSourceResourcePatient(buildJaneSourcePatient()); + Patient sourcePatient = createGoldenPatient(buildJaneSourcePatient()); Patient patient1 = createPatient(buildJanePatient()); Patient patient2 = createPatient(buildJanePatient()); assertEquals(0, myEmpiLinkDao.count()); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java index 3536fc582e3..9e16a1a88a8 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java @@ -54,10 +54,10 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Autowired IInterceptorService myInterceptorService; - private Patient myFromSourcePatient; - private Patient myToSourcePatient; - private Long myFromSourcePatientPid; - private Long myToSourcePatientPid; + private Patient myFromGoldenPatient; + private Patient myToGoldenPatient; + private Long myFromGoldenPatientPid; + private Long myToGoldenPatientPid; private Patient myTargetPatient1; private Patient myTargetPatient2; private Patient myTargetPatient3; @@ -66,12 +66,12 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { public void before() { super.loadEmpiSearchParameters(); - myFromSourcePatient = createSourceResourcePatient(); - IdType fromSourcePatientId = myFromSourcePatient.getIdElement().toUnqualifiedVersionless(); - myFromSourcePatientPid = myIdHelperService.getPidOrThrowException(fromSourcePatientId); - myToSourcePatient = createSourceResourcePatient(); - IdType toSourcePatientId = myToSourcePatient.getIdElement().toUnqualifiedVersionless(); - myToSourcePatientPid = myIdHelperService.getPidOrThrowException(toSourcePatientId); + myFromGoldenPatient = createGoldenPatient(); + IdType fromSourcePatientId = myFromGoldenPatient.getIdElement().toUnqualifiedVersionless(); + myFromGoldenPatientPid = myIdHelperService.getPidOrThrowException(fromSourcePatientId); + myToGoldenPatient = createGoldenPatient(); + IdType toGoldenPatientId = myToGoldenPatient.getIdElement().toUnqualifiedVersionless(); + myToGoldenPatientPid = myIdHelperService.getPidOrThrowException(toGoldenPatientId); myTargetPatient1 = createPatient(); myTargetPatient2 = createPatient(); @@ -90,19 +90,19 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void emptyMerge() { - assertEquals(2, getAllSourcePatients().size()); - assertEquals(2, getAllActiveSourcePatients().size()); + assertEquals(2, getAllGoldenPatients().size()); + assertEquals(2, getAllActiveGoldenPatients().size()); - Patient mergedSourcePatients = mergeSourcePatients(); - assertEquals(myToSourcePatient.getIdElement(), mergedSourcePatients.getIdElement()); - assertThat(mergedSourcePatients, is(sameSourceResourceAs(mergedSourcePatients))); - assertEquals(2, getAllSourcePatients().size()); - assertEquals(1, getAllActiveSourcePatients().size()); + Patient mergedGoldenPatient = mergeGoldenPatients(); + assertEquals(myToGoldenPatient.getIdElement(), mergedGoldenPatient.getIdElement()); + assertThat(mergedGoldenPatient, is(sameSourceResourceAs(mergedGoldenPatient))); + assertEquals(2, getAllGoldenPatients().size()); + assertEquals(1, getAllActiveGoldenPatients().size()); } - private Patient mergeSourcePatients() { + private Patient mergeGoldenPatients() { assertEquals(0, redirectLinkCount()); - Patient retval = (Patient) myEmpiPersonMergerSvc.mergePersons(myFromSourcePatient, myToSourcePatient, createEmpiContext()); + Patient retval = (Patient) myEmpiPersonMergerSvc.mergePersons(myFromGoldenPatient, myToGoldenPatient, createEmpiContext()); assertEquals(1, redirectLinkCount()); return retval; } @@ -122,8 +122,8 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void mergeRemovesPossibleDuplicatesLink() { EmpiLink empiLink = myEmpiLinkDaoSvc.newEmpiLink() - .setSourceResourcePid(myToSourcePatientPid) - .setTargetPid(myFromSourcePatientPid) + .setSourceResourcePid(myToGoldenPatientPid) + .setTargetPid(myFromGoldenPatientPid) .setEmpiTargetType("Patient") .setMatchResult(EmpiMatchResultEnum.POSSIBLE_DUPLICATE) .setLinkSource(EmpiLinkSourceEnum.AUTO); @@ -138,7 +138,7 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { myEmpiLinkHelper.logEmpiLinks(); - mergeSourcePatients(); + mergeGoldenPatients(); { List foundLinks = myEmpiLinkDao.findAll(); @@ -149,9 +149,9 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void fullFromEmptyTo() { - populatePerson(myFromSourcePatient); + populatePerson(myFromGoldenPatient); - Patient mergedSourcePatient = mergeSourcePatients(); + Patient mergedSourcePatient = mergeGoldenPatients(); // TODO NG - Revisit when rules are ready // HumanName returnedName = mergedSourcePatient.getNameFirstRep(); // assertEquals(GIVEN_NAME, returnedName.getGivenAsSingleString()); @@ -161,10 +161,10 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void emptyFromFullTo() { - myFromSourcePatient.getName().add(new HumanName().addGiven(BAD_GIVEN_NAME)); - populatePerson(myToSourcePatient); + myFromGoldenPatient.getName().add(new HumanName().addGiven(BAD_GIVEN_NAME)); + populatePerson(myToGoldenPatient); - Patient mergedSourcePatient = mergeSourcePatients(); + Patient mergedSourcePatient = mergeGoldenPatients(); HumanName returnedName = mergedSourcePatient.getNameFirstRep(); assertEquals(GIVEN_NAME, returnedName.getGivenAsSingleString()); assertEquals(FAMILY_NAME, returnedName.getFamily()); @@ -173,92 +173,92 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void fromLinkToNoLink() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); - Patient mergedSourcePatient = mergeSourcePatients(); - List links = getNonRedirectLinksByPerson(mergedSourcePatient); + Patient mergedGoldenPatient = mergeGoldenPatients(); + List links = getNonRedirectLinksByPerson(mergedGoldenPatient); assertEquals(1, links.size()); - assertThat(mergedSourcePatient, is(possibleLinkedTo(myTargetPatient1))); -// fail("FIXME"); TODO NG - Confirm it's ok + assertThat(mergedGoldenPatient, is(possibleLinkedTo(myTargetPatient1))); } @Test public void fromNoLinkToLink() { - createEmpiLink(myToSourcePatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient1); - Patient mergedSourcePatient = mergeSourcePatients(); + Patient mergedSourcePatient = mergeGoldenPatients(); List links = getNonRedirectLinksByPerson(mergedSourcePatient); assertEquals(1, links.size()); assertThat(mergedSourcePatient, is(possibleLinkedTo(myTargetPatient1))); -// fail("FIXME"); TODO NG - Confirm it's ok } @Test public void fromManualLinkOverridesAutoToLink() { - EmpiLink fromLink = createEmpiLink(myFromSourcePatient, myTargetPatient1); + EmpiLink fromLink = createEmpiLink(myFromGoldenPatient, myTargetPatient1); fromLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); fromLink.setMatchResult(EmpiMatchResultEnum.MATCH); saveLink(fromLink); - createEmpiLink(myToSourcePatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient1); - mergeSourcePatients(); - List links = getNonRedirectLinksByPerson(myToSourcePatient); + mergeGoldenPatients(); + List links = getNonRedirectLinksByPerson(myToGoldenPatient); assertEquals(1, links.size()); assertEquals(EmpiLinkSourceEnum.MANUAL, links.get(0).getLinkSource()); } - private List getNonRedirectLinksByPerson(Patient theSourcePatient) { - return myEmpiLinkDaoSvc.findEmpiLinksBySourceResource(theSourcePatient).stream() + private List getNonRedirectLinksByPerson(Patient theGoldenPatient) { + return myEmpiLinkDaoSvc.findEmpiLinksBySourceResource(theGoldenPatient).stream() .filter(link -> !link.isRedirect()) .collect(Collectors.toList()); } @Test public void fromManualNoMatchLinkOverridesAutoToLink() { - EmpiLink fromLink = createEmpiLink(myFromSourcePatient, myTargetPatient1); + EmpiLink fromLink = createEmpiLink(myFromGoldenPatient, myTargetPatient1); fromLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); fromLink.setMatchResult(EmpiMatchResultEnum.NO_MATCH); saveLink(fromLink); - createEmpiLink(myToSourcePatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient1); - mergeSourcePatients(); - List links = getNonRedirectLinksByPerson(myToSourcePatient); + mergeGoldenPatients(); + List links = getNonRedirectLinksByPerson(myToGoldenPatient); assertEquals(1, links.size()); assertEquals(EmpiLinkSourceEnum.MANUAL, links.get(0).getLinkSource()); + assertEquals(EmpiMatchResultEnum.NO_MATCH, links.get(0).getMatchResult()); } @Test public void fromManualAutoMatchLinkNoOverridesManualToLink() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); - EmpiLink toLink = createEmpiLink(myToSourcePatient, myTargetPatient1); + EmpiLink toLink = createEmpiLink(myToGoldenPatient, myTargetPatient1); toLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); toLink.setMatchResult(EmpiMatchResultEnum.NO_MATCH); saveLink(toLink); - mergeSourcePatients(); - List links = getNonRedirectLinksByPerson(myToSourcePatient); + mergeGoldenPatients(); + List links = getNonRedirectLinksByPerson(myToGoldenPatient); assertEquals(1, links.size()); assertEquals(EmpiLinkSourceEnum.MANUAL, links.get(0).getLinkSource()); + assertEquals(EmpiMatchResultEnum.NO_MATCH, links.get(0).getMatchResult()); } @Test public void fromNoMatchMergeToManualMatchIsError() { - EmpiLink fromLink = createEmpiLink(myFromSourcePatient, myTargetPatient1); + EmpiLink fromLink = createEmpiLink(myFromGoldenPatient, myTargetPatient1); fromLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); fromLink.setMatchResult(EmpiMatchResultEnum.NO_MATCH); saveLink(fromLink); - EmpiLink toLink = createEmpiLink(myToSourcePatient, myTargetPatient1); + EmpiLink toLink = createEmpiLink(myToGoldenPatient, myTargetPatient1); toLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); toLink.setMatchResult(EmpiMatchResultEnum.MATCH); saveLink(toLink); try { - mergeSourcePatients(); + mergeGoldenPatients(); fail(); } catch (InvalidRequestException e) { assertEquals("A MANUAL NO_MATCH link may not be merged into a MANUAL MATCH link for the same target", e.getMessage()); @@ -267,18 +267,18 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void fromMatchMergeToManualNoMatchIsError() { - EmpiLink fromLink = createEmpiLink(myFromSourcePatient, myTargetPatient1); + EmpiLink fromLink = createEmpiLink(myFromGoldenPatient, myTargetPatient1); fromLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); fromLink.setMatchResult(EmpiMatchResultEnum.MATCH); saveLink(fromLink); - EmpiLink toLink = createEmpiLink(myToSourcePatient, myTargetPatient1); + EmpiLink toLink = createEmpiLink(myToGoldenPatient, myTargetPatient1); toLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); toLink.setMatchResult(EmpiMatchResultEnum.NO_MATCH); saveLink(toLink); try { - mergeSourcePatients(); + mergeGoldenPatients(); fail(); } catch (InvalidRequestException e) { assertEquals("A MANUAL MATCH link may not be merged into a MANUAL NO_MATCH link for the same target", e.getMessage()); @@ -287,35 +287,38 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void fromNoMatchMergeToManualMatchDifferentPatientIsOk() { - EmpiLink fromLink = createEmpiLink(myFromSourcePatient, myTargetPatient1); + EmpiLink fromLink = createEmpiLink(myFromGoldenPatient, myTargetPatient1); fromLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); fromLink.setMatchResult(EmpiMatchResultEnum.NO_MATCH); saveLink(fromLink); - EmpiLink toLink = createEmpiLink(myToSourcePatient, myTargetPatient2); + EmpiLink toLink = createEmpiLink(myToGoldenPatient, myTargetPatient2); toLink.setLinkSource(EmpiLinkSourceEnum.MANUAL); toLink.setMatchResult(EmpiMatchResultEnum.MATCH); saveLink(toLink); - mergeSourcePatients(); - assertResourceHasLinkCount(myToSourcePatient, 1, (e) -> false); -// assertEquals(1, myToSourcePatient.getLink().size()); + mergeGoldenPatients(); + + assertResourceHasLinkCount(myToGoldenPatient, 3, (e) -> true); + assertResourceHasLinkCount(myFromGoldenPatient, 0, (e) -> true); + // TODO ENSURE PROPER LINK TYPES + // TODO MAKE IT MORE READABLE + assertEquals(3, myEmpiLinkDao.count()); } @Test public void from123To1() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); - createEmpiLink(myFromSourcePatient, myTargetPatient2); - createEmpiLink(myFromSourcePatient, myTargetPatient3); - createEmpiLink(myToSourcePatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient2); + createEmpiLink(myFromGoldenPatient, myTargetPatient3); + createEmpiLink(myToGoldenPatient, myTargetPatient1); - mergeSourcePatients(); + mergeGoldenPatients(); myEmpiLinkHelper.logEmpiLinks(); - assertThat(myToSourcePatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); -// assertEquals(3, myToSourcePatient.getLink().size()); - assertResourceHasAutoLinkCount(myToSourcePatient, 3); + assertThat(myToGoldenPatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); + assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } private void assertResourceHasAutoLinkCount(IBaseResource theResource, int theCount) { @@ -329,49 +332,47 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Test public void from1To123() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); - createEmpiLink(myToSourcePatient, myTargetPatient1); - createEmpiLink(myToSourcePatient, myTargetPatient2); - createEmpiLink(myToSourcePatient, myTargetPatient3); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient2); + createEmpiLink(myToGoldenPatient, myTargetPatient3); - mergeSourcePatients(); + mergeGoldenPatients(); myEmpiLinkHelper.logEmpiLinks(); - assertThat(myToSourcePatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); - assertResourceHasAutoLinkCount(myToSourcePatient, 3); + assertThat(myToGoldenPatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); + assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } @Test public void from123To123() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); - createEmpiLink(myFromSourcePatient, myTargetPatient2); - createEmpiLink(myFromSourcePatient, myTargetPatient3); - createEmpiLink(myToSourcePatient, myTargetPatient1); - createEmpiLink(myToSourcePatient, myTargetPatient2); - createEmpiLink(myToSourcePatient, myTargetPatient3); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient2); + createEmpiLink(myFromGoldenPatient, myTargetPatient3); + createEmpiLink(myToGoldenPatient, myTargetPatient1); + createEmpiLink(myToGoldenPatient, myTargetPatient2); + createEmpiLink(myToGoldenPatient, myTargetPatient3); - mergeSourcePatients(); + mergeGoldenPatients(); - assertThat(myToSourcePatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); + assertThat(myToGoldenPatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); -// assertEquals(3, myToSourcePatient.getLink().size()); - assertResourceHasAutoLinkCount(myToSourcePatient, 3); + assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } @Test public void from12To23() { - createEmpiLink(myFromSourcePatient, myTargetPatient1); - createEmpiLink(myFromSourcePatient, myTargetPatient2); - createEmpiLink(myToSourcePatient, myTargetPatient2); - createEmpiLink(myToSourcePatient, myTargetPatient3); + createEmpiLink(myFromGoldenPatient, myTargetPatient1); + createEmpiLink(myFromGoldenPatient, myTargetPatient2); + createEmpiLink(myToGoldenPatient, myTargetPatient2); + createEmpiLink(myToGoldenPatient, myTargetPatient3); - mergeSourcePatients(); + mergeGoldenPatients(); myEmpiLinkHelper.logEmpiLinks(); - assertThat(myToSourcePatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); + assertThat(myToGoldenPatient, is(possibleLinkedTo(myTargetPatient1, myTargetPatient2, myTargetPatient3))); -// assertEquals(3, myToSourcePatient.getLink().size()); - assertResourceHasAutoLinkCount(myToSourcePatient, 3); + assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } @Test @@ -412,17 +413,19 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { } @Test - public void testMergeIdentities() { - myFromSourcePatient.addIdentifier().setValue("aaa"); - myFromSourcePatient.addIdentifier().setValue("bbb"); - assertThat(myFromSourcePatient.getIdentifier(), hasSize(2)); + public void testMergeIdentifiers() { + myFromGoldenPatient.addIdentifier().setValue("aaa").setSystem("SYSTEM1"); + myFromGoldenPatient.addIdentifier().setValue("bbb").setSystem("SYSTEM1"); + myFromGoldenPatient.addIdentifier().setValue("ccc").setSystem("SYSTEM2"); + assertThat(myFromGoldenPatient.getIdentifier(), hasSize(3)); - myToSourcePatient.addIdentifier().setValue("aaa"); - myToSourcePatient.addIdentifier().setValue("ccc"); - assertThat(myToSourcePatient.getIdentifier(), hasSize(2)); + myToGoldenPatient.addIdentifier().setValue("aaa").setSystem("SYSTEM1"); + myToGoldenPatient.addIdentifier().setValue("ccc").setSystem("SYSTEM1"); + assertThat(myToGoldenPatient.getIdentifier(), hasSize(2)); - mergeSourcePatients(); - assertThat(myToSourcePatient.getIdentifier(), hasSize(3)); + mergeGoldenPatients(); + + assertThat(myToGoldenPatient.getIdentifier(), hasSize(4)); } private EmpiLink createEmpiLink(Patient theSourcePatient, Patient theTargetPatient) { diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java index 687c491ffe2..54054d23fc4 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java @@ -25,10 +25,10 @@ public class EmpiResourceDaoSvcTest extends BaseEmpiR4Test { @Test public void testSearchPersonByEidExcludesInactive() { - Patient goodSourcePatient = addExternalEID(createSourceResourcePatient(), TEST_EID); + Patient goodSourcePatient = addExternalEID(createGoldenPatient(), TEST_EID); myPatientDao.update(goodSourcePatient); - Patient badSourcePatient = addExternalEID(createSourceResourcePatient(), TEST_EID); + Patient badSourcePatient = addExternalEID(createGoldenPatient(), TEST_EID); badSourcePatient.setActive(false); myPatientDao.update(badSourcePatient); @@ -39,10 +39,10 @@ public class EmpiResourceDaoSvcTest extends BaseEmpiR4Test { @Test public void testSearchPersonByEidExcludesNonEmpiManaged() { - Patient goodSourcePatient = addExternalEID(createSourceResourcePatient(), TEST_EID); + Patient goodSourcePatient = addExternalEID(createGoldenPatient(), TEST_EID); myPatientDao.update(goodSourcePatient); - Patient badSourcePatient = addExternalEID(createSourceResourcePatient(new Patient(), false), TEST_EID); + Patient badSourcePatient = addExternalEID(createGoldenPatient(new Patient(), false), TEST_EID); myPatientDao.update(badSourcePatient); Optional foundSourcePatient = myResourceDaoSvc.searchSourceResourceByEID(TEST_EID, "Patient"); diff --git a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PersonHelper.java b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PersonHelper.java index 6e6746ede6f..fcd6bca8558 100644 --- a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PersonHelper.java +++ b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PersonHelper.java @@ -25,7 +25,6 @@ import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.empi.api.EmpiConstants; import ca.uhn.fhir.empi.api.IEmpiLinkQuerySvc; import ca.uhn.fhir.empi.api.IEmpiSettings; import ca.uhn.fhir.empi.log.Logs; @@ -37,7 +36,6 @@ import ca.uhn.fhir.util.FhirTerser; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseBackboneElement; -import org.hl7.fhir.instance.model.api.IBaseCoding; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; @@ -67,6 +65,8 @@ import static ca.uhn.fhir.context.FhirVersionEnum.R4; public class PersonHelper { private static final Logger ourLog = Logs.getEmpiTroubleshootingLog(); + private static final String FIELD_NAME_IDENTIFIER = "identifier"; + @Autowired private IEmpiSettings myEmpiConfig; @Autowired @@ -96,7 +96,7 @@ public class PersonHelper { IBaseResource newSourceResource = resourceDefinition.newInstance(); // hapi has 2 metamodels: for children and types - BaseRuntimeChildDefinition sourceResourceIdentifier = resourceDefinition.getChildByName("identifier"); + BaseRuntimeChildDefinition sourceResourceIdentifier = resourceDefinition.getChildByName(FIELD_NAME_IDENTIFIER); cloneAllExternalEidsIntoNewSourceResource(sourceResourceIdentifier, theIncomingResource, newSourceResource); @@ -145,7 +145,7 @@ public class PersonHelper { // get a ref to the actual ID Field RuntimeResourceDefinition resourceDefinition = myFhirContext.getResourceDefinition(theResourceToCloneInto); // hapi has 2 metamodels: for children and types - BaseRuntimeChildDefinition resourceIdentifier = resourceDefinition.getChildByName("identifier"); + BaseRuntimeChildDefinition resourceIdentifier = resourceDefinition.getChildByName(FIELD_NAME_IDENTIFIER); cloneEidIntoResource(resourceIdentifier, toId(theEid), theResourceToCloneInto); } @@ -154,7 +154,7 @@ public class PersonHelper { */ private void cloneEidIntoResource(BaseRuntimeChildDefinition theIdentifierDefinition, IBase theEid, IBase theResourceToCloneEidInto) { // FHIR choice types - fields within fhir where we have a choice of ids - BaseRuntimeElementCompositeDefinition childIdentifier = (BaseRuntimeElementCompositeDefinition) theIdentifierDefinition.getChildByName("identifier"); + BaseRuntimeElementCompositeDefinition childIdentifier = (BaseRuntimeElementCompositeDefinition) theIdentifierDefinition.getChildByName(FIELD_NAME_IDENTIFIER); IBase resourceNewIdentifier = childIdentifier.newInstance(); FhirTerser terser = myFhirContext.newTerser(); @@ -162,6 +162,44 @@ public class PersonHelper { theIdentifierDefinition.getMutator().addValue(theResourceToCloneEidInto, resourceNewIdentifier); } + /** + * Clones specified composite field (collection). Composite field values must confirm to the collections + * contract. + * + * @param theFrom Resource to clone the specified filed from + * @param theTo Resource to clone the specified filed to + * @param field Field name to be copied + */ + private void cloneCompositeField(IBaseResource theFrom, IBaseResource theTo, String field) { + FhirTerser terser = myFhirContext.newTerser(); + + RuntimeResourceDefinition definition = myFhirContext.getResourceDefinition(theFrom); + BaseRuntimeChildDefinition childDefinition = definition.getChildByName(field); + + IFhirPath fhirPath = myFhirContext.newFhirPath(); + List theFromFieldValues = childDefinition.getAccessor().getValues(theFrom); + List theToFieldValues = childDefinition.getAccessor().getValues(theTo); + + for (IBase theFromFieldValue: theFromFieldValues) { + if (contains(theFromFieldValue, theToFieldValues)) { + continue; + } + + BaseRuntimeElementCompositeDefinition compositeDefinition = (BaseRuntimeElementCompositeDefinition) childDefinition.getChildByName(field); + IBase newFieldValue = compositeDefinition.newInstance(); + terser.cloneInto(theFromFieldValue, newFieldValue, true); + + theToFieldValues.add(newFieldValue); + } + } + + private boolean contains(IBase theItem, List theItems) { + PrimitiveTypeComparingPredicate predicate = new PrimitiveTypeComparingPredicate(); + return theItems.stream().filter(i -> { + return predicate.test(i, theItem); + }).findFirst().isPresent(); + } + private void cloneAllExternalEidsIntoNewSourceResource(BaseRuntimeChildDefinition theSourceResourceIdentifier, IBase theSourceResource, IBase theNewSourceResource) { // FHIR choice types - fields within fhir where we have a choice of ids IFhirPath fhirPath = myFhirContext.newFhirPath(); @@ -246,7 +284,7 @@ public class PersonHelper { } BaseRuntimeElementCompositeDefinition childIdentifier = (BaseRuntimeElementCompositeDefinition) - theSourceResourceIdentifier.getChildByName("identifier"); + theSourceResourceIdentifier.getChildByName(FIELD_NAME_IDENTIFIER); IBase sourceResourceNewIdentifier = childIdentifier.newInstance(); terser.cloneInto(base, sourceResourceNewIdentifier, true); @@ -263,7 +301,7 @@ public class PersonHelper { // get a ref to the actual ID Field RuntimeResourceDefinition resourceDefinition = myFhirContext.getResourceDefinition(theSourceResource); - BaseRuntimeChildDefinition sourceResourceIdentifier = resourceDefinition.getChildByName("identifier"); + BaseRuntimeChildDefinition sourceResourceIdentifier = resourceDefinition.getChildByName(FIELD_NAME_IDENTIFIER); clearExternalEidsFromTheSourceResource(sourceResourceIdentifier, theSourceResource); } @@ -282,7 +320,7 @@ public class PersonHelper { } } - private T toId(CanonicalEID eid) { + private T toId(CanonicalEID eid) { switch (myFhirContext.getVersion().getVersion()) { case R4: return (T) eid.toR4(); @@ -330,16 +368,19 @@ public class PersonHelper { } public void mergeFields(IBaseResource theFromPerson, IBaseResource theToPerson) { - switch (myFhirContext.getVersion().getVersion()) { - case R4: - mergeR4PersonFields(theFromPerson, theToPerson); - break; - case DSTU3: - mergeDstu3PersonFields(theFromPerson, theToPerson); - break; - default: - throw new UnsupportedOperationException("Version not supported: " + myFhirContext.getVersion().getVersion()); - } + // TODO NG - Revisit when merge rules are defined + cloneCompositeField(theFromPerson, theToPerson, FIELD_NAME_IDENTIFIER); + +// switch (myFhirContext.getVersion().getVersion()) { +// case R4: +// mergeR4PersonFields(theFromPerson, theToPerson); +// break; +// case DSTU3: +// mergeDstu3PersonFields(theFromPerson, theToPerson); +// break; +// default: +// throw new UnsupportedOperationException("Version not supported: " + myFhirContext.getVersion().getVersion()); +// } } private void mergeR4PersonFields(IBaseResource theFromPerson, IBaseResource theToPerson) { diff --git a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicate.java b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicate.java new file mode 100644 index 00000000000..993d6bbeedf --- /dev/null +++ b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicate.java @@ -0,0 +1,70 @@ +package ca.uhn.fhir.empi.util; + +import ca.uhn.fhir.context.BaseRuntimeChildDefinition; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.util.FhirTerser; +import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IPrimitiveType; + +import java.lang.reflect.Field; +import java.util.function.BiPredicate; + +public class PrimitiveTypeComparingPredicate implements BiPredicate { + + @Override + public boolean test(Object theBase1, Object theBase2) { + if (theBase1 == null) { + return theBase2 == null; + } + if (theBase2 == null) { + return false; + } + if (!theBase1.getClass().equals(theBase2.getClass())) { + return false; + } + + for (Field f : theBase1.getClass().getDeclaredFields()) { + Class fieldClass = f.getType(); + + if (!IPrimitiveType.class.isAssignableFrom(fieldClass)) { + continue; + } + + IPrimitiveType val1, val2; + + f.setAccessible(true); + try { + val1 = (IPrimitiveType) f.get(theBase1); + val2 = (IPrimitiveType) f.get(theBase2); + } catch (Exception e) { + // swallow + continue; + } + + if (val1 == null && val2 == null) { + continue; + } + if (val1 == null && val2 != null) { + return false; + } + + Object actualVal1 = val1.getValue(); + Object actualVal2 = val2.getValue(); + + if (actualVal1 == null && actualVal2 == null) { + continue; + } + if (actualVal1 == null && actualVal2 != null) { + return false; + } + if (!actualVal1.equals(actualVal2)) { + return false; + } + + } + + return true; + } +} diff --git a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java new file mode 100644 index 00000000000..ff46be62495 --- /dev/null +++ b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java @@ -0,0 +1,11 @@ +package ca.uhn.fhir.empi.util; + +import static org.junit.jupiter.api.Assertions.*; + +class PrimitiveTypeComparingPredicateTest { + + // TODO NG ADD TESTS + // compare identifiers + // compare something scary to show that it failed + +} From 1d530c6517e942a0b8deca03e4c0399f2b4bd060 Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Thu, 12 Nov 2020 17:22:04 -0500 Subject: [PATCH 2/2] Added tests and cleaned test code --- .../jpa/empi/svc/EmpiPersonMergerSvcTest.java | 18 ++-- .../PrimitiveTypeComparingPredicateTest.java | 101 +++++++++++++++++- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java index 9e16a1a88a8..7f176ac5e94 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonMergerSvcTest.java @@ -299,11 +299,9 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { mergeGoldenPatients(); - assertResourceHasLinkCount(myToGoldenPatient, 3, (e) -> true); - assertResourceHasLinkCount(myFromGoldenPatient, 0, (e) -> true); + assertResourceHasLinkCount(myToGoldenPatient, 3); + assertResourceHasLinkCount(myFromGoldenPatient, 0); // TODO ENSURE PROPER LINK TYPES - // TODO MAKE IT MORE READABLE - assertEquals(3, myEmpiLinkDao.count()); } @@ -321,13 +319,10 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } - private void assertResourceHasAutoLinkCount(IBaseResource theResource, int theCount) { - assertResourceHasLinkCount(theResource, theCount, EmpiLink::isAuto); - } - private void assertResourceHasLinkCount(IBaseResource theResource, int theCount, Predicate thePredicate) { + private void assertResourceHasLinkCount(IBaseResource theResource, int theCount) { List links = myEmpiLinkDaoSvc.findEmpiLinksBySourceResource(theResource); - assertEquals(theCount, links.stream().filter(thePredicate).count()); + assertEquals(theCount, links.size()); } @Test @@ -344,6 +339,11 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { assertResourceHasAutoLinkCount(myToGoldenPatient, 3); } + private void assertResourceHasAutoLinkCount(Patient myToGoldenPatient, int theCount) { + List links = myEmpiLinkDaoSvc.findEmpiLinksBySourceResource(myToGoldenPatient); + assertEquals(theCount, links.stream().filter(EmpiLink::isAuto).count()); + } + @Test public void from123To123() { createEmpiLink(myFromGoldenPatient, myTargetPatient1); diff --git a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java index ff46be62495..a4e0b24744d 100644 --- a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java +++ b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/util/PrimitiveTypeComparingPredicateTest.java @@ -1,11 +1,104 @@ package ca.uhn.fhir.empi.util; -import static org.junit.jupiter.api.Assertions.*; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.util.FhirTerser; +import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.r4.model.Address; +import org.hl7.fhir.r4.model.BooleanType; +import org.hl7.fhir.r4.model.DateType; +import org.hl7.fhir.r4.model.Enumerations; +import org.hl7.fhir.r4.model.Identifier; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; class PrimitiveTypeComparingPredicateTest { - // TODO NG ADD TESTS - // compare identifiers - // compare something scary to show that it failed + private static FhirContext myFhirContext; + + private FhirTerser myTerser; + + private IBase myPositiveTest1; + + private IBase myPositiveTest2; + + private IBase myPositiveTest3; + + private IBase myNegativeTest; + + private PrimitiveTypeComparingPredicate cut = new PrimitiveTypeComparingPredicate(); + + @BeforeAll + public static void initContext() { + myFhirContext = FhirContext.forR4(); + } + + @BeforeEach + public void init() { + myTerser = myFhirContext.newTerser(); + + myPositiveTest1 = newPatient(); + myPositiveTest2 = newPatient(); + myPositiveTest3 = newPatient(); + + Patient patient = newPatient(); + patient.setActive(false); + patient.setMultipleBirth(new BooleanType(false)); + myNegativeTest = patient; + } + + private Patient newPatient() { + Patient patient; + patient = new Patient(); + patient.setActive(true); + patient.setGender(Enumerations.AdministrativeGender.FEMALE); + patient.setBirthDateElement(new DateType("1901-01-01")); + + Address address = new Address(); + address.addLine("Somwhere"); + address.setCity("Toronto"); + address.setCountry("Canada"); + patient.setAddress(Collections.singletonList(address)); + return patient; + } + + @Test + public void testNegativeMatchOnTheSameType() { + assertFalse(cut.test(myPositiveTest1, myNegativeTest)); + assertFalse(cut.test(myNegativeTest, myPositiveTest1)); + } + + @Test + public void testNegativeMatchOnDifferentTypes() { + Patient patient = newPatient(); + Identifier identifier = patient.addIdentifier(); + identifier.setValue("TEST_VALUE"); + assertFalse(cut.test(myNegativeTest, identifier)); + } + + @Test + public void testSymmetry() { + assertTrue(cut.test(myPositiveTest1, myPositiveTest2)); + assertTrue(cut.test(myPositiveTest2, myPositiveTest1)); + } + + @Test + public void testReflexivity() { + assertTrue(cut.test(myPositiveTest1, myPositiveTest1)); + } + + @Test + public void testTransitivity() { + assertTrue(cut.test(myPositiveTest1, myPositiveTest2)); + assertTrue(cut.test(myPositiveTest2, myPositiveTest3)); + assertTrue(cut.test(myPositiveTest1, myPositiveTest3)); + } + }