From 9bdef1a9fad8b372448ad5500c9258abee4c8322 Mon Sep 17 00:00:00 2001 From: Frank Tao Date: Sat, 28 Aug 2021 13:24:09 -0400 Subject: [PATCH] Fix search of :not modifier --- .../fhir/jpa/search/builder/QueryStack.java | 50 +++- .../ResourceProviderSearchModifierR4Test.java | 248 ++++++++++++++++++ 2 files changed, 286 insertions(+), 12 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java 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 253b07e33b6..71857a9e0b5 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,9 +988,12 @@ public class QueryStack { String theSpnamePrefix, RuntimeSearchParam theSearchParam, List theList, SearchFilterParser.CompareOperation theOperation, RequestPartitionId theRequestPartitionId) { - List tokens = new ArrayList<>(); + List tokens = new ArrayList<>(); + + boolean paramInverted = false; + + TokenParamModifier modifier = null; for (IQueryParameterType nextOr : theList) { - if (nextOr instanceof TokenParam) { if (!((TokenParam) nextOr).isEmpty()) { TokenParam id = (TokenParam) nextOr; @@ -1009,17 +1012,23 @@ public class QueryStack { } return createPredicateString(theSourceJoinColumn, theResourceName, theSpnamePrefix, theSearchParam, theList, null, theRequestPartitionId); + } + + modifier = id.getModifier(); + // for all :not modifier, create a token list, and remove the :not modifier + if (modifier != null && modifier == TokenParamModifier.NOT) { + IQueryParameterType notToken = new TokenParam(((TokenParam) nextOr).getSystem(), ((TokenParam) nextOr).getValue()); + tokens.add(notToken); + paramInverted = true; + } else { + tokens.add(nextOr); } - - tokens.add(nextOr); - } } else { tokens.add(nextOr); } - } if (tokens.isEmpty()) { @@ -1027,14 +1036,31 @@ 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(); - TokenPredicateBuilder join = 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); + } - 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); + predicate = tokenJoin.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theOperation, theRequestPartitionId); + join = tokenJoin; + } + 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 new file mode 100644 index 00000000000..22210d36e56 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java @@ -0,0 +1,248 @@ +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