diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchCriteriaBuilderSvc.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchCriteriaBuilderSvc.java index 00c93ce4a00..b2e033b60b0 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchCriteriaBuilderSvc.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchCriteriaBuilderSvc.java @@ -27,6 +27,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -43,19 +44,24 @@ public class EmpiCandidateSearchCriteriaBuilderSvc { * Patient?active=true&name.given=Gary,Grant&name.family=Graham */ @Nonnull - public Optional buildResourceQueryString(String theResourceType, IAnyResource theResource, List theFilterCriteria, EmpiResourceSearchParamJson resourceSearchParam) { + public Optional buildResourceQueryString(String theResourceType, IAnyResource theResource, List theFilterCriteria, @Nullable EmpiResourceSearchParamJson resourceSearchParam) { List criteria = new ArrayList<>(); - resourceSearchParam.iterator().forEachRemaining(searchParam -> { - //to compare it to all known PERSON objects, using the overlapping search parameters that they have. - List valuesFromResourceForSearchParam = myEmpiSearchParamSvc.getValueFromResourceForSearchParam(theResource, searchParam); - if (!valuesFromResourceForSearchParam.isEmpty()) { - criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam)); + // If there are candidate search params, then make use of them, otherwise, search with only the filters. + if (resourceSearchParam != null) { + resourceSearchParam.iterator().forEachRemaining(searchParam -> { + //to compare it to all known PERSON objects, using the overlapping search parameters that they have. + List valuesFromResourceForSearchParam = myEmpiSearchParamSvc.getValueFromResourceForSearchParam(theResource, searchParam); + if (!valuesFromResourceForSearchParam.isEmpty()) { + criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam)); + } + }); + if (criteria.isEmpty()) { + //TODO GGG/KHS, re-evaluate whether we should early drop here. + return Optional.empty(); } - }); - if (criteria.isEmpty()) { - return Optional.empty(); } + criteria.addAll(theFilterCriteria); return Optional.of(theResourceType + "?" + String.join("&", criteria)); } diff --git a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchSvc.java b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchSvc.java index e78ba51cb27..bae3f518447 100644 --- a/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchSvc.java +++ b/hapi-fhir-jpaserver-empi/src/main/java/ca/uhn/fhir/jpa/empi/svc/candidate/EmpiCandidateSearchSvc.java @@ -75,18 +75,23 @@ public class EmpiCandidateSearchSvc { */ public Collection findCandidates(String theResourceType, IAnyResource theResource) { Map matchedPidsToResources = new HashMap<>(); - List filterSearchParams = myEmpiConfig.getEmpiRules().getCandidateFilterSearchParams(); - List filterCriteria = buildFilterQuery(filterSearchParams, theResourceType); + List candidateSearchParams = myEmpiConfig.getEmpiRules().getCandidateSearchParams(); - for (EmpiResourceSearchParamJson resourceSearchParam : myEmpiConfig.getEmpiRules().getCandidateSearchParams()) { + //If there are zero EmpiResourceSearchParamJson, we end up only making a single search, otherwise we + //must perform one search per EmpiResourceSearchParamJson. + if (candidateSearchParams.isEmpty()) { + searchForIdsAndAddToMap(theResourceType, theResource, matchedPidsToResources, filterCriteria, null); + } else { + for (EmpiResourceSearchParamJson resourceSearchParam : candidateSearchParams) { - if (!isSearchParamForResource(theResourceType, resourceSearchParam)) { - continue; + if (!isSearchParamForResource(theResourceType, resourceSearchParam)) { + continue; + } + + searchForIdsAndAddToMap(theResourceType, theResource, matchedPidsToResources, filterCriteria, resourceSearchParam); } - - searchForIdsAndAddToMap(theResourceType, theResource, matchedPidsToResources, filterCriteria, resourceSearchParam); } //Obviously we don't want to consider the freshly added resource as a potential candidate. //Sometimes, we are running this function on a resource that has not yet been persisted, 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 151026e6116..e25627127ee 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 @@ -50,6 +50,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import javax.annotation.Nonnull; +import java.io.IOException; import java.util.Collections; import java.util.Date; import java.util.List; @@ -105,7 +106,7 @@ abstract public class BaseEmpiR4Test extends BaseJpaR4Test { @Override @AfterEach - public void after() { + public void after() throws IOException { myEmpiLinkDao.deleteAll(); assertEquals(0, myEmpiLinkDao.count()); super.after(); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/BaseProviderR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/BaseProviderR4Test.java index d1848efb3c8..4cd59c59be6 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/BaseProviderR4Test.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/BaseProviderR4Test.java @@ -4,11 +4,20 @@ import ca.uhn.fhir.empi.api.IEmpiLinkQuerySvc; import ca.uhn.fhir.empi.api.IEmpiLinkUpdaterSvc; import ca.uhn.fhir.empi.api.IEmpiMatchFinderSvc; import ca.uhn.fhir.empi.api.IEmpiPersonMergerSvc; +import ca.uhn.fhir.empi.api.IEmpiSettings; import ca.uhn.fhir.empi.provider.EmpiProviderR4; +import ca.uhn.fhir.empi.rules.config.EmpiSettings; import ca.uhn.fhir.jpa.empi.BaseEmpiR4Test; import ca.uhn.fhir.validation.IResourceLoader; +import com.google.common.base.Charsets; +import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.io.DefaultResourceLoader; +import org.springframework.core.io.Resource; + +import java.io.IOException; public abstract class BaseProviderR4Test extends BaseEmpiR4Test { EmpiProviderR4 myEmpiProviderR4; @@ -22,9 +31,27 @@ public abstract class BaseProviderR4Test extends BaseEmpiR4Test { private IEmpiLinkQuerySvc myEmpiLinkQuerySvc; @Autowired private IResourceLoader myResourceLoader; + @Autowired + private IEmpiSettings myEmpiSettings; + + private String defaultScript; + + protected void setEmpiRuleJson(String theString) throws IOException { + DefaultResourceLoader resourceLoader = new DefaultResourceLoader(); + Resource resource = resourceLoader.getResource(theString); + String json = IOUtils.toString(resource.getInputStream(), Charsets.UTF_8); + ((EmpiSettings)myEmpiSettings).getScriptText(); + ((EmpiSettings)myEmpiSettings).setScriptText(json); + } @BeforeEach public void before() { myEmpiProviderR4 = new EmpiProviderR4(myFhirContext, myEmpiMatchFinderSvc, myPersonMergerSvc, myEmpiLinkUpdaterSvc, myEmpiLinkQuerySvc, myResourceLoader); + defaultScript = ((EmpiSettings)myEmpiSettings).getScriptText(); + } + @AfterEach + public void after() throws IOException { + super.after(); + ((EmpiSettings)myEmpiSettings).setScriptText(defaultScript); } } diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMatchR4Test.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMatchR4Test.java new file mode 100644 index 00000000000..d8f4dca618b --- /dev/null +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMatchR4Test.java @@ -0,0 +1,44 @@ +package ca.uhn.fhir.jpa.empi.provider; + +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class EmpiProviderMatchR4Test extends BaseProviderR4Test { + + + @Override + @BeforeEach + public void before() { + super.before(); + super.loadEmpiSearchParameters(); + } + + @Test + public void testMatch() throws Exception { + Patient jane = buildJanePatient(); + jane.setActive(true); + Patient createdJane = createPatient(jane); + Patient newJane = buildJanePatient(); + + Bundle result = myEmpiProviderR4.match(newJane); + assertEquals(1, result.getEntry().size()); + assertEquals(createdJane.getId(), result.getEntryFirstRep().getResource().getId()); + } + + @Test + public void testMatchWithEmptySearchParamCandidates() throws Exception { + setEmpiRuleJson("empi/empty-candidate-search-params.json"); + Patient jane = buildJanePatient(); + jane.setActive(true); + Patient createdJane = createPatient(jane); + Patient newJane = buildJanePatient(); + + Bundle result = myEmpiProviderR4.match(newJane); + assertEquals(1, result.getEntry().size()); + assertEquals(createdJane.getId(), result.getEntryFirstRep().getResource().getId()); + } +} 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 7c3d4068e5e..7c03cee7fb6 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 @@ -5,8 +5,6 @@ import ca.uhn.fhir.empi.api.EmpiMatchResultEnum; import ca.uhn.fhir.empi.util.AssuranceLevelUtil; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; -import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Person; import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.BeforeEach; @@ -39,18 +37,6 @@ public class EmpiProviderMergePersonsR4Test extends BaseProviderR4Test { myToPersonId = new StringType(myToPerson.getIdElement().getValue()); } - @Test - public void testMatch() { - Patient jane = buildJanePatient(); - jane.setActive(true); - Patient createdJane = createPatient(jane); - Patient newJane = buildJanePatient(); - - Bundle result = myEmpiProviderR4.match(newJane); - assertEquals(1, result.getEntry().size()); - assertEquals(createdJane.getId(), result.getEntryFirstRep().getResource().getId()); - } - @Test public void testMerge() { Person mergedPerson = myEmpiProviderR4.mergePersons(myFromPersonId, myToPersonId, myRequestDetails); diff --git a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiCandidateSearchCriteriaBuilderSvcTest.java b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiCandidateSearchCriteriaBuilderSvcTest.java index 8ca038b4127..fc7a1ee1930 100644 --- a/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiCandidateSearchCriteriaBuilderSvcTest.java +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/svc/EmpiCandidateSearchCriteriaBuilderSvcTest.java @@ -9,11 +9,13 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import java.util.Collections; +import java.util.List; import java.util.Optional; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -67,4 +69,21 @@ public class EmpiCandidateSearchCriteriaBuilderSvcTest extends BaseEmpiR4Test { assertTrue(result.isPresent()); assertEquals(result.get(), "Patient?identifier=urn:oid:1.2.36.146.595.217.0.1|12345"); } + + @Test + public void testOmittingCandidateSearchParamsIsAllowed() { + Patient patient = new Patient(); + Optional result = myEmpiCandidateSearchCriteriaBuilderSvc.buildResourceQueryString("Patient", patient, Collections.emptyList(), null); + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), is(equalTo("Patient?"))); + } + + @Test + public void testEmptyCandidateSearchParamsWorksInConjunctionWithFilterParams() { + Patient patient = new Patient(); + List filterParams = Collections.singletonList("active=true"); + Optional result = myEmpiCandidateSearchCriteriaBuilderSvc.buildResourceQueryString("Patient", patient, filterParams, null); + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), is(equalTo("Patient?active=true"))); + } } 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 d249fa5849e..98591556def 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 @@ -14,6 +14,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.io.IOException; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -29,7 +31,7 @@ public class EmpiLinkSvcTest extends BaseEmpiR4Test { @Override @AfterEach - public void after() { + public void after() throws IOException { myExpungeEverythingService.expungeEverythingByType(EmpiLink.class); super.after(); } 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 3ef04c84fa5..a399319dc15 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 @@ -25,6 +25,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.io.IOException; import java.util.Collections; import java.util.List; import java.util.UUID; @@ -82,7 +83,7 @@ public class EmpiPersonMergerSvcTest extends BaseEmpiR4Test { @Override @AfterEach - public void after() { + public void after() throws IOException { myInterceptorService.unregisterInterceptor(myEmpiStorageInterceptor); super.after(); } diff --git a/hapi-fhir-jpaserver-empi/src/test/resources/empi/empty-candidate-search-params.json b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empty-candidate-search-params.json new file mode 100644 index 00000000000..7b43177798c --- /dev/null +++ b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empty-candidate-search-params.json @@ -0,0 +1,34 @@ +{ + "version": "1", + "candidateSearchParams": [], + "candidateFilterSearchParams": [ + { + "resourceType": "*", + "searchParam": "active", + "fixedValue": "true" + } + ], + "matchFields": [ + { + "name": "cosine-given-name", + "resourceType": "*", + "resourcePath": "name.given", + "metric": "COSINE", + "matchThreshold": 0.8, + "exact": true + }, + { + "name": "jaro-last-name", + "resourceType": "*", + "resourcePath": "name.family", + "metric": "JARO_WINKLER", + "matchThreshold": 0.8, + "exact": true + } + ], + "matchResultMap": { + "cosine-given-name" : "POSSIBLE_MATCH", + "cosine-given-name,jaro-last-name" : "MATCH" + }, + "eidSystem": "http://company.io/fhir/NamingSystem/custom-eid-system" +} diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java index 8efab9f3ebc..29ee25dbcb6 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java @@ -40,6 +40,7 @@ import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionTemplate; +import java.io.IOException; import java.util.concurrent.Callable; @TestPropertySource(properties = { @@ -71,7 +72,7 @@ public abstract class BaseJpaTest { MemoryCacheService myMemoryCacheService; @AfterEach - public void after() { + public void after() throws IOException { ourLog.info("\n --- @After ---"); myExpungeEverythingService.expungeEverything(null); myMemoryCacheService.invalidateAllCaches(); diff --git a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/rules/json/EmpiResourceSearchParamJson.java b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/rules/json/EmpiResourceSearchParamJson.java index 9cc3e78fec8..6837ae4e56a 100644 --- a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/rules/json/EmpiResourceSearchParamJson.java +++ b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/rules/json/EmpiResourceSearchParamJson.java @@ -46,14 +46,18 @@ public class EmpiResourceSearchParamJson implements IModelJson, Iterable } public Iterator iterator() { - return mySearchParams.iterator(); + return getSearchParams().iterator(); } public EmpiResourceSearchParamJson addSearchParam(String theSearchParam) { + getSearchParams().add(theSearchParam); + return this; + } + + private List getSearchParams() { if (mySearchParams == null) { mySearchParams = new ArrayList<>(); } - mySearchParams.add(theSearchParam); - return this; + return mySearchParams; } }