:nickname Qualifier Support for Custom SearchParameters (#3969)

* - add failing test

* 4977 Allow any search parameter to have a nickname qualifier.

* IT test to ensure nickname expansion is working on custom SearchParameters

* capture log output in tests

* changelog

* code review changes

Co-authored-by: nathaniel.doef <nathaniel.doef@smilecdr.com>
Co-authored-by: kylejule <kyle.jule@smilecdr.com>

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java

@@ -31,11 +31,15 @@ import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import static org.apache.commons.lang3.StringUtils.defaultString;
 
 public class StringParam extends BaseParam implements IQueryParameterType {
 
+	private static final Logger ourLog = LoggerFactory.getLogger(StringParam.class);
+
 	private boolean myContains;
 	private boolean myExact;
 	private String myValue;
@@ -102,11 +106,11 @@ public class StringParam extends BaseParam implements IQueryParameterType {
 	@Override
 	void doSetValueAsQueryToken(FhirContext theContext, String theParamName, String theQualifier, String theValue) {
 		if (Constants.PARAMQUALIFIER_NICKNAME.equals(theQualifier)) {
-			if ("name".equals(theParamName) || "given".equals(theParamName)) {
+			myNicknameExpand = true;
-				myNicknameExpand = true;
+			theQualifier = "";
-				theQualifier = "";
+
-			} else {
+			if (!("name".equals(theParamName) || "given".equals(theParamName))){
-				throw new InvalidRequestException(Msg.code(2077) + "Modifier " + Constants.PARAMQUALIFIER_NICKNAME + " may only be used with 'name' and 'given' search parameters");
+				ourLog.debug(":nickname qualifier was assigned to a search parameter other than one of the intended parameters \"name\" and \"given\"");
 			}
 		}
 

hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/StringParamTest.java

@@ -2,10 +2,45 @@ package ca.uhn.fhir.rest.param;
 
 import static org.junit.jupiter.api.Assertions.*;
 
+import ca.uhn.fhir.context.FhirContext;
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import ch.qos.logback.classic.Logger;
+import org.slf4j.LoggerFactory;
 
+import java.util.List;
+import java.util.stream.Collectors;
+
+@ExtendWith(MockitoExtension.class)
 public class StringParamTest {
 
+	private static final Logger ourLog = (Logger) LoggerFactory.getLogger(StringParam.class);
+	private ListAppender<ILoggingEvent> myListAppender = new ListAppender<>();
+
+	@Mock
+	private FhirContext myContext;
+
+	@BeforeEach
+	public void beforeEach(){
+		myListAppender = new ListAppender<>();
+		myListAppender.start();
+		ourLog.addAppender(myListAppender);
+	}
+
+	@AfterEach
+	public void afterEach(){
+		myListAppender.stop();
+	}
+
 	@Test
 	public void testEquals() {
 		StringParam input = new StringParam("foo", true);
@@ -15,5 +50,47 @@ public class StringParamTest {
 		assertFalse(input.equals(""));
 		assertFalse(input.equals(new StringParam("foo", false)));
 	}
+
+	@Test
+	public void doSetValueAsQueryToken_withCustomSearchParameterAndNicknameQualifier_enablesNicknameExpansion(){
+		String customSearchParamName = "someCustomSearchParameter";
+		StringParam stringParam = new StringParam();
+		stringParam.doSetValueAsQueryToken(myContext, customSearchParamName, ":nickname", "John");
+		assertNicknameQualifierSearchParameterIsValid(stringParam, "John");
+		assertNicknameWarningLogged(true);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {"name", "given"})
+	public void doSetValueAsQueryToken_withPredefinedSearchParametersAndNicknameQualifier_enablesNicknameExpansion(String theSearchParameterName){
+		StringParam stringParam = new StringParam();
+		stringParam.doSetValueAsQueryToken(myContext, theSearchParameterName, ":nickname", "John");
+		assertNicknameQualifierSearchParameterIsValid(stringParam, "John");
+		assertNicknameWarningLogged(false);
+	}
+
+	private void assertNicknameQualifierSearchParameterIsValid(StringParam theStringParam, String theExpectedValue){
+		assertTrue(theStringParam.isNicknameExpand());
+		assertFalse(theStringParam.isExact());
+		assertFalse(theStringParam.isContains());
+		assertEquals(theExpectedValue, theStringParam.getValue());
+	}
+
+	private void assertNicknameWarningLogged(boolean theWasLogged){
+		String expectedMessage = ":nickname qualifier was assigned to a search parameter other than one of the intended parameters \"name\" and \"given\"";
+		Level expectedLevel = Level.DEBUG;
+		List<ILoggingEvent> warningLogs = myListAppender
+			.list
+			.stream()
+			.filter(event -> expectedMessage.equals(event.getFormattedMessage()))
+			.filter(event -> expectedLevel.equals(event.getLevel()))
+			.collect(Collectors.toList());
+
+		if (theWasLogged) {
+			assertEquals(1, warningLogs.size());
+		} else {
+			assertTrue(warningLogs.isEmpty());
+		}
+	}
 	
 }

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3972-nickname-qualifier-support-for-any-string-searchparameters.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 3972
+jira: SMILE-4977
+title: "Previously, the `:nickname` qualifier only worked with the predefined `name` and `given` SearchParameters.
+This has been fixed and now the `:nickname` qualifier can be used with any string SearchParameters."

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderCustomSearchParamR4Test.java

@@ -36,6 +36,7 @@ import org.hl7.fhir.r4.model.CodeType;
 import org.hl7.fhir.r4.model.Enumerations;
 import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender;
 import org.hl7.fhir.r4.model.ExplanationOfBenefit;
+import org.hl7.fhir.r4.model.HumanName;
 import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Observation.ObservationStatus;
 import org.hl7.fhir.r4.model.Patient;
@@ -43,6 +44,7 @@ import org.hl7.fhir.r4.model.Practitioner;
 import org.hl7.fhir.r4.model.Reference;
 import org.hl7.fhir.r4.model.SearchParameter;
 import org.hl7.fhir.r4.model.SearchParameter.XPathUsageType;
+import org.hl7.fhir.r4.model.StringType;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -589,5 +591,53 @@ public class ResourceProviderCustomSearchParamR4Test extends BaseResourceProvide
 		}
 	}
 
+	@Test
+	public void testNicknameExpansionWithCustomSearchParameter() {
+
+		SearchParameter firstNameSp = new SearchParameter();
+		firstNameSp.setId("patient-firstName");
+		firstNameSp.setTitle("Patient First Name");
+		firstNameSp.setUrl("http://some.url.com");
+		firstNameSp.setName("firstName");
+		firstNameSp.setStatus(org.hl7.fhir.r4.model.Enumerations.PublicationStatus.ACTIVE);
+		firstNameSp.setCode("firstName");
+		firstNameSp.addBase("Patient");
+		firstNameSp.setType(org.hl7.fhir.r4.model.Enumerations.SearchParamType.STRING);
+		firstNameSp.setDescription("First given name of first patient name");
+		firstNameSp.setExpression("Patient.name[0].where(use='official' or use='usual' or use.exists().not()).given[0]");
+		firstNameSp.setXpathUsage(org.hl7.fhir.r4.model.SearchParameter.XPathUsageType.NORMAL);
+
+		mySearchParameterDao.create(firstNameSp, mySrd);
+
+		myCaptureQueriesListener.clear();
+		mySearchParamRegistry.forceRefresh();
+		myCaptureQueriesListener.logAllQueriesForCurrentThread();
+
+		Patient pat = new Patient();
+		pat.getNameFirstRep()
+			.setUse(HumanName.NameUse.OFFICIAL)
+			.setFamily("Chalmders")
+			.setGiven(List.of(new StringType("Kenneth"), new StringType("James")));
+
+		IIdType patId = myPatientDao.create(pat, mySrd).getId().toUnqualifiedVersionless();
+
+		Patient pat2 = new Patient();
+		pat2.getNameFirstRep()
+			.setUse(HumanName.NameUse.OFFICIAL)
+			.setFamily("Smith")
+			.setGiven(List.of(new StringType("Tom"), new StringType("Fred")));
+		IIdType patId2 = myPatientDao.create(pat2, mySrd).getId().toUnqualifiedVersionless();
+
+
+		Bundle result = myClient
+			.search()
+			.byUrl("Patient?firstName:nickname=Ken")
+			.returnBundle(Bundle.class)
+			.execute();
+
+		List<String> foundResources = toUnqualifiedVersionlessIdValues(result);
+		assertEquals(1, foundResources.size());
+		assertThat(foundResources, contains(patId.getValue()));
+	}
 
 }

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/TokenParamTest.java

@@ -66,16 +66,4 @@ public class TokenParamTest {
 		assertTrue(param.isNicknameExpand());
 	}
 
-	@Test
-	public void testInvalidNickname() {
-		StringParam param = new StringParam();
-		assertFalse(param.isNicknameExpand());
-		try {
-			param.setValueAsQueryToken(ourCtx, "family", Constants.PARAMQUALIFIER_NICKNAME, "kenny");
-			fail();
-		} catch (InvalidRequestException e) {
-			assertEquals("HAPI-2077: Modifier :nickname may only be used with 'name' and 'given' search parameters", e.getMessage());
-		}
-	}
-
 }