From 82b4864d793ab133040c396ca7dbf055ec2631f4 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 9 Jun 2020 09:52:40 -0400 Subject: [PATCH] Empi 57 filter out inactive person (#1903) * begin with failing test * test passes * pre-review cleanup * little fix * fix intermittent --- .../jpa/empi/svc/EmpiPersonFindingSvc.java | 5 +- .../fhir/jpa/empi/svc/EmpiResourceDaoSvc.java | 36 ++++++++++--- .../ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java | 5 ++ .../fhir/jpa/empi/dao/EmpiLinkDaoSvcTest.java | 4 +- .../EmpiMatchLinkSvcMultipleEidModeTest.java | 11 ++-- .../jpa/empi/svc/EmpiResourceDaoSvcTest.java | 52 +++++++++++++++++++ 6 files changed, 97 insertions(+), 16 deletions(-) create mode 100644 hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonFindingSvc.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonFindingSvc.java index 14882e3807a..c0b4cdd5eac 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonFindingSvc.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonFindingSvc.java @@ -93,8 +93,9 @@ public class EmpiPersonFindingSvc { List eidFromResource = myEIDHelper.getExternalEid(theBaseResource); if (!eidFromResource.isEmpty()) { for (CanonicalEID eid : eidFromResource) { - IBaseResource foundPerson = myEmpiResourceDaoSvc.searchPersonByEid(eid.getValue()); - if (foundPerson != null) { + Optional oFoundPerson = myEmpiResourceDaoSvc.searchPersonByEid(eid.getValue()); + if (oFoundPerson.isPresent()) { + IAnyResource foundPerson = oFoundPerson.get(); Long pidOrNull = myIdHelperService.getPidOrNull(foundPerson); MatchedPersonCandidate mpc = new MatchedPersonCandidate(new ResourcePersistentId(pidOrNull), EmpiMatchResultEnum.MATCH); ourLog.debug("Matched {} by EID {}", foundPerson.getIdElement(), eid); diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvc.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvc.java index e0848e49e45..5e9b5084898 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvc.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvc.java @@ -20,7 +20,9 @@ package ca.uhn.fhir.jpa.empi.svc; * #L% */ +import ca.uhn.fhir.empi.api.EmpiConstants; import ca.uhn.fhir.empi.api.IEmpiSettings; +import ca.uhn.fhir.empi.util.EmpiUtil; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; @@ -28,6 +30,7 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; 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.exceptions.InternalErrorException; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -35,9 +38,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import javax.annotation.PostConstruct; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; @Service public class EmpiResourceDaoSvc { + private static final int MAX_MATCHING_PERSONS = 1000; @Autowired DaoRegistry myDaoRegistry; @Autowired @@ -74,16 +81,33 @@ public class EmpiResourceDaoSvc { return (IAnyResource) myPersonDao.readByPid(thePersonPid); } - public IAnyResource searchPersonByEid(String theEidFromResource) { + public Optional searchPersonByEid(String theEid) { SearchParameterMap map = new SearchParameterMap(); map.setLoadSynchronous(true); - map.add("identifier", new TokenParam(myEmpiConfig.getEmpiRules().getEnterpriseEIDSystem(), theEidFromResource)); + map.add("identifier", new TokenParam(myEmpiConfig.getEmpiRules().getEnterpriseEIDSystem(), theEid)); + map.add("active", new TokenParam("true")); IBundleProvider search = myPersonDao.search(map); - if (search.isEmpty()) { - return null; + + // Could add the meta tag to the query, but it's probably more efficient to filter on it afterwards since in practice + // it will always be present. + List list = search.getResources(0, MAX_MATCHING_PERSONS).stream() + .filter(EmpiUtil::isEmpiManaged) + .collect(Collectors.toList()); + + if (list.isEmpty()) { + return Optional.empty(); + } else if (list.size() > 1) { + throw new InternalErrorException("Found more than one active " + + EmpiConstants.CODE_HAPI_EMPI_MANAGED + + " Person with EID " + + theEid + + ": " + + list.get(0).getIdElement().getValue() + + ", " + + list.get(1).getIdElement().getValue() + ); } else { - return (IAnyResource) search.getResources(0, 1).get(0); + return Optional.of((IAnyResource) list.get(0)); } } - } 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 e28078a3ae5..de9a7aadd01 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 @@ -256,6 +256,11 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { return thePatient; } + protected Person addExternalEID(Person thePerson, String theEID) { + thePerson.addIdentifier().setSystem(myEmpiConfig.getEmpiRules().getEnterpriseEIDSystem()).setValue(theEID); + return thePerson; + } + protected Patient clearExternalEIDs(Patient thePatient) { thePatient.getIdentifier().removeIf(theIdentifier -> theIdentifier.getSystem().equalsIgnoreCase(myEmpiConfig.getEmpiRules().getEnterpriseEIDSystem())); return thePatient; diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvcTest.java index 5d9b8d4f57a..0209a4bb297 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/dao/EmpiLinkDaoSvcTest.java @@ -12,8 +12,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; public class EmpiLinkDaoSvcTest extends BaseEmpiR4Test { @Autowired @@ -27,7 +27,7 @@ public class EmpiLinkDaoSvcTest extends BaseEmpiR4Test { myEmpiLinkDaoSvc.save(empiLink); assertThat(empiLink.getCreated(), is(notNullValue())); assertThat(empiLink.getUpdated(), is(notNullValue())); - assertEquals(empiLink.getCreated(), empiLink.getUpdated()); + assertTrue(empiLink.getUpdated().getTime() - empiLink.getCreated().getTime() < 1000); } @Test diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiMatchLinkSvcMultipleEidModeTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiMatchLinkSvcMultipleEidModeTest.java index 04020c2bb57..3f8db3f40fb 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiMatchLinkSvcMultipleEidModeTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiMatchLinkSvcMultipleEidModeTest.java @@ -1,15 +1,14 @@ package ca.uhn.fhir.jpa.empi.svc; import ca.uhn.fhir.empi.api.EmpiConstants; -import ca.uhn.fhir.empi.api.IEmpiLinkSvc; import ca.uhn.fhir.empi.model.CanonicalEID; import ca.uhn.fhir.empi.util.EIDHelper; -import ca.uhn.fhir.empi.util.PersonHelper; import ca.uhn.fhir.jpa.empi.BaseEmpiR4Test; import ca.uhn.fhir.jpa.entity.EmpiLink; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Person; +import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -33,12 +32,12 @@ import static org.slf4j.LoggerFactory.getLogger; public class EmpiMatchLinkSvcMultipleEidModeTest extends BaseEmpiR4Test { private static final Logger ourLog = getLogger(EmpiMatchLinkSvcMultipleEidModeTest.class); @Autowired - IEmpiLinkSvc myEmpiLinkSvc; - @Autowired private EIDHelper myEidHelper; - @Autowired - private PersonHelper myPersonHelper; + @Before + public void before() { + super.loadEmpiSearchParameters(); + } @Test public void testIncomingPatientWithEIDThatMatchesPersonWithHapiEidAddsExternalEidsToPerson() { 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 new file mode 100644 index 00000000000..ac0ee6943fb --- /dev/null +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiResourceDaoSvcTest.java @@ -0,0 +1,52 @@ +package ca.uhn.fhir.jpa.empi.svc; + +import ca.uhn.fhir.jpa.empi.BaseEmpiR4Test; +import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.r4.model.Person; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.Optional; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +public class EmpiResourceDaoSvcTest extends BaseEmpiR4Test { + private static final String TEST_EID = "TEST_EID"; + @Autowired + EmpiResourceDaoSvc myResourceDaoSvc; + + @Before + public void before() { + super.loadEmpiSearchParameters(); + } + + @Test + public void testSearchPersonByEidExcludesInactive() { + Person goodPerson = addExternalEID(createPerson(), TEST_EID); + myPersonDao.update(goodPerson); + + Person badPerson = addExternalEID(createPerson(), TEST_EID); + badPerson.setActive(false); + myPersonDao.update(badPerson); + + Optional foundPerson = myResourceDaoSvc.searchPersonByEid(TEST_EID); + assertTrue(foundPerson.isPresent()); + assertThat(foundPerson.get().getIdElement().toUnqualifiedVersionless().getValue(), is(goodPerson.getIdElement().toUnqualifiedVersionless().getValue())); + } + + @Test + public void testSearchPersonByEidExcludesNonEmpiManaged() { + Person goodPerson = addExternalEID(createPerson(), TEST_EID); + myPersonDao.update(goodPerson); + + Person badPerson = addExternalEID(createPerson(new Person(), false), TEST_EID); + myPersonDao.update(badPerson); + + Optional foundPerson = myResourceDaoSvc.searchPersonByEid(TEST_EID); + assertTrue(foundPerson.isPresent()); + assertThat(foundPerson.get().getIdElement().toUnqualifiedVersionless().getValue(), is(goodPerson.getIdElement().toUnqualifiedVersionless().getValue())); + } +}