From 9b50b332a4cc13dc9e9247b2283776005c9f89eb Mon Sep 17 00:00:00 2001 From: Nathan Doef Date: Wed, 7 Sep 2022 11:52:37 -0400 Subject: [PATCH] :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 Co-authored-by: kylejule --- .../ca/uhn/fhir/rest/param/StringParam.java | 14 ++-- .../uhn/fhir/rest/param/StringParamTest.java | 77 +++++++++++++++++++ ...pport-for-any-string-searchparameters.yaml | 6 ++ ...sourceProviderCustomSearchParamR4Test.java | 50 ++++++++++++ .../uhn/fhir/rest/param/TokenParamTest.java | 12 --- 5 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3972-nickname-qualifier-support-for-any-string-searchparameters.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java index b7b54e3f3cf..3e62ddd9192 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java +++ b/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; - theQualifier = ""; - } else { - throw new InvalidRequestException(Msg.code(2077) + "Modifier " + Constants.PARAMQUALIFIER_NICKNAME + " may only be used with 'name' and 'given' search parameters"); + myNicknameExpand = true; + theQualifier = ""; + + if (!("name".equals(theParamName) || "given".equals(theParamName))){ + ourLog.debug(":nickname qualifier was assigned to a search parameter other than one of the intended parameters \"name\" and \"given\""); } } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/StringParamTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/StringParamTest.java index 4bbce1afa1f..0f682312694 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/StringParamTest.java +++ b/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 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 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()); + } + } } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3972-nickname-qualifier-support-for-any-string-searchparameters.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3972-nickname-qualifier-support-for-any-string-searchparameters.yaml new file mode 100644 index 00000000000..39e63a7c54e --- /dev/null +++ b/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." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderCustomSearchParamR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderCustomSearchParamR4Test.java index 3166775237c..4475fb3163f 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderCustomSearchParamR4Test.java +++ b/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 foundResources = toUnqualifiedVersionlessIdValues(result); + assertEquals(1, foundResources.size()); + assertThat(foundResources, contains(patId.getValue())); + } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/TokenParamTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/TokenParamTest.java index 4f35e91cfd1..64fdf393cca 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/TokenParamTest.java +++ b/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()); - } - } - }