Fix NPE in iterator, fix empi-rules.json, add extra tests

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

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<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();
+
+		//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 {

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

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

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<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")));
+	}
 }

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": [
 		{

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<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 mySearchParams;
-		return this;
 	}
 }