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 new file mode 100644 index 00000000000..fed478a7120 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2837-search-with-not-modifier-on-multiple-codes.yaml @@ -0,0 +1,4 @@ +--- +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 253b07e33b6..89bcb4d6a8d 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,20 @@ 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()) { @@ -1027,14 +1033,37 @@ 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(); + + if (theSourceJoinColumn == null) { + predicate = new InCondition(join.getResourceIdColumn(), subSelect).setNegate(true); + } else { + //-- for the resource link, need join with target_resource_id + predicate = new InCondition(theSourceJoinColumn, 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/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 9f14b0bd4d5..d718a195e09 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -221,11 +221,15 @@ public class SearchBuilder implements ISearchBuilder { SearchContainedModeEnum searchContainedMode = theParams.getSearchContainedMode(); - // Handle _id last, since it can typically be tacked onto a different parameter - List paramNames = myParams.keySet().stream().filter(t -> !t.equals(IAnyResource.SP_RES_ID)).collect(Collectors.toList()); + // Handle _id and _tag last, since they can typically be tacked onto a different parameter + List paramNames = myParams.keySet().stream().filter(t -> !t.equals(IAnyResource.SP_RES_ID)) + .filter(t -> !t.equals(Constants.PARAM_TAG)).collect(Collectors.toList()); if (myParams.containsKey(IAnyResource.SP_RES_ID)) { paramNames.add(IAnyResource.SP_RES_ID); } + if (myParams.containsKey(Constants.PARAM_TAG)) { + paramNames.add(Constants.PARAM_TAG); + } // Handle each parameter for (String nextParamName : paramNames) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java index aa1e6d7ea7e..80a2e316767 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java @@ -62,6 +62,7 @@ import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.SpecialParam; 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.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; @@ -338,6 +339,7 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder { boolean foundChainMatch = false; List candidateTargetTypes = new ArrayList<>(); List orPredicates = new ArrayList<>(); + boolean paramInverted = false; QueryStack childQueryFactory = myQueryStack.newChildQueryFactoryWithFullBuilderReuse(); for (String nextType : resourceTypes) { String chain = theReferenceParam.getChain(); @@ -383,6 +385,13 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder { if (chainValue == null) { continue; } + + // For the token param, if it's a :not modifier, need switch OR to AND + if (!paramInverted && chainValue instanceof TokenParam) { + if (((TokenParam) chainValue).getModifier() == TokenParamModifier.NOT) { + paramInverted = true; + } + } foundChainMatch = true; orValues.add(chainValue); } @@ -410,10 +419,17 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder { warnAboutPerformanceOnUnqualifiedResources(theParamName, theRequest, candidateTargetTypes); } - Condition multiTypeOrPredicate = toOrPredicate(orPredicates); + // If :not modifier for a token, switch OR with AND in the multi-type case + Condition multiTypePredicate; + if (paramInverted) { + multiTypePredicate = toAndPredicate(orPredicates); + } else { + multiTypePredicate = toOrPredicate(orPredicates); + } + List pathsToMatch = createResourceLinkPaths(theResourceName, theParamName); Condition pathPredicate = createPredicateSourcePaths(pathsToMatch); - return toAndPredicate(pathPredicate, multiTypeOrPredicate); + return toAndPredicate(pathPredicate, multiTypePredicate); } @Nonnull diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/TestUtil.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/TestUtil.java index 22676af6bf8..f878494e559 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/TestUtil.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/TestUtil.java @@ -96,7 +96,7 @@ public class TestUtil { .collect(Collectors.toSet()); } - ImmutableSet classes = ClassPath.from(TestUtil.class.getClassLoader()).getTopLevelClasses(packageName); + ImmutableSet classes = ClassPath.from(TestUtil.class.getClassLoader()).getTopLevelClassesRecursive(packageName); Set names = new HashSet(); if (classes.size() <= 1) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index 7714d442b05..9c60bafa905 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -63,6 +63,7 @@ import org.hl7.fhir.dstu3.model.AllergyIntolerance; import org.hl7.fhir.dstu3.model.Appointment; import org.hl7.fhir.dstu3.model.AuditEvent; import org.hl7.fhir.dstu3.model.Binary; +import org.hl7.fhir.dstu3.model.BodySite; import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.CarePlan; import org.hl7.fhir.dstu3.model.CodeSystem; @@ -95,6 +96,7 @@ import org.hl7.fhir.dstu3.model.Organization; import org.hl7.fhir.dstu3.model.Patient; import org.hl7.fhir.dstu3.model.Practitioner; import org.hl7.fhir.dstu3.model.PractitionerRole; +import org.hl7.fhir.dstu3.model.Procedure; import org.hl7.fhir.dstu3.model.ProcedureRequest; import org.hl7.fhir.dstu3.model.Questionnaire; import org.hl7.fhir.dstu3.model.QuestionnaireResponse; @@ -317,6 +319,12 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("myTaskDaoDstu3") protected IFhirResourceDao myTaskDao; @Autowired + @Qualifier("myBodySiteDaoDstu3") + protected IFhirResourceDao myBodySiteDao; + @Autowired + @Qualifier("myProcedureDaoDstu3") + protected IFhirResourceDao myProcedureDao; + @Autowired protected ITermConceptDao myTermConceptDao; @Autowired protected ITermCodeSystemDao myTermCodeSystemDao; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java index a84edb5bdc9..d58e23b5675 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java @@ -21,18 +21,29 @@ import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.param.TokenParamModifier; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.BodyStructure; +import org.hl7.fhir.r4.model.CodeableConcept; +import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.DateTimeType; +import org.hl7.fhir.r4.model.Device; import org.hl7.fhir.r4.model.Enumerations; +import org.hl7.fhir.r4.model.Extension; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Procedure; +import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; +import org.hl7.fhir.r4.model.SearchParameter; +import org.hl7.fhir.r4.model.StringType; +import org.hl7.fhir.r4.model.UriType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -906,6 +917,117 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { } + @Test + public void testSearchOnUnderscoreParams_AvoidHFJResourceJoins() { + // This Issue: https://github.com/hapifhir/hapi-fhir/issues/2942 + // See this PR for a similar type of Fix: https://github.com/hapifhir/hapi-fhir/pull/2909 + SearchParameter searchParameter = new SearchParameter(); + searchParameter.addBase("BodySite").addBase("Procedure"); + searchParameter.setCode("focalAccess"); + searchParameter.setType(Enumerations.SearchParamType.REFERENCE); + searchParameter.setExpression("Procedure.extension('Procedure#focalAccess')"); + searchParameter.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); + searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); + IIdType spId = mySearchParameterDao.create(searchParameter).getId().toUnqualifiedVersionless(); + mySearchParamRegistry.forceRefresh(); + + BodyStructure bs = new BodyStructure(); + bs.setDescription("BodyStructure in R4 replaced BodySite from DSTU4"); + IIdType bsId = myBodyStructureDao.create(bs, mySrd).getId().toUnqualifiedVersionless(); + + Patient patient = new Patient(); + patient.setId("P1"); + patient.setActive(true); + patient.addName().setFamily("FamilyName"); + Extension extParent = patient + .addExtension() + .setUrl("http://hl7.org/fhir/us/core/StructureDefinition/us-core-ethnicity"); + extParent + .addExtension() + .setUrl("ombCategory") + .setValue(new CodeableConcept().addCoding(new Coding().setSystem("urn:oid:2.16.840.1.113883.5.50") + .setCode("2186-5") + .setDisplay("Not Hispanic or Latino"))); + extParent + .addExtension() + .setUrl("text") + .setValue(new StringType("Not Hispanic or Latino")); + myPatientDao.update(patient); + CodeableConcept categoryCodeableConcept1 = new CodeableConcept().addCoding(new Coding().setSystem("acc_proccat_fkc") + .setCode("CANN") + .setDisplay("Cannulation")); + Procedure procedure = new Procedure(); + procedure.setSubject(new Reference("Patient/P1")); + procedure.setStatus(Procedure.ProcedureStatus.COMPLETED); + procedure.setCategory(categoryCodeableConcept1); + Extension extProcedure = procedure + .addExtension() + .setUrl("Procedure#focalAccess") + .setValue(new UriType("BodyStructure/" + bsId.getIdPartAsLong())); + procedure.getMeta() + .addTag("acc_procext_fkc", "1STCANN2NDL", "First Successful Cannulation with 2 Needles"); + IIdType procedureId = myProcedureDao.create(procedure).getId().toUnqualifiedVersionless(); + + logAllResources(); + logAllResourceTags(); + logAllResourceVersions(); + + // Search example 1: + // http://FHIR_SERVER/fhir_request/Procedure + // ?status%3Anot=entered-in-error&subject=B + // &category=CANN&focalAccess=BodySite%2F3530342921&_tag=TagValue + // NOTE: This gets sorted once so the order is different once it gets executed! + { + // IMPORTANT: Keep the query param order exactly as shown below! + SearchParameterMap map = SearchParameterMap.newSynchronous(); + // _tag, category, status, subject, focalAccess + map.add("_tag", new TokenParam("TagValue")); + map.add("category", new TokenParam("CANN")); + map.add("status", new TokenParam("entered-in-error").setModifier(TokenParamModifier.NOT)); + map.add("subject", new ReferenceParam("Patient/P1")); + map.add("focalAccess", new ReferenceParam("BodyStructure/" + bsId.getIdPart())); + myCaptureQueriesListener.clear(); + IBundleProvider outcome = myProcedureDao.search(map, new SystemRequestDetails()); + ourLog.info("Search returned {} resources.", outcome.getResources(0, 999).size()); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + String selectQuery = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + // Check for a particular WHERE CLAUSE in the generated SQL to make sure we are verifying the correct query + assertEquals(2, StringUtils.countMatches(selectQuery.toLowerCase(), " join hfj_res_link "), selectQuery); + + // Ensure that we do NOT see a couple of particular WHERE clauses + assertEquals(0, StringUtils.countMatches(selectQuery.toLowerCase(), ".res_type = 'procedure'"), selectQuery); + assertEquals(0, StringUtils.countMatches(selectQuery.toLowerCase(), ".res_deleted_at is null"), selectQuery); + } + + // Search example 2: + // http://FHIR_SERVER/fhir_request/Procedure + // ?status%3Anot=entered-in-error&category=CANN&focalAccess=3692871435 + // &_tag=1STCANN1NDL%2C1STCANN2NDL&outcome=SUCCESS&_count=1&_requestTrace=True + // NOTE: This gets sorted once so the order is different once it gets executed! + { + // IMPORTANT: Keep the query param order exactly as shown below! + // NOTE: The "outcome" SearchParameter is not being used below, but it doesn't affect the test. + SearchParameterMap map = SearchParameterMap.newSynchronous(); + // _tag, category, status, focalAccess + map.add("_tag", new TokenParam("TagValue")); + map.add("category", new TokenParam("CANN")); + map.add("status", new TokenParam("entered-in-error").setModifier(TokenParamModifier.NOT)); + map.add("focalAccess", new ReferenceParam("BodyStructure/" + bsId.getIdPart())); + myCaptureQueriesListener.clear(); + IBundleProvider outcome = myProcedureDao.search(map, new SystemRequestDetails()); + ourLog.info("Search returned {} resources.", outcome.getResources(0, 999).size()); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + String selectQuery = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + // Check for a particular WHERE CLAUSE in the generated SQL to make sure we are verifying the correct query + assertEquals(1, StringUtils.countMatches(selectQuery.toLowerCase(), " join hfj_res_link "), selectQuery); + + // Ensure that we do NOT see a couple of particular WHERE clauses + assertEquals(0, StringUtils.countMatches(selectQuery.toLowerCase(), ".res_type = 'procedure'"), selectQuery); + assertEquals(0, StringUtils.countMatches(selectQuery.toLowerCase(), ".res_deleted_at is null"), selectQuery); + } + } @AfterEach public void afterResetDao() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java index 44a615682f0..7b635e56f1b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java @@ -2272,7 +2272,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", searchSql); assertEquals(0, StringUtils.countMatches(searchSql, "PARTITION_ID"), searchSql); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'"), searchSql); - assertEquals(1, StringUtils.countMatches(searchSql, "t1.HASH_SYS_AND_VALUE ="), searchSql); + assertEquals(1, StringUtils.countMatches(searchSql, ".HASH_SYS_AND_VALUE ="), searchSql); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderSearchModifierDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderSearchModifierDstu3Test.java new file mode 100644 index 00000000000..d4c25bba299 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderSearchModifierDstu3Test.java @@ -0,0 +1,77 @@ +package ca.uhn.fhir.jpa.provider.dstu3; + +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.jpa.searchparam.MatchUrlService; +import ca.uhn.fhir.jpa.searchparam.ResourceSearch; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import org.hl7.fhir.dstu3.model.BodySite; +import org.hl7.fhir.dstu3.model.CodeableConcept; +import org.hl7.fhir.dstu3.model.Coding; +import org.hl7.fhir.dstu3.model.Encounter; +import org.hl7.fhir.dstu3.model.Observation; +import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.dstu3.model.Procedure; +import org.hl7.fhir.dstu3.model.Reference; +import org.hl7.fhir.dstu3.model.SearchParameter; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.Collections; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class ResourceProviderSearchModifierDstu3Test extends BaseResourceProviderDstu3Test{ + @Autowired + MatchUrlService myMatchUrlService; + + @Test + public void testReplicateBugWithNotDuringChain() { + Encounter enc = new Encounter(); + enc.setType(Collections.singletonList(new CodeableConcept().addCoding(new Coding("system", "value", "display")))); + IIdType encId = myEncounterDao.create(enc).getId(); + + Observation obs = new Observation(); + obs.setContext(new Reference(encId)); + myObservationDao.create(obs).getId(); + + { + //Works when not chained: + String encounterSearchString = "Encounter?type:not=system|value"; + ResourceSearch resourceSearch = myMatchUrlService.getResourceSearch(encounterSearchString); + SearchParameterMap searchParameterMap = resourceSearch.getSearchParameterMap(); + IBundleProvider search = myEncounterDao.search(searchParameterMap); + assertThat(search.size(), is(equalTo(0))); + } + { + //Works without the NOT qualifier. + String resultSearchString = "Observation?context.type=system|value"; + ResourceSearch resourceSearch = myMatchUrlService.getResourceSearch(resultSearchString); + SearchParameterMap searchParameterMap = resourceSearch.getSearchParameterMap(); + IBundleProvider search = myObservationDao.search(searchParameterMap); + assertThat(search.size(), is(equalTo(1))); + } + + { + //Works in a chain + String noResultSearchString = "Observation?context.type:not=system|value"; + ResourceSearch resourceSearch = myMatchUrlService.getResourceSearch(noResultSearchString); + SearchParameterMap searchParameterMap = resourceSearch.getSearchParameterMap(); + IBundleProvider search = myObservationDao.search(searchParameterMap); + assertThat(search.size(), is(equalTo(0))); + } + { + //Works in a chain with only value + String noResultSearchString = "Observation?context.type:not=value"; + ResourceSearch resourceSearch = myMatchUrlService.getResourceSearch(noResultSearchString); + SearchParameterMap searchParameterMap = resourceSearch.getSearchParameterMap(); + IBundleProvider search = myObservationDao.search(searchParameterMap); + assertThat(search.size(), is(equalTo(0))); + + } + + } +} 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..31f892b6fb3 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderSearchModifierR4Test.java @@ -0,0 +1,249 @@ +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