From 6a5b96d05f03e91f8a61cf9575b3e6293ef0f401 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 23 Jul 2020 15:52:12 -0700 Subject: [PATCH 1/6] Start with broken test for #1996 --- .../src/test/resources/empi/empi-rules.json | 13 +------------ .../empi/rules/config/EmpiRuleValidatorTest.java | 10 ++++++++++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json index 24d8fcb0b5e..9231853fa61 100644 --- a/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json +++ b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json @@ -1,18 +1,7 @@ { "version": "1", "candidateSearchParams": [ - { - "resourceType": "Patient", - "searchParams": ["birthdate"] - }, - { - "resourceType": "*", - "searchParams": ["identifier"] - }, - { - "resourceType": "Patient", - "searchParams": ["general-practitioner"] - } + ], "candidateFilterSearchParams": [ { diff --git a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java index b2766a6f70b..e9fe675c3f2 100644 --- a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java +++ b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java @@ -76,6 +76,16 @@ public class EmpiRuleValidatorTest extends BaseR4Test { } } + @Test + public void testMatcherEmptyCandidateSearchParams() throws IOException { + try { + setEmpiRuleJson("bad-rules-missing-candidate-search-params.json"); + fail(); + } catch (ConfigurationException e) { + assertThat(e.getMessage(), startsWith("Error in candidateSearchParams: Patient does not have a search parameter called 'foo'")); + } + } + @Test public void testMatcherBadFilter() throws IOException { try { From 2da64119bb2f171bd7c46faa49f1175c0bb028d4 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 23 Jul 2020 17:28:20 -0700 Subject: [PATCH 2/6] Resolve issue with test --- ...EmpiCandidateSearchCriteriaBuilderSvc.java | 19 ++++++++++--------- .../svc/candidate/EmpiCandidateSearchSvc.java | 15 ++++++++++----- 2 files changed, 20 insertions(+), 14 deletions(-) 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..38c7753941b 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 @@ -46,16 +46,17 @@ public class EmpiCandidateSearchCriteriaBuilderSvc { public Optional buildResourceQueryString(String theResourceType, IAnyResource theResource, List theFilterCriteria, 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 (criteria.isEmpty()) { - return Optional.empty(); + //If there is no candidateSearchParams, then we want to just use 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)); + } + }); } + 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..175a753a7dd 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 @@ -80,13 +80,18 @@ public class EmpiCandidateSearchSvc { List filterCriteria = buildFilterQuery(filterSearchParams, theResourceType); - for (EmpiResourceSearchParamJson resourceSearchParam : myEmpiConfig.getEmpiRules().getCandidateSearchParams()) { + List candidateSearchParams = myEmpiConfig.getEmpiRules().getCandidateSearchParams(); + if (candidateSearchParams == null || 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, From 698b18eb2f8a841f45dae03f1f5db0b36bbe5a8d Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 23 Jul 2020 18:01:08 -0700 Subject: [PATCH 3/6] Fix NPE in iterator, fix empi-rules.json, add extra tests --- ...EmpiCandidateSearchCriteriaBuilderSvc.java | 19 ++++++++++-- .../svc/candidate/EmpiCandidateSearchSvc.java | 6 ++-- .../provider/EmpiProviderMatchR4Test.java | 31 +++++++++++++++++++ .../EmpiProviderMergePersonsR4Test.java | 14 --------- ...CandidateSearchCriteriaBuilderSvcTest.java | 19 ++++++++++++ .../src/test/resources/empi/empi-rules.json | 13 +++++++- .../json/EmpiResourceSearchParamJson.java | 10 ++++-- 7 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMatchR4Test.java 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 38c7753941b..00a732fccd2 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,10 +44,10 @@ 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<>(); - //If there is no candidateSearchParams, then we want to just use the filters. + // 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. @@ -55,6 +56,20 @@ public class EmpiCandidateSearchCriteriaBuilderSvc { criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam)); } }); + + //TODO GGG/KHS, here's a question: What scenario would be actually want to return this empty optional. + //In the case where the resource being matched doesnt have any of the values that the EmpiResourceSearchParamJson wants? + //e.g. if i have a patient with name 'gary', but no birthdate, and i have a search param saying + // { + // "resourceType": "Patient", + // "searchParams": ["birthdate"] + // }, + // do I actually want it to return Zero candidates? if so, this following conditional is valid. However + // What if I still want to match that person? Will they be unmatchable since they have no birthdate? + + if (criteria.isEmpty()) { + return Optional.empty(); + } } criteria.addAll(theFilterCriteria); 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 175a753a7dd..ab6afb332ce 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,12 +75,12 @@ 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(); + + //If there are zero EmpiResourceSearchParamJson, we end up only making a single search, otherwise we + //must perform one search per EmpiResourceSearchParamJson. if (candidateSearchParams == null || candidateSearchParams.isEmpty()) { searchForIdsAndAddToMap(theResourceType, theResource, matchedPidsToResources, filterCriteria, null); } else { 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..88bf57c92e2 --- /dev/null +++ b/hapi-fhir-jpaserver-empi/src/test/java/ca/uhn/fhir/jpa/empi/provider/EmpiProviderMatchR4Test.java @@ -0,0 +1,31 @@ +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() { + 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/resources/empi/empi-rules.json b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json index 9231853fa61..24d8fcb0b5e 100644 --- a/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json +++ b/hapi-fhir-jpaserver-empi/src/test/resources/empi/empi-rules.json @@ -1,7 +1,18 @@ { "version": "1", "candidateSearchParams": [ - + { + "resourceType": "Patient", + "searchParams": ["birthdate"] + }, + { + "resourceType": "*", + "searchParams": ["identifier"] + }, + { + "resourceType": "Patient", + "searchParams": ["general-practitioner"] + } ], "candidateFilterSearchParams": [ { 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; } } From 31d0ed3ac19b5ef5ba777e1009bfa49dfa49e1f7 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 23 Jul 2020 18:09:10 -0700 Subject: [PATCH 4/6] remove question, put in PR instead --- .../EmpiCandidateSearchCriteriaBuilderSvc.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) 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 00a732fccd2..6185f76287c 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 @@ -57,17 +57,8 @@ public class EmpiCandidateSearchCriteriaBuilderSvc { } }); - //TODO GGG/KHS, here's a question: What scenario would be actually want to return this empty optional. - //In the case where the resource being matched doesnt have any of the values that the EmpiResourceSearchParamJson wants? - //e.g. if i have a patient with name 'gary', but no birthdate, and i have a search param saying - // { - // "resourceType": "Patient", - // "searchParams": ["birthdate"] - // }, - // do I actually want it to return Zero candidates? if so, this following conditional is valid. However - // What if I still want to match that person? Will they be unmatchable since they have no birthdate? - if (criteria.isEmpty()) { + //TODO GGG/KHS, re-evaluate whether we should early drop here. return Optional.empty(); } } From 7fd690f83005c0c56ef4f74e598a512678c9bd99 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 24 Jul 2020 10:42:21 -0700 Subject: [PATCH 5/6] Add test in provider with different empi json rules --- ...EmpiCandidateSearchCriteriaBuilderSvc.java | 1 - .../svc/candidate/EmpiCandidateSearchSvc.java | 2 +- .../ca/uhn/fhir/jpa/empi/BaseEmpiR4Test.java | 3 +- .../jpa/empi/provider/BaseProviderR4Test.java | 27 +++++++++++++++ .../provider/EmpiProviderMatchR4Test.java | 15 +++++++- .../fhir/jpa/empi/svc/EmpiLinkSvcTest.java | 4 ++- .../jpa/empi/svc/EmpiPersonMergerSvcTest.java | 3 +- .../empi/empty-candidate-search-params.json | 34 +++++++++++++++++++ .../ca/uhn/fhir/jpa/test/BaseJpaTest.java | 3 +- 9 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 hapi-fhir-jpaserver-empi/src/test/resources/empi/empty-candidate-search-params.json 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 6185f76287c..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 @@ -56,7 +56,6 @@ public class EmpiCandidateSearchCriteriaBuilderSvc { criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam)); } }); - if (criteria.isEmpty()) { //TODO GGG/KHS, re-evaluate whether we should early drop here. return Optional.empty(); 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 ab6afb332ce..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 @@ -81,7 +81,7 @@ public class EmpiCandidateSearchSvc { //If there are zero EmpiResourceSearchParamJson, we end up only making a single search, otherwise we //must perform one search per EmpiResourceSearchParamJson. - if (candidateSearchParams == null || candidateSearchParams.isEmpty()) { + if (candidateSearchParams.isEmpty()) { searchForIdsAndAddToMap(theResourceType, theResource, matchedPidsToResources, filterCriteria, null); } else { for (EmpiResourceSearchParamJson resourceSearchParam : candidateSearchParams) { 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 index 88bf57c92e2..d8f4dca618b 100644 --- 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 @@ -18,7 +18,20 @@ public class EmpiProviderMatchR4Test extends BaseProviderR4Test { } @Test - public void testMatch() { + 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); 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(); From df6f028768f526b248bdcc142e69fbea1efd201c Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 24 Jul 2020 11:52:32 -0700 Subject: [PATCH 6/6] It is allowed to have empty candidate search params, so validator test is, ironically, invalid --- .../fhir/empi/rules/config/EmpiRuleValidatorTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java index e9fe675c3f2..b2766a6f70b 100644 --- a/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java +++ b/hapi-fhir-server-empi/src/test/java/ca/uhn/fhir/empi/rules/config/EmpiRuleValidatorTest.java @@ -76,16 +76,6 @@ public class EmpiRuleValidatorTest extends BaseR4Test { } } - @Test - public void testMatcherEmptyCandidateSearchParams() throws IOException { - try { - setEmpiRuleJson("bad-rules-missing-candidate-search-params.json"); - fail(); - } catch (ConfigurationException e) { - assertThat(e.getMessage(), startsWith("Error in candidateSearchParams: Patient does not have a search parameter called 'foo'")); - } - } - @Test public void testMatcherBadFilter() throws IOException { try {