From 1785c07283b622d8ac7f907713377c26afc38eb0 Mon Sep 17 00:00:00 2001 From: michaelabuckley Date: Wed, 13 Jul 2022 11:51:24 -0400 Subject: [PATCH] Mb implement token :not inmemory (#3784) Implement token :not and fix :not-in in InMemoryMatcher Both `:not` and `:not-in` require that NONE of the values in an OR-list match, so we need some machinery to do a none-match instead of any-match. --- .../fhir/rest/param/TokenParamModifier.java | 9 ++++++ .../rest/param/TokenParamModifierTest.java | 23 +++++++++++++++ .../3784-support-token-not-in-memory.yaml | 5 ++++ .../matcher/InMemoryResourceMatcher.java | 29 +++++++++++++++++-- .../InMemoryResourceMatcherR5Test.java | 29 ++++++++++++++----- .../InMemorySubscriptionMatcherR4Test.java | 18 ------------ 6 files changed, 85 insertions(+), 28 deletions(-) create mode 100644 hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/TokenParamModifierTest.java create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3784-support-token-not-in-memory.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParamModifier.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParamModifier.java index 7710c372dc1..52163f936de 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParamModifier.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParamModifier.java @@ -95,4 +95,13 @@ public enum TokenParamModifier { return VALUE_TO_ENUM.get(theValue); } + public boolean isNegative() { + switch (this) { + case NOT: + case NOT_IN: + return true; + default: + return false; + } + } } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/TokenParamModifierTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/TokenParamModifierTest.java new file mode 100644 index 00000000000..482b6f0dcf4 --- /dev/null +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/TokenParamModifierTest.java @@ -0,0 +1,23 @@ +package ca.uhn.fhir.rest.param; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import java.util.EnumSet; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TokenParamModifierTest { + + @ParameterizedTest + @EnumSource() + void negativeModifiers(TokenParamModifier theTokenParamModifier) { + EnumSet negativeSet = EnumSet.of( + TokenParamModifier.NOT, + TokenParamModifier.NOT_IN + ); + + assertEquals(negativeSet.contains(theTokenParamModifier), theTokenParamModifier.isNegative()); + } + +} diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3784-support-token-not-in-memory.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3784-support-token-not-in-memory.yaml new file mode 100644 index 00000000000..c232573dd9f --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3784-support-token-not-in-memory.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 3784 +title: "The InMemoryMatcher used by Subscription matching not supports the token `:not` modifier. + The `:not-in` modifier was also corrected for cases of multiple values in a `,` separated or-list." diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java index 1568529087e..08bf978e9da 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java @@ -42,6 +42,7 @@ import ca.uhn.fhir.rest.param.ParamPrefixEnum; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.param.TokenParamModifier; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.util.MetaUtil; @@ -308,8 +309,27 @@ public class InMemoryResourceMatcher { } } - private boolean matchParams(ModelConfig theModelConfig, String theResourceName, String theParamName, RuntimeSearchParam paramDef, List theNextAnd, ResourceIndexedSearchParams theSearchParams) { - return theNextAnd.stream().anyMatch(token -> matchParam(theModelConfig, theResourceName, theParamName, paramDef, theSearchParams, token)); + private boolean matchParams(ModelConfig theModelConfig, String theResourceName, String theParamName, RuntimeSearchParam theParamDef, List theOrList, ResourceIndexedSearchParams theSearchParams) { + + boolean isNegativeTest = isNegative(theParamDef, theOrList); + // negative tests like :not and :not-in must not match any or-clause, so we invert the quantifier. + if (isNegativeTest) { + return theOrList.stream().allMatch(token -> matchParam(theModelConfig, theResourceName, theParamName, theParamDef, theSearchParams, token)); + } else { + return theOrList.stream().anyMatch(token -> matchParam(theModelConfig, theResourceName, theParamName, theParamDef, theSearchParams, token)); + } + } + + /** Some modifiers are negative, and must match NONE of their or-list */ + private boolean isNegative(RuntimeSearchParam theParamDef, List theOrList) { + if (theParamDef.getParamType().equals(RestSearchParameterTypeEnum.TOKEN)) { + TokenParam tokenParam = (TokenParam) theOrList.get(0); + TokenParamModifier modifier = tokenParam.getModifier(); + return modifier != null && modifier.isNegative(); + } else { + return false; + } + } private boolean matchParam(ModelConfig theModelConfig, String theResourceName, String theParamName, RuntimeSearchParam theParamDef, ResourceIndexedSearchParams theSearchParams, IQueryParameterType theToken) { @@ -322,6 +342,7 @@ public class InMemoryResourceMatcher { /** * Checks whether a query parameter of type token matches one of the search parameters of an in-memory resource. + * The :not modifier is supported. * The :in and :not-in qualifiers are supported only if a bean implementing IValidationSupport is available. * Any other qualifier will be ignored and the match will be treated as unqualified. * @param theModelConfig a model configuration @@ -343,6 +364,8 @@ public class InMemoryResourceMatcher { return theSearchParams.myTokenParams.stream() .filter(t -> t.getParamName().equals(theParamName)) .noneMatch(t -> systemContainsCode(theQueryParam, t)); + case NOT: + return !theSearchParams.matchParam(theModelConfig, theResourceName, theParamName, theParamDef, theQueryParam); default: return theSearchParams.matchParam(theModelConfig, theResourceName, theParamName, theParamDef, theQueryParam); } @@ -426,6 +449,8 @@ public class InMemoryResourceMatcher { case NOT_IN: // Support for these qualifiers is dependent on an implementation of IValidationSupport being available to delegate the check to return getValidationSupportOrNull() != null; + case NOT: + return true; default: return false; } diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java index bed6b16a5a1..606a26154e3 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java @@ -140,10 +140,15 @@ public class InMemoryResourceMatcherR5Test { } @Test - public void testUnsupportedNot() { - InMemoryMatchResult result = myInMemoryResourceMatcher.match("code" + TokenParamModifier.NOT.getValue() + "=" + OBSERVATION_CODE, myObservation, mySearchParams); - assertFalse(result.supported()); - assertEquals("Parameter: Reason: Qualified parameter not supported", result.getUnsupportedReason()); + public void testSupportedNot() { + String criteria = "code" + TokenParamModifier.NOT.getValue() + "=" + OBSERVATION_CODE + ",a_different_code"; + InMemoryMatchResult result = myInMemoryResourceMatcher.match(criteria, myObservation, mySearchParams); + assertTrue(result.supported()); + assertFalse(result.matched(), ":not must not match any of the OR-list"); + + result = myInMemoryResourceMatcher.match("code:not=a_different_code,and_another", myObservation, mySearchParams); + assertTrue(result.supported()); + assertTrue(result.matched(), ":not matches when NONE match"); } @Test @@ -184,12 +189,20 @@ public class InMemoryResourceMatcherR5Test { @Test public void testSupportedNotIn_NoMatch() { - IValidationSupport.CodeValidationResult codeValidationResult = new IValidationSupport.CodeValidationResult().setCode(OBSERVATION_CODE); - when(myValidationSupport.validateCode(any(), any(), any(), any(), any(), any())).thenReturn(codeValidationResult); + IValidationSupport.CodeValidationResult matchResult = new IValidationSupport.CodeValidationResult().setCode(OBSERVATION_CODE); + IValidationSupport.CodeValidationResult noMatchResult = new IValidationSupport.CodeValidationResult() + .setSeverity(IValidationSupport.IssueSeverity.ERROR) + .setMessage("not in"); - InMemoryMatchResult result = myInMemoryResourceMatcher.match("code" + TokenParamModifier.NOT_IN.getValue() + "=" + OBSERVATION_CODE_VALUE_SET_URI, myObservation, mySearchParams); + // mock 2 value sets. Once containing the code, and one not. + String otherValueSet = OBSERVATION_CODE_VALUE_SET_URI + "-different"; + when(myValidationSupport.validateCode(any(), any(), any(), any(), any(), eq(OBSERVATION_CODE_VALUE_SET_URI))).thenReturn(matchResult); + when(myValidationSupport.validateCode(any(), any(), any(), any(), any(), eq(otherValueSet))).thenReturn(noMatchResult); + + String criteria = "code" + TokenParamModifier.NOT_IN.getValue() + "=" + OBSERVATION_CODE_VALUE_SET_URI + "," + otherValueSet; + InMemoryMatchResult result = myInMemoryResourceMatcher.match(criteria, myObservation, mySearchParams); assertTrue(result.supported()); - assertFalse(result.matched()); + assertFalse(result.matched(), ":not-in matches when NONE of the OR-list match"); verify(myValidationSupport).validateCode(any(), any(), eq(OBSERVATION_CODE_SYSTEM), eq(OBSERVATION_CODE), isNull(), eq(OBSERVATION_CODE_VALUE_SET_URI)); } diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java index 780bda14dbd..1dca8b80dd9 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java @@ -745,24 +745,6 @@ public class InMemorySubscriptionMatcherR4Test { } } - @Test - public void testSearchTokenWithNotModifierUnsupported() { - Patient patient = new Patient(); - patient.addIdentifier().setSystem("urn:system").setValue("001"); - patient.addName().setFamily("Tester").addGiven("Joe"); - patient.setGender(Enumerations.AdministrativeGender.MALE); - - SearchParameterMap params; - - params = new SearchParameterMap(); - params.add(Patient.SP_GENDER, new TokenParam(null, "male")); - assertMatched(patient, params); - - params = new SearchParameterMap(); - params.add(Patient.SP_GENDER, new TokenParam(null, "male").setModifier(TokenParamModifier.NOT)); - assertUnsupported(patient, params); - } - @Test public void testSearchTokenWrongParam() { Patient p1 = new Patient();