From 26ff15f9392f361090b02c72853a12d7c0694030 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Thu, 2 Sep 2021 14:33:30 -0400 Subject: [PATCH] Revert "Fix search of :not modifier" --- ...h-with-not-modifier-on-multiple-codes.yaml | 4 - .../fhir/jpa/search/builder/QueryStack.java | 51 +--- .../ResourceProviderSearchModifierR4Test.java | 248 ------------------ 3 files changed, 14 insertions(+), 289 deletions(-) delete mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2837-search-with-not-modifier-on-multiple-codes.yaml delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2837-search-with-not-modifier-on-multiple-codes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2837-search-with-not-modifier-on-multiple-codes.yaml deleted file mode 100644 index fed478a7120..00000000000 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2837-search-with-not-modifier-on-multiple-codes.yaml +++ /dev/null @@ -1,4 +0,0 @@ ---- -type: fix -issue: 2837 -title: "The :not modifier does not currently work for observations with multiple codes for the search. This is fixed." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 068c06c5fe9..253b07e33b6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -988,12 +988,9 @@ public class QueryStack { String theSpnamePrefix, RuntimeSearchParam theSearchParam, List theList, SearchFilterParser.CompareOperation theOperation, RequestPartitionId theRequestPartitionId) { - List tokens = new ArrayList<>(); - - boolean paramInverted = false; - TokenParamModifier modifier = null; - + List tokens = new ArrayList<>(); for (IQueryParameterType nextOr : theList) { + if (nextOr instanceof TokenParam) { if (!((TokenParam) nextOr).isEmpty()) { TokenParam id = (TokenParam) nextOr; @@ -1012,20 +1009,17 @@ public class QueryStack { } return createPredicateString(theSourceJoinColumn, theResourceName, theSpnamePrefix, theSearchParam, theList, null, theRequestPartitionId); - } - - modifier = id.getModifier(); - // for :not modifier, create a token and remove the :not modifier - if (modifier != null && modifier == TokenParamModifier.NOT) { - tokens.add(new TokenParam(((TokenParam) nextOr).getSystem(), ((TokenParam) nextOr).getValue())); - paramInverted = true; - } else { - tokens.add(nextOr); } + + tokens.add(nextOr); + } + } else { + tokens.add(nextOr); } + } if (tokens.isEmpty()) { @@ -1033,31 +1027,14 @@ public class QueryStack { } String paramName = getParamNameWithPrefix(theSpnamePrefix, theSearchParam.getName()); - Condition predicate; - BaseJoiningPredicateBuilder join; - - if (paramInverted) { - SearchQueryBuilder sqlBuilder = mySqlBuilder.newChildSqlBuilder(); - TokenPredicateBuilder tokenSelector = sqlBuilder.addTokenPredicateBuilder(null); - sqlBuilder.addPredicate(tokenSelector.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theRequestPartitionId)); - SelectQuery sql = sqlBuilder.getSelect(); - Expression subSelect = new Subquery(sql); - - join = mySqlBuilder.getOrCreateFirstPredicateBuilder(); - predicate = new InCondition(join.getResourceIdColumn(), subSelect).setNegate(true); - - } else { - - TokenPredicateBuilder tokenJoin = createOrReusePredicateBuilder(PredicateBuilderTypeEnum.TOKEN, theSourceJoinColumn, paramName, () -> mySqlBuilder.addTokenPredicateBuilder(theSourceJoinColumn)).getResult(); - if (theList.get(0).getMissing() != null) { - return tokenJoin.createPredicateParamMissingForNonReference(theResourceName, paramName, theList.get(0).getMissing(), theRequestPartitionId); - } + TokenPredicateBuilder join = createOrReusePredicateBuilder(PredicateBuilderTypeEnum.TOKEN, theSourceJoinColumn, paramName, () -> mySqlBuilder.addTokenPredicateBuilder(theSourceJoinColumn)).getResult(); - predicate = tokenJoin.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theOperation, theRequestPartitionId); - join = tokenJoin; - } - + if (theList.get(0).getMissing() != null) { + return join.createPredicateParamMissingForNonReference(theResourceName, paramName, theList.get(0).getMissing(), theRequestPartitionId); + } + + Condition predicate = join.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theOperation, theRequestPartitionId); return join.combineWithRequestPartitionIdPredicate(theRequestPartitionId, predicate); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java deleted file mode 100644 index 22210d36e56..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java +++ /dev/null @@ -1,248 +0,0 @@ -package ca.uhn.fhir.jpa.provider.r4; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.List; - -import org.apache.commons.io.IOUtils; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.CodeableConcept; -import org.hl7.fhir.r4.model.DateTimeType; -import org.hl7.fhir.r4.model.Observation; -import org.hl7.fhir.r4.model.Observation.ObservationStatus; -import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Quantity; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.parser.StrictErrorHandler; -import ca.uhn.fhir.rest.client.interceptor.CapturingInterceptor; - - -public class ResourceProviderSearchModifierR4Test extends BaseResourceProviderR4Test { - - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderSearchModifierR4Test.class); - private CapturingInterceptor myCapturingInterceptor = new CapturingInterceptor(); - - @Override - @AfterEach - public void after() throws Exception { - super.after(); - - myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); - myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); - myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); - myDaoConfig.setCountSearchResultsUpTo(new DaoConfig().getCountSearchResultsUpTo()); - myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); - myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); - myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields()); - - myClient.unregisterInterceptor(myCapturingInterceptor); - } - - @BeforeEach - @Override - public void before() throws Exception { - super.before(); - myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); - - myDaoConfig.setAllowMultipleDelete(true); - myClient.registerInterceptor(myCapturingInterceptor); - myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); - } - - @BeforeEach - public void beforeDisableResultReuse() { - myDaoConfig.setReuseCachedSearchResultsForMillis(null); - } - - @Test - public void testSearch_SingleCode_not_modifier() throws Exception { - - List obsList = createObs(10, false); - - String uri = ourServerBase + "/Observation?code:not=2345-3"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(9, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(2).toString(), ids.get(2)); - assertEquals(obsList.get(4).toString(), ids.get(3)); - assertEquals(obsList.get(5).toString(), ids.get(4)); - assertEquals(obsList.get(6).toString(), ids.get(5)); - assertEquals(obsList.get(7).toString(), ids.get(6)); - assertEquals(obsList.get(8).toString(), ids.get(7)); - assertEquals(obsList.get(9).toString(), ids.get(8)); - } - - @Test - public void testSearch_SingleCode_multiple_not_modifier() throws Exception { - - List obsList = createObs(10, false); - - String uri = ourServerBase + "/Observation?code:not=2345-3&code:not=2345-7&code:not=2345-9"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(7, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(2).toString(), ids.get(2)); - assertEquals(obsList.get(4).toString(), ids.get(3)); - assertEquals(obsList.get(5).toString(), ids.get(4)); - assertEquals(obsList.get(6).toString(), ids.get(5)); - assertEquals(obsList.get(8).toString(), ids.get(6)); - } - - @Test - public void testSearch_SingleCode_mix_modifier() throws Exception { - - List obsList = createObs(10, false); - - // Observation?code:not=2345-3&code:not=2345-7&code:not=2345-9 - // slower than Observation?code:not=2345-3&code=2345-7&code:not=2345-9 - String uri = ourServerBase + "/Observation?code:not=2345-3&code=2345-7&code:not=2345-9"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(1, ids.size()); - assertEquals(obsList.get(7).toString(), ids.get(0)); - } - - @Test - public void testSearch_SingleCode_or_not_modifier() throws Exception { - - List obsList = createObs(10, false); - - String uri = ourServerBase + "/Observation?code:not=2345-3,2345-7,2345-9"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(7, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(2).toString(), ids.get(2)); - assertEquals(obsList.get(4).toString(), ids.get(3)); - assertEquals(obsList.get(5).toString(), ids.get(4)); - assertEquals(obsList.get(6).toString(), ids.get(5)); - assertEquals(obsList.get(8).toString(), ids.get(6)); - } - - @Test - public void testSearch_MultiCode_not_modifier() throws Exception { - - List obsList = createObs(10, true); - - String uri = ourServerBase + "/Observation?code:not=2345-3"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(8, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(4).toString(), ids.get(2)); - assertEquals(obsList.get(5).toString(), ids.get(3)); - assertEquals(obsList.get(6).toString(), ids.get(4)); - assertEquals(obsList.get(7).toString(), ids.get(5)); - assertEquals(obsList.get(8).toString(), ids.get(6)); - assertEquals(obsList.get(9).toString(), ids.get(7)); - } - - @Test - public void testSearch_MultiCode_multiple_not_modifier() throws Exception { - - List obsList = createObs(10, true); - - String uri = ourServerBase + "/Observation?code:not=2345-3&code:not=2345-4"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(7, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(5).toString(), ids.get(2)); - assertEquals(obsList.get(6).toString(), ids.get(3)); - assertEquals(obsList.get(7).toString(), ids.get(4)); - assertEquals(obsList.get(8).toString(), ids.get(5)); - assertEquals(obsList.get(9).toString(), ids.get(6)); - } - - @Test - public void testSearch_MultiCode_mix_modifier() throws Exception { - - List obsList = createObs(10, true); - - String uri = ourServerBase + "/Observation?code:not=2345-3&code=2345-7&code:not=2345-9"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(2, ids.size()); - assertEquals(obsList.get(6).toString(), ids.get(0)); - assertEquals(obsList.get(7).toString(), ids.get(1)); - } - - @Test - public void testSearch_MultiCode_or_not_modifier() throws Exception { - - List obsList = createObs(10, true); - - String uri = ourServerBase + "/Observation?code:not=2345-3,2345-7,2345-9"; - List ids = searchAndReturnUnqualifiedVersionlessIdValues(uri); - - assertEquals(4, ids.size()); - assertEquals(obsList.get(0).toString(), ids.get(0)); - assertEquals(obsList.get(1).toString(), ids.get(1)); - assertEquals(obsList.get(4).toString(), ids.get(2)); - assertEquals(obsList.get(5).toString(), ids.get(3)); - } - - - private List searchAndReturnUnqualifiedVersionlessIdValues(String uri) throws IOException { - List ids; - HttpGet get = new HttpGet(uri); - - try (CloseableHttpResponse response = ourHttpClient.execute(get)) { - String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); - ourLog.info("Response was: {}", resp); - Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, resp); - ourLog.info("Bundle: \n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); - ids = toUnqualifiedVersionlessIdValues(bundle); - } - return ids; - } - - private List createObs(int obsNum, boolean isMultiple) { - - Patient patient = new Patient(); - patient.addIdentifier().setSystem("urn:system").setValue("001"); - patient.addName().setFamily("Tester").addGiven("Joe"); - IIdType pid = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - - List obsIds = new ArrayList<>(); - IIdType obsId = null; - for (int i=0; i