Merge pull request #1997 from jamesagnew/1996-empi-match-bug

Allow empty candidateSearchParams in EMPI module config.
This commit is contained in:
Tadgh 2020-07-24 14:22:29 -07:00 committed by GitHub
commit ff1d5221b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 167 additions and 37 deletions

View File

@ -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<String> buildResourceQueryString(String theResourceType, IAnyResource theResource, List<String> theFilterCriteria, EmpiResourceSearchParamJson resourceSearchParam) {
public Optional<String> buildResourceQueryString(String theResourceType, IAnyResource theResource, List<String> theFilterCriteria, @Nullable EmpiResourceSearchParamJson resourceSearchParam) {
List<String> criteria = new ArrayList<>();
resourceSearchParam.iterator().forEachRemaining(searchParam -> {
//to compare it to all known PERSON objects, using the overlapping search parameters that they have.
List<String> 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<String> 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));
}

View File

@ -75,18 +75,23 @@ public class EmpiCandidateSearchSvc {
*/
public Collection<IAnyResource> findCandidates(String theResourceType, IAnyResource theResource) {
Map<Long, IAnyResource> matchedPidsToResources = new HashMap<>();
List<EmpiFilterSearchParamJson> filterSearchParams = myEmpiConfig.getEmpiRules().getCandidateFilterSearchParams();
List<String> filterCriteria = buildFilterQuery(filterSearchParams, theResourceType);
List<EmpiResourceSearchParamJson> 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,

View File

@ -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();

View File

@ -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);
}
}

View File

@ -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());
}
}

View File

@ -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);

View File

@ -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<String> 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<String> filterParams = Collections.singletonList("active=true");
Optional<String> result = myEmpiCandidateSearchCriteriaBuilderSvc.buildResourceQueryString("Patient", patient, filterParams, null);
assertThat(result.isPresent(), is(true));
assertThat(result.get(), is(equalTo("Patient?active=true")));
}
}

View File

@ -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();
}

View File

@ -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();
}

View File

@ -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"
}

View File

@ -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();

View File

@ -46,14 +46,18 @@ public class EmpiResourceSearchParamJson implements IModelJson, Iterable<String>
}
public Iterator<String> iterator() {
return mySearchParams.iterator();
return getSearchParams().iterator();
}
public EmpiResourceSearchParamJson addSearchParam(String theSearchParam) {
getSearchParams().add(theSearchParam);
return this;
}
private List<String> getSearchParams() {
if (mySearchParams == null) {
mySearchParams = new ArrayList<>();
}
mySearchParams.add(theSearchParam);
return this;
return mySearchParams;
}
}