Empi 57 filter out inactive person (#1903)

* begin with failing test

* test passes

* pre-review cleanup

* little fix

* fix intermittent

hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/EmpiPersonFindingSvc.java

@@ -93,8 +93,9 @@ public class EmpiPersonFindingSvc {
 		List<CanonicalEID> eidFromResource = myEIDHelper.getExternalEid(theBaseResource);
 		if (!eidFromResource.isEmpty()) {
 			for (CanonicalEID eid : eidFromResource) {
-				IBaseResource foundPerson = myEmpiResourceDaoSvc.searchPersonByEid(eid.getValue());
+				Optional<IAnyResource> oFoundPerson = myEmpiResourceDaoSvc.searchPersonByEid(eid.getValue());
-				if (foundPerson != null) {
+				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);

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<IAnyResource> 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;
-		} else {
-			return (IAnyResource) search.getResources(0, 1).get(0);
-		}
-	}
 
+		// 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<IBaseResource> 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 Optional.of((IAnyResource) list.get(0));
+		}
+	}
 }

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;

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

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() {

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<IAnyResource> 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<IAnyResource> foundPerson = myResourceDaoSvc.searchPersonByEid(TEST_EID);
+		assertTrue(foundPerson.isPresent());
+		assertThat(foundPerson.get().getIdElement().toUnqualifiedVersionless().getValue(), is(goodPerson.getIdElement().toUnqualifiedVersionless().getValue()));
+	}
+}