From feff73473610b49bd4fc9492fab8c8e1933b3174 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 22 Jan 2021 11:24:23 -0500 Subject: [PATCH 01/12] Bump jaxb-runtime from 2.3.1 to 3.0.0 (#2289) * Bump jaxb-runtime from 2.3.1 to 3.0.0 Bumps jaxb-runtime from 2.3.1 to 3.0.0. Signed-off-by: dependabot[bot] * Bump jaxb-runtime from 2.3.1 to 3.0.0 Bumps jaxb-runtime from 2.3.1 to 3.0.0. Signed-off-by: dependabot[bot] * Add jakarta bindings * Dependency cleanup * More depndency cleanup * Work on deps * Replace transaction annotation * Cleanup * Another fix * Build fix Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: jamesagnew --- hapi-deployable-pom/pom.xml | 12 ++--- hapi-fhir-cli/hapi-fhir-cli-api/pom.xml | 8 --- hapi-fhir-jpaserver-api/pom.xml | 50 ++++++------------- hapi-fhir-jpaserver-base/pom.xml | 45 ----------------- .../search/HapiLuceneAnalysisConfigurer.java | 2 - hapi-fhir-jpaserver-model/pom.xml | 16 +++--- hapi-fhir-structures-dstu2.1/pom.xml | 10 ---- hapi-fhir-structures-dstu2/pom.xml | 10 ---- hapi-fhir-structures-dstu3/pom.xml | 38 -------------- hapi-fhir-structures-hl7org-dstu2/pom.xml | 10 ---- hapi-fhir-structures-r4/pom.xml | 10 ---- hapi-fhir-validation/pom.xml | 10 ---- pom.xml | 10 +--- 13 files changed, 26 insertions(+), 205 deletions(-) diff --git a/hapi-deployable-pom/pom.xml b/hapi-deployable-pom/pom.xml index 2781a057c6f..4b57d899174 100644 --- a/hapi-deployable-pom/pom.xml +++ b/hapi-deployable-pom/pom.xml @@ -99,14 +99,6 @@ javax.mail javax.mail-api - - javax.activation - javax.activation-api - - - com.helger - ph-schematron - commons-logging commons-logging @@ -151,6 +143,10 @@ net.bytebuddy byte-buddy + + javax.xml.bind + jaxb-api + com.sun.xml.bind jaxb-impl diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/pom.xml b/hapi-fhir-cli/hapi-fhir-cli-api/pom.xml index 1d373e7221d..6fb9e4b6835 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/pom.xml +++ b/hapi-fhir-cli/hapi-fhir-cli-api/pom.xml @@ -224,14 +224,6 @@ javax.xml.bind jaxb-api - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/hapi-fhir-jpaserver-api/pom.xml b/hapi-fhir-jpaserver-api/pom.xml index 843ebfd672a..07c7e0c1bfe 100644 --- a/hapi-fhir-jpaserver-api/pom.xml +++ b/hapi-fhir-jpaserver-api/pom.xml @@ -56,37 +56,15 @@ hapi-fhir-structures-hl7org-dstu2 ${project.version} - - ca.uhn.hapi.fhir - hapi-fhir-jpaserver-model - ${project.version} - - - ca.uhn.hapi.fhir - hapi-fhir-jpaserver-searchparam - ${project.version} - - org.hibernate - hibernate-core - - - xml-apis - xml-apis - - - org.jboss.spec.javax.transaction - jboss-transaction-api_1.2_spec - - - javax.activation - activation - - - javax.activation - javax.activation-api - - + ca.uhn.hapi.fhir + hapi-fhir-jpaserver-model + ${project.version} + + + ca.uhn.hapi.fhir + hapi-fhir-jpaserver-searchparam + ${project.version} org.hibernate.search @@ -137,13 +115,13 @@ javax.annotation-api - - javax.servlet - javax.servlet-api - provided - + + javax.servlet + javax.servlet-api + provided + - + ch.qos.logback logback-classic diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 4322697f967..f012a3ff2e1 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -373,43 +373,13 @@ - - org.hibernate - hibernate-core - - - xml-apis - xml-apis - - - javax.activation - javax.activation-api - - - - - - - org.hibernate hibernate-entitymanager - - - org.jboss.spec.javax.transaction - jboss-transaction-api_1.2_spec - - org.hibernate hibernate-java8 - - - org.jboss.spec.javax.transaction - jboss-transaction-api_1.2_spec - - org.hibernate @@ -435,11 +405,6 @@ - - com.sun.activation - javax.activation - 1.2.0 - javax.mail javax.mail-api @@ -715,16 +680,6 @@ jaxb-runtime ${jaxb_runtime_version} - - - - - - - - - - diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/HapiLuceneAnalysisConfigurer.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/HapiLuceneAnalysisConfigurer.java index 5e2ac24acda..63be4acab22 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/HapiLuceneAnalysisConfigurer.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/HapiLuceneAnalysisConfigurer.java @@ -20,7 +20,6 @@ package ca.uhn.fhir.jpa.search; * #L% */ -import com.sun.xml.bind.api.impl.NameConverter; import org.apache.lucene.analysis.core.KeywordTokenizerFactory; import org.apache.lucene.analysis.core.LowerCaseFilterFactory; import org.apache.lucene.analysis.core.StopFilterFactory; @@ -35,7 +34,6 @@ import org.apache.lucene.analysis.standard.StandardTokenizerFactory; import org.hibernate.search.backend.lucene.analysis.LuceneAnalysisConfigurationContext; import org.hibernate.search.backend.lucene.analysis.LuceneAnalysisConfigurer; import org.springframework.stereotype.Component; -import org.springframework.stereotype.Service; /** * Factory for defining the analysers. diff --git a/hapi-fhir-jpaserver-model/pom.xml b/hapi-fhir-jpaserver-model/pom.xml index a33574e8ccf..20f587e4ab0 100644 --- a/hapi-fhir-jpaserver-model/pom.xml +++ b/hapi-fhir-jpaserver-model/pom.xml @@ -12,16 +12,16 @@ hapi-fhir-jpaserver-model jar - HAPI FHIR Model + HAPI FHIR JPA Model - + - org.fhir - ucum - - + org.fhir + ucum + + ca.uhn.hapi.fhir hapi-fhir-base @@ -71,10 +71,6 @@ xml-apis xml-apis - - org.jboss.spec.javax.transaction - jboss-transaction-api_1.2_spec - javax.activation activation diff --git a/hapi-fhir-structures-dstu2.1/pom.xml b/hapi-fhir-structures-dstu2.1/pom.xml index 8ab6ce22d2e..7b99ba1e215 100644 --- a/hapi-fhir-structures-dstu2.1/pom.xml +++ b/hapi-fhir-structures-dstu2.1/pom.xml @@ -235,16 +235,6 @@ jaxb-api test - - - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/hapi-fhir-structures-dstu2/pom.xml b/hapi-fhir-structures-dstu2/pom.xml index b8873469e41..e33032dd929 100644 --- a/hapi-fhir-structures-dstu2/pom.xml +++ b/hapi-fhir-structures-dstu2/pom.xml @@ -125,16 +125,6 @@ jaxb-api test - - - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/hapi-fhir-structures-dstu3/pom.xml b/hapi-fhir-structures-dstu3/pom.xml index 90d3d27520a..477963a6cdd 100644 --- a/hapi-fhir-structures-dstu3/pom.xml +++ b/hapi-fhir-structures-dstu3/pom.xml @@ -197,23 +197,6 @@ true - - - - com.github.ben-manes.caffeine @@ -292,27 +275,6 @@ javax.activation-api test - - javax.xml.bind - jaxb-api - test - - - - - - - - - - - - - org.glassfish.jaxb - jaxb-runtime - test - - diff --git a/hapi-fhir-structures-hl7org-dstu2/pom.xml b/hapi-fhir-structures-hl7org-dstu2/pom.xml index 61c226a13de..f60c3d14611 100644 --- a/hapi-fhir-structures-hl7org-dstu2/pom.xml +++ b/hapi-fhir-structures-hl7org-dstu2/pom.xml @@ -176,16 +176,6 @@ jaxb-api test - - - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/hapi-fhir-structures-r4/pom.xml b/hapi-fhir-structures-r4/pom.xml index efa1c54d01f..cfecffc5da3 100644 --- a/hapi-fhir-structures-r4/pom.xml +++ b/hapi-fhir-structures-r4/pom.xml @@ -245,16 +245,6 @@ jaxb-api test - - - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/hapi-fhir-validation/pom.xml b/hapi-fhir-validation/pom.xml index f4f609cda83..6122a2d5224 100644 --- a/hapi-fhir-validation/pom.xml +++ b/hapi-fhir-validation/pom.xml @@ -268,16 +268,6 @@ jaxb-api test - - - - - - - - - - org.glassfish.jaxb jaxb-runtime diff --git a/pom.xml b/pom.xml index f38a6af45b0..efc9b20b587 100644 --- a/pom.xml +++ b/pom.xml @@ -746,7 +746,7 @@ 2.2.11_1 2.3.1 2.3.0.1 - 2.3.1 + 3.0.0 3.16.0 3.0.0 @@ -892,12 +892,6 @@ com.helger ph-schematron ${ph_schematron_version} - - - jakarta.xml.bind - jakarta.xml.bind-api - - com.helger @@ -917,7 +911,7 @@ com.sun.activation jakarta.activation - 1.2.1 + 2.0.0 com.sun.mail From 749934d5f1e412bf69cf4c5b27bc0d2b222b7571 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Fri, 22 Jan 2021 15:39:26 -0500 Subject: [PATCH 02/12] fixed name text indexing (#2309) * fixed name text indexing * fixed name text indexing --- .../changelog/5_3_0/2309-name-text-search.yaml | 4 ++++ .../jpa/dao/r4/SearchParamExtractorR4Test.java | 16 ++++++++++++++++ .../extractor/BaseSearchParamExtractor.java | 6 ++++++ 3 files changed, 26 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2309-name-text-search.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2309-name-text-search.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2309-name-text-search.yaml new file mode 100644 index 00000000000..30bf181cd56 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2309-name-text-search.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2309 +title: "In the JPA server, HumanName.name.text was not being indexed and therefore was not searchable. This has been corrected." diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchParamExtractorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchParamExtractorR4Test.java index a680c5a27db..d29371959ad 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchParamExtractorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchParamExtractorR4Test.java @@ -30,6 +30,7 @@ import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Consent; import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.Extension; +import org.hl7.fhir.r4.model.HumanName; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Quantity; @@ -50,6 +51,8 @@ import java.util.Set; import java.util.stream.Collectors; import static java.util.Comparator.comparing; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -82,6 +85,19 @@ public class SearchParamExtractorR4Test { assertEquals("CODE", token.getValue()); } + @Test + public void testName() { + Patient patient = new Patient(); + HumanName humanName = patient.addName(); + humanName.addGiven("Jimmy"); + humanName.setFamily("Jones"); + humanName.setText("Jimmy Jones"); + SearchParamExtractorR4 extractor = new SearchParamExtractorR4(new ModelConfig(), new PartitionSettings(), ourCtx, mySearchParamRegistry); + ISearchParamExtractor.SearchParamSet stringSearchParams = extractor.extractSearchParamStrings(patient); + List nameValues = stringSearchParams.stream().filter(param -> "name".equals(param.getParamName())).map(ResourceIndexedSearchParamString::getValueExact).collect(Collectors.toList()); + assertThat(nameValues, containsInAnyOrder("Jimmy", "Jones", "Jimmy Jones")); + } + @Test public void testTokenOnSearchParamContext() { SearchParameter sp = new SearchParameter(); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index d2324ef102f..10c8bd2ce2d 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -132,6 +132,7 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor private BaseRuntimeChildDefinition myDurationValueValueChild; private BaseRuntimeChildDefinition myHumanNameFamilyValueChild; private BaseRuntimeChildDefinition myHumanNameGivenValueChild; + private BaseRuntimeChildDefinition myHumanNameTextValueChild; private BaseRuntimeChildDefinition myContactPointValueValueChild; private BaseRuntimeChildDefinition myIdentifierSystemValueChild; private BaseRuntimeChildDefinition myIdentifierValueValueChild; @@ -877,6 +878,10 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor for (String next : givens) { createStringIndexIfNotBlank(theResourceType, theParams, theSearchParam, next); } + List texts = extractValuesAsStrings(myHumanNameTextValueChild, theValue); + for (String next : texts) { + createStringIndexIfNotBlank(theResourceType, theParams, theSearchParam, next); + } } private void addString_Quantity(String theResourceType, Set theParams, RuntimeSearchParam theSearchParam, IBase theValue) { @@ -1138,6 +1143,7 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor BaseRuntimeElementCompositeDefinition humanNameDefinition = (BaseRuntimeElementCompositeDefinition) getContext().getElementDefinition("HumanName"); myHumanNameFamilyValueChild = humanNameDefinition.getChildByName("family"); myHumanNameGivenValueChild = humanNameDefinition.getChildByName("given"); + myHumanNameTextValueChild = humanNameDefinition.getChildByName("text"); BaseRuntimeElementCompositeDefinition contactPointDefinition = (BaseRuntimeElementCompositeDefinition) getContext().getElementDefinition("ContactPoint"); myContactPointValueValueChild = contactPointDefinition.getChildByName("value"); From 522efc87d91001e7c03bcd6b66ce7996686f7a63 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 22 Jan 2021 19:17:08 -0500 Subject: [PATCH 03/12] Reusable interceptor service (#2318) * Reusable interceptor service * Azure fix --- azure-pipelines.yml | 2 +- .../api/IBaseInterceptorBroadcaster.java | 50 ++ .../api/IBaseInterceptorService.java | 94 +++ .../api/IInterceptorBroadcaster.java | 27 +- .../interceptor/api/IInterceptorService.java | 72 +- .../uhn/fhir/interceptor/api/IPointcut.java | 17 + .../ca/uhn/fhir/interceptor/api/Pointcut.java | 5 +- .../executor/BaseInterceptorService.java | 621 ++++++++++++++++++ .../executor/InterceptorService.java | 537 +-------------- 9 files changed, 797 insertions(+), 628 deletions(-) create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorBroadcaster.java create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorService.java create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IPointcut.java create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/BaseInterceptorService.java diff --git a/azure-pipelines.yml b/azure-pipelines.yml index b026fee6b54..04c4ea0b33d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -52,7 +52,7 @@ jobs: inputs: targetPath: '$(Build.ArtifactStagingDirectory)/' artifactName: 'full_logs.zip' - - script: bash <(curl https://codecov.io/bash) -t $(CODECOV_TOKEN) + - script: bash <(curl https://codecov.io/bash) -C $(Build.SourceVersion) displayName: 'codecov' - task: PublishTestResults@2 inputs: diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorBroadcaster.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorBroadcaster.java new file mode 100644 index 00000000000..404112fa2e1 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorBroadcaster.java @@ -0,0 +1,50 @@ +package ca.uhn.fhir.interceptor.api; + +/*- + * #%L + * HAPI FHIR - Core Library + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +public interface IBaseInterceptorBroadcaster { + + /** + * Invoke registered interceptor hook methods for the given Pointcut. + * + * @return Returns false if any of the invoked hook methods returned + * false, and returns true otherwise. + */ + boolean callHooks(POINTCUT thePointcut, HookParams theParams); + + /** + * Invoke registered interceptor hook methods for the given Pointcut. This method + * should only be called for pointcuts that return a type other than + * void or boolean + * + * @return Returns the object returned by the first hook method that did not return null + */ + Object callHooksAndReturnObject(POINTCUT thePointcut, HookParams theParams); + + /** + * Does this broadcaster have any hooks for the given pointcut? + * + * @param thePointcut The poointcut + * @return Does this broadcaster have any hooks for the given pointcut? + * @since 4.0.0 + */ + boolean hasHooks(POINTCUT thePointcut); +} diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorService.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorService.java new file mode 100644 index 00000000000..b6efd6b7132 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IBaseInterceptorService.java @@ -0,0 +1,94 @@ +package ca.uhn.fhir.interceptor.api; + +/*- + * #%L + * HAPI FHIR - Core Library + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import javax.annotation.Nullable; +import java.util.Collection; +import java.util.List; +import java.util.function.Predicate; + +public interface IBaseInterceptorService extends IBaseInterceptorBroadcaster { + + /** + * Register an interceptor that will be used in a {@link ThreadLocal} context. + * This means that events will only be broadcast to the given interceptor if + * they were fired from the current thread. + *

+ * Note that it is almost always desirable to call this method with a + * try-finally statement that removes the interceptor afterwards, since + * this can lead to memory leakage, poor performance due to ever-increasing + * numbers of interceptors, etc. + *

+ *

+ * Note that most methods such as {@link #getAllRegisteredInterceptors()} and + * {@link #unregisterAllInterceptors()} do not affect thread local interceptors + * as they are kept in a separate list. + *

+ * + * @param theInterceptor The interceptor + * @return Returns true if at least one valid hook method was found on this interceptor + */ + boolean registerThreadLocalInterceptor(Object theInterceptor); + + /** + * Unregisters a ThreadLocal interceptor + * + * @param theInterceptor The interceptor + * @see #registerThreadLocalInterceptor(Object) + */ + void unregisterThreadLocalInterceptor(Object theInterceptor); + + /** + * Register an interceptor. This method has no effect if the given interceptor is already registered. + * + * @param theInterceptor The interceptor to register + * @return Returns true if at least one valid hook method was found on this interceptor + */ + boolean registerInterceptor(Object theInterceptor); + + /** + * Unregister an interceptor. This method has no effect if the given interceptor is not already registered. + * + * @param theInterceptor The interceptor to unregister + * @return Returns true if the interceptor was found and removed + */ + boolean unregisterInterceptor(Object theInterceptor); + + /** + * Returns all currently registered interceptors (excluding any thread local interceptors). + */ + List getAllRegisteredInterceptors(); + + /** + * Unregisters all registered interceptors. Note that this method does not unregister + * any {@link #registerThreadLocalInterceptor(Object) thread local interceptors}. + */ + void unregisterAllInterceptors(); + + void unregisterInterceptors(@Nullable Collection theInterceptors); + + void registerInterceptors(@Nullable Collection theInterceptors); + + /** + * Unregisters all interceptors that are indicated by the given callback function returning true + */ + void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction); +} diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorBroadcaster.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorBroadcaster.java index fd0ab5442fc..73e8add78a4 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorBroadcaster.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorBroadcaster.java @@ -20,31 +20,6 @@ package ca.uhn.fhir.interceptor.api; * #L% */ -public interface IInterceptorBroadcaster { +public interface IInterceptorBroadcaster extends IBaseInterceptorBroadcaster { - /** - * Invoke registered interceptor hook methods for the given Pointcut. - * - * @return Returns false if any of the invoked hook methods returned - * false, and returns true otherwise. - */ - boolean callHooks(Pointcut thePointcut, HookParams theParams); - - /** - * Invoke registered interceptor hook methods for the given Pointcut. This method - * should only be called for pointcuts that return a type other than - * void or boolean - * - * @return Returns the object returned by the first hook method that did not return null - */ - Object callHooksAndReturnObject(Pointcut thePointcut, HookParams theParams); - - /** - * Does this broadcaster have any hooks for the given pointcut? - * - * @param thePointcut The poointcut - * @return Does this broadcaster have any hooks for the given pointcut? - * @since 4.0.0 - */ - boolean hasHooks(Pointcut thePointcut); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorService.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorService.java index 6cc72d4de7e..876b515bc2f 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorService.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IInterceptorService.java @@ -20,80 +20,10 @@ package ca.uhn.fhir.interceptor.api; * #L% */ -import javax.annotation.Nullable; -import java.util.Collection; -import java.util.List; -import java.util.function.Function; -import java.util.function.Predicate; - -public interface IInterceptorService extends IInterceptorBroadcaster { - - /** - * Register an interceptor that will be used in a {@link ThreadLocal} context. - * This means that events will only be broadcast to the given interceptor if - * they were fired from the current thread. - *

- * Note that it is almost always desirable to call this method with a - * try-finally statement that removes the interceptor afterwards, since - * this can lead to memory leakage, poor performance due to ever-increasing - * numbers of interceptors, etc. - *

- *

- * Note that most methods such as {@link #getAllRegisteredInterceptors()} and - * {@link #unregisterAllInterceptors()} do not affect thread local interceptors - * as they are kept in a separate list. - *

- * - * @param theInterceptor The interceptor - * @return Returns true if at least one valid hook method was found on this interceptor - */ - boolean registerThreadLocalInterceptor(Object theInterceptor); - - /** - * Unregisters a ThreadLocal interceptor - * - * @param theInterceptor The interceptor - * @see #registerThreadLocalInterceptor(Object) - */ - void unregisterThreadLocalInterceptor(Object theInterceptor); - - /** - * Register an interceptor. This method has no effect if the given interceptor is already registered. - * - * @param theInterceptor The interceptor to register - * @return Returns true if at least one valid hook method was found on this interceptor - */ - boolean registerInterceptor(Object theInterceptor); - - /** - * Unregister an interceptor. This method has no effect if the given interceptor is not already registered. - * - * @param theInterceptor The interceptor to unregister - * @return Returns true if the interceptor was found and removed - */ - boolean unregisterInterceptor(Object theInterceptor); +public interface IInterceptorService extends IBaseInterceptorService, IInterceptorBroadcaster { void registerAnonymousInterceptor(Pointcut thePointcut, IAnonymousInterceptor theInterceptor); void registerAnonymousInterceptor(Pointcut thePointcut, int theOrder, IAnonymousInterceptor theInterceptor); - /** - * Returns all currently registered interceptors (excluding any thread local interceptors). - */ - List getAllRegisteredInterceptors(); - - /** - * Unregisters all registered interceptors. Note that this method does not unregister - * any {@link #registerThreadLocalInterceptor(Object) thread local interceptors}. - */ - void unregisterAllInterceptors(); - - void unregisterInterceptors(@Nullable Collection theInterceptors); - - void registerInterceptors(@Nullable Collection theInterceptors); - - /** - * Unregisters all interceptors that are indicated by the given callback function returning true - */ - void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IPointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IPointcut.java new file mode 100644 index 00000000000..d9abf1de3f9 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/IPointcut.java @@ -0,0 +1,17 @@ +package ca.uhn.fhir.interceptor.api; + +import javax.annotation.Nonnull; +import java.util.List; + +public interface IPointcut { + @Nonnull + Class getReturnType(); + + @Nonnull + List getParameterTypes(); + + @Nonnull + String name(); + + boolean isShouldLogAndSwallowException(Throwable theException); +} diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index 51562475365..0498e9585f8 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -48,7 +48,7 @@ import java.util.Set; * *

*/ -public enum Pointcut { +public enum Pointcut implements IPointcut { /** * Interceptor Framework Hook: @@ -2178,6 +2178,7 @@ public enum Pointcut { this(theReturnType, new ExceptionHandlingSpec(), theParameterTypes); } + @Override public boolean isShouldLogAndSwallowException(@Nonnull Throwable theException) { for (Class next : myExceptionHandlingSpec.myTypesToLogAndSwallow) { if (next.isAssignableFrom(theException.getClass())) { @@ -2187,11 +2188,13 @@ public enum Pointcut { return false; } + @Override @Nonnull public Class getReturnType() { return myReturnType; } + @Override @Nonnull public List getParameterTypes() { return myParameterTypes; diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/BaseInterceptorService.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/BaseInterceptorService.java new file mode 100644 index 00000000000..4458c7d6a91 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/BaseInterceptorService.java @@ -0,0 +1,621 @@ +package ca.uhn.fhir.interceptor.executor; + +/*- + * #%L + * HAPI FHIR - Core Library + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.interceptor.api.Hook; +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.IBaseInterceptorBroadcaster; +import ca.uhn.fhir.interceptor.api.IBaseInterceptorService; +import ca.uhn.fhir.interceptor.api.IPointcut; +import ca.uhn.fhir.interceptor.api.Interceptor; +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.util.ReflectionUtil; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Multimaps; +import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; +import org.apache.commons.lang3.reflect.MethodUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +public abstract class BaseInterceptorService implements IBaseInterceptorService, IBaseInterceptorBroadcaster { + private static final Logger ourLog = LoggerFactory.getLogger(BaseInterceptorService.class); + private final List myInterceptors = new ArrayList<>(); + private final ListMultimap myGlobalInvokers = ArrayListMultimap.create(); + private final ListMultimap myAnonymousInvokers = ArrayListMultimap.create(); + private final Object myRegistryMutex = new Object(); + private final ThreadLocal> myThreadlocalInvokers = new ThreadLocal<>(); + private String myName; + private boolean myThreadlocalInvokersEnabled = true; + + /** + * Constructor which uses a default name of "default" + */ + public BaseInterceptorService() { + this("default"); + } + + /** + * Constructor + * + * @param theName The name for this registry (useful for troubleshooting) + */ + public BaseInterceptorService(String theName) { + super(); + myName = theName; + } + + /** + * Are threadlocal interceptors enabled on this registry (defaults to true) + */ + public boolean isThreadlocalInvokersEnabled() { + return myThreadlocalInvokersEnabled; + } + + /** + * Are threadlocal interceptors enabled on this registry (defaults to true) + */ + public void setThreadlocalInvokersEnabled(boolean theThreadlocalInvokersEnabled) { + myThreadlocalInvokersEnabled = theThreadlocalInvokersEnabled; + } + + @VisibleForTesting + List getGlobalInterceptorsForUnitTest() { + return myInterceptors; + } + + public void setName(String theName) { + myName = theName; + } + + protected void registerAnonymousInterceptor(POINTCUT thePointcut, Object theInterceptor, BaseInvoker theInvoker) { + Validate.notNull(thePointcut); + Validate.notNull(theInterceptor); + synchronized (myRegistryMutex) { + + myAnonymousInvokers.put(thePointcut, theInvoker); + if (!isInterceptorAlreadyRegistered(theInterceptor)) { + myInterceptors.add(theInterceptor); + } + } + } + + @Override + public List getAllRegisteredInterceptors() { + synchronized (myRegistryMutex) { + List retVal = new ArrayList<>(); + retVal.addAll(myInterceptors); + return Collections.unmodifiableList(retVal); + } + } + + @Override + @VisibleForTesting + public void unregisterAllInterceptors() { + synchronized (myRegistryMutex) { + myAnonymousInvokers.clear(); + myGlobalInvokers.clear(); + myInterceptors.clear(); + } + } + + @Override + public void unregisterInterceptors(@Nullable Collection theInterceptors) { + if (theInterceptors != null) { + theInterceptors.forEach(t -> unregisterInterceptor(t)); + } + } + + @Override + public void registerInterceptors(@Nullable Collection theInterceptors) { + if (theInterceptors != null) { + theInterceptors.forEach(t -> registerInterceptor(t)); + } + } + + @Override + public void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction) { + unregisterInterceptorsIf(theShouldUnregisterFunction, myGlobalInvokers); + unregisterInterceptorsIf(theShouldUnregisterFunction, myAnonymousInvokers); + } + + private void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction, ListMultimap theGlobalInvokers) { + theGlobalInvokers.entries().removeIf(t -> theShouldUnregisterFunction.test(t.getValue().getInterceptor())); + } + + @Override + public boolean registerThreadLocalInterceptor(Object theInterceptor) { + if (!myThreadlocalInvokersEnabled) { + return false; + } + ListMultimap invokers = getThreadLocalInvokerMultimap(); + scanInterceptorAndAddToInvokerMultimap(theInterceptor, invokers); + return !invokers.isEmpty(); + + } + + @Override + public void unregisterThreadLocalInterceptor(Object theInterceptor) { + if (myThreadlocalInvokersEnabled) { + ListMultimap invokers = getThreadLocalInvokerMultimap(); + invokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); + if (invokers.isEmpty()) { + myThreadlocalInvokers.remove(); + } + } + } + + private ListMultimap getThreadLocalInvokerMultimap() { + ListMultimap invokers = myThreadlocalInvokers.get(); + if (invokers == null) { + invokers = Multimaps.synchronizedListMultimap(ArrayListMultimap.create()); + myThreadlocalInvokers.set(invokers); + } + return invokers; + } + + @Override + public boolean registerInterceptor(Object theInterceptor) { + synchronized (myRegistryMutex) { + + if (isInterceptorAlreadyRegistered(theInterceptor)) { + return false; + } + + List addedInvokers = scanInterceptorAndAddToInvokerMultimap(theInterceptor, myGlobalInvokers); + if (addedInvokers.isEmpty()) { + ourLog.warn("Interceptor registered with no valid hooks - Type was: {}", theInterceptor.getClass().getName()); + return false; + } + + // Add to the global list + myInterceptors.add(theInterceptor); + sortByOrderAnnotation(myInterceptors); + + return true; + } + } + + private boolean isInterceptorAlreadyRegistered(Object theInterceptor) { + for (Object next : myInterceptors) { + if (next == theInterceptor) { + return true; + } + } + return false; + } + + @Override + public boolean unregisterInterceptor(Object theInterceptor) { + synchronized (myRegistryMutex) { + boolean removed = myInterceptors.removeIf(t -> t == theInterceptor); + removed |= myGlobalInvokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); + removed |= myAnonymousInvokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); + return removed; + } + } + + private void sortByOrderAnnotation(List theObjects) { + IdentityHashMap interceptorToOrder = new IdentityHashMap<>(); + for (Object next : theObjects) { + Interceptor orderAnnotation = next.getClass().getAnnotation(Interceptor.class); + int order = orderAnnotation != null ? orderAnnotation.order() : 0; + interceptorToOrder.put(next, order); + } + + theObjects.sort((a, b) -> { + Integer orderA = interceptorToOrder.get(a); + Integer orderB = interceptorToOrder.get(b); + return orderA - orderB; + }); + } + + @Override + public Object callHooksAndReturnObject(POINTCUT thePointcut, HookParams theParams) { + assert haveAppropriateParams(thePointcut, theParams); + assert thePointcut.getReturnType() != void.class; + + return doCallHooks(thePointcut, theParams, null); + } + + @Override + public boolean hasHooks(POINTCUT thePointcut) { + return myGlobalInvokers.containsKey(thePointcut) + || myAnonymousInvokers.containsKey(thePointcut) + || hasThreadLocalHooks(thePointcut); + } + + private boolean hasThreadLocalHooks(POINTCUT thePointcut) { + ListMultimap hooks = myThreadlocalInvokersEnabled ? myThreadlocalInvokers.get() : null; + return hooks != null && hooks.containsKey(thePointcut); + } + + @Override + public boolean callHooks(POINTCUT thePointcut, HookParams theParams) { + assert haveAppropriateParams(thePointcut, theParams); + assert thePointcut.getReturnType() == void.class || thePointcut.getReturnType() == boolean.class; + + Object retValObj = doCallHooks(thePointcut, theParams, true); + return (Boolean) retValObj; + } + + private Object doCallHooks(POINTCUT thePointcut, HookParams theParams, Object theRetVal) { + List invokers = getInvokersForPointcut(thePointcut); + + /* + * Call each hook in order + */ + for (BaseInvoker nextInvoker : invokers) { + Object nextOutcome = nextInvoker.invoke(theParams); + Class pointcutReturnType = thePointcut.getReturnType(); + if (pointcutReturnType.equals(boolean.class)) { + Boolean nextOutcomeAsBoolean = (Boolean) nextOutcome; + if (Boolean.FALSE.equals(nextOutcomeAsBoolean)) { + ourLog.trace("callHooks({}) for invoker({}) returned false", thePointcut, nextInvoker); + theRetVal = false; + break; + } + } else if (pointcutReturnType.equals(void.class) == false) { + if (nextOutcome != null) { + theRetVal = nextOutcome; + break; + } + } + } + + return theRetVal; + } + + @VisibleForTesting + List getInterceptorsWithInvokersForPointcut(POINTCUT thePointcut) { + return getInvokersForPointcut(thePointcut) + .stream() + .map(BaseInvoker::getInterceptor) + .collect(Collectors.toList()); + } + + /** + * Returns an ordered list of invokers for the given pointcut. Note that + * a new and stable list is returned to.. do whatever you want with it. + */ + private List getInvokersForPointcut(POINTCUT thePointcut) { + List invokers; + + synchronized (myRegistryMutex) { + List globalInvokers = myGlobalInvokers.get(thePointcut); + List anonymousInvokers = myAnonymousInvokers.get(thePointcut); + List threadLocalInvokers = null; + if (myThreadlocalInvokersEnabled) { + ListMultimap pointcutToInvokers = myThreadlocalInvokers.get(); + if (pointcutToInvokers != null) { + threadLocalInvokers = pointcutToInvokers.get(thePointcut); + } + } + invokers = union(globalInvokers, anonymousInvokers, threadLocalInvokers); + } + + return invokers; + } + + /** + * First argument must be the global invoker list!! + */ + @SafeVarargs + private final List union(List... theInvokersLists) { + List haveOne = null; + boolean haveMultiple = false; + for (List nextInvokerList : theInvokersLists) { + if (nextInvokerList == null || nextInvokerList.isEmpty()) { + continue; + } + + if (haveOne == null) { + haveOne = nextInvokerList; + } else { + haveMultiple = true; + } + } + + if (haveOne == null) { + return Collections.emptyList(); + } + + List retVal; + + if (haveMultiple == false) { + + // The global list doesn't need to be sorted every time since it's sorted on + // insertion each time. Doing so is a waste of cycles.. + if (haveOne == theInvokersLists[0]) { + retVal = haveOne; + } else { + retVal = new ArrayList<>(haveOne); + retVal.sort(Comparator.naturalOrder()); + } + + } else { + + retVal = Arrays + .stream(theInvokersLists) + .filter(t -> t != null) + .flatMap(t -> t.stream()) + .sorted() + .collect(Collectors.toList()); + + } + + return retVal; + } + + /** + * Only call this when assertions are enabled, it's expensive + */ + boolean haveAppropriateParams(POINTCUT thePointcut, HookParams theParams) { + Validate.isTrue(theParams.getParamsForType().values().size() == thePointcut.getParameterTypes().size(), "Wrong number of params for pointcut %s - Wanted %s but found %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), theParams.getParamsForType().values().stream().map(t -> t != null ? t.getClass().getSimpleName() : "null").sorted().collect(Collectors.toList())); + + List wantedTypes = new ArrayList<>(thePointcut.getParameterTypes()); + + ListMultimap, Object> givenTypes = theParams.getParamsForType(); + for (Class nextTypeClass : givenTypes.keySet()) { + String nextTypeName = nextTypeClass.getName(); + for (Object nextParamValue : givenTypes.get(nextTypeClass)) { + Validate.isTrue(nextParamValue == null || nextTypeClass.isAssignableFrom(nextParamValue.getClass()), "Invalid params for pointcut %s - %s is not of type %s", thePointcut.name(), nextParamValue != null ? nextParamValue.getClass() : "null", nextTypeClass); + Validate.isTrue(wantedTypes.remove(nextTypeName), "Invalid params for pointcut %s - Wanted %s but found %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), nextTypeName); + } + } + + return true; + } + + private List scanInterceptorAndAddToInvokerMultimap(Object theInterceptor, ListMultimap theInvokers) { + Class interceptorClass = theInterceptor.getClass(); + int typeOrder = determineOrder(interceptorClass); + + List addedInvokers = scanInterceptorForHookMethods(theInterceptor, typeOrder); + + // Invoke the REGISTERED pointcut for any added hooks + addedInvokers.stream() + .filter(t -> Pointcut.INTERCEPTOR_REGISTERED.equals(t.getPointcut())) + .forEach(t -> t.invoke(new HookParams())); + + // Register the interceptor and its various hooks + for (HookInvoker nextAddedHook : addedInvokers) { + IPointcut nextPointcut = nextAddedHook.getPointcut(); + if (nextPointcut.equals(Pointcut.INTERCEPTOR_REGISTERED)) { + continue; + } + theInvokers.put((POINTCUT) nextPointcut, nextAddedHook); + } + + // Make sure we're always sorted according to the order declared in + // @Order + for (IPointcut nextPointcut : theInvokers.keys()) { + List nextInvokerList = theInvokers.get((POINTCUT) nextPointcut); + nextInvokerList.sort(Comparator.naturalOrder()); + } + + return addedInvokers; + } + + protected abstract static class BaseInvoker implements Comparable { + + private final int myOrder; + private final Object myInterceptor; + + BaseInvoker(Object theInterceptor, int theOrder) { + myInterceptor = theInterceptor; + myOrder = theOrder; + } + + public Object getInterceptor() { + return myInterceptor; + } + + abstract Object invoke(HookParams theParams); + + @Override + public int compareTo(BaseInvoker theInvoker) { + return myOrder - theInvoker.myOrder; + } + } + + private static class HookInvoker extends BaseInvoker { + + private final Method myMethod; + private final Class[] myParameterTypes; + private final int[] myParameterIndexes; + private final IPointcut myPointcut; + + /** + * Constructor + */ + private HookInvoker(HookDescriptor theHook, @Nonnull Object theInterceptor, @Nonnull Method theHookMethod, int theOrder) { + super(theInterceptor, theOrder); + myPointcut = theHook.getPointcut(); + myParameterTypes = theHookMethod.getParameterTypes(); + myMethod = theHookMethod; + + Class returnType = theHookMethod.getReturnType(); + if (myPointcut.getReturnType().equals(boolean.class)) { + Validate.isTrue(boolean.class.equals(returnType) || void.class.equals(returnType), "Method does not return boolean or void: %s", theHookMethod); + } else if (myPointcut.getReturnType().equals(void.class)) { + Validate.isTrue(void.class.equals(returnType), "Method does not return void: %s", theHookMethod); + } else { + Validate.isTrue(myPointcut.getReturnType().isAssignableFrom(returnType) || void.class.equals(returnType), "Method does not return %s or void: %s", myPointcut.getReturnType(), theHookMethod); + } + + myParameterIndexes = new int[myParameterTypes.length]; + Map, AtomicInteger> typeToCount = new HashMap<>(); + for (int i = 0; i < myParameterTypes.length; i++) { + AtomicInteger counter = typeToCount.computeIfAbsent(myParameterTypes[i], t -> new AtomicInteger(0)); + myParameterIndexes[i] = counter.getAndIncrement(); + } + + myMethod.setAccessible(true); + } + + @Override + public String toString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("method", myMethod) + .toString(); + } + + public IPointcut getPointcut() { + return myPointcut; + } + + /** + * @return Returns true/false if the hook method returns a boolean, returns true otherwise + */ + @Override + Object invoke(HookParams theParams) { + + Object[] args = new Object[myParameterTypes.length]; + for (int i = 0; i < myParameterTypes.length; i++) { + Class nextParamType = myParameterTypes[i]; + if (nextParamType.equals(Pointcut.class)) { + args[i] = myPointcut; + } else { + int nextParamIndex = myParameterIndexes[i]; + Object nextParamValue = theParams.get(nextParamType, nextParamIndex); + args[i] = nextParamValue; + } + } + + // Invoke the method + try { + return myMethod.invoke(getInterceptor(), args); + } catch (InvocationTargetException e) { + Throwable targetException = e.getTargetException(); + if (myPointcut.isShouldLogAndSwallowException(targetException)) { + ourLog.error("Exception thrown by interceptor: " + targetException.toString(), targetException); + return null; + } + + if (targetException instanceof RuntimeException) { + throw ((RuntimeException) targetException); + } else { + throw new InternalErrorException("Failure invoking interceptor for pointcut(s) " + getPointcut(), targetException); + } + } catch (Exception e) { + throw new InternalErrorException(e); + } + + } + + } + + /** + * @return Returns a list of any added invokers + */ + private List scanInterceptorForHookMethods(Object theInterceptor, int theTypeOrder) { + ArrayList retVal = new ArrayList<>(); + for (Method nextMethod : ReflectionUtil.getDeclaredMethods(theInterceptor.getClass(), true)) { + Optional hook = scanForHook(nextMethod); + + if (hook.isPresent()) { + int methodOrder = theTypeOrder; + int methodOrderAnnotation = hook.get().getOrder(); + if (methodOrderAnnotation != Interceptor.DEFAULT_ORDER) { + methodOrder = methodOrderAnnotation; + } + + retVal.add(new HookInvoker(hook.get(), theInterceptor, nextMethod, methodOrder)); + } + } + + return retVal; + } + + protected abstract Optional scanForHook(Method nextMethod); + + protected static Optional findAnnotation(AnnotatedElement theObject, Class theHookClass) { + T annotation; + if (theObject instanceof Method) { + annotation = MethodUtils.getAnnotation((Method) theObject, theHookClass, true, true); + } else { + annotation = theObject.getAnnotation(theHookClass); + } + return Optional.ofNullable(annotation); + } + + private static int determineOrder(Class theInterceptorClass) { + int typeOrder = Interceptor.DEFAULT_ORDER; + Optional typeOrderAnnotation = findAnnotation(theInterceptorClass, Interceptor.class); + if (typeOrderAnnotation.isPresent()) { + typeOrder = typeOrderAnnotation.get().order(); + } + return typeOrder; + } + + private static String toErrorString(List theParameterTypes) { + return theParameterTypes + .stream() + .sorted() + .collect(Collectors.joining(",")); + } + + protected static class HookDescriptor { + + private final IPointcut myPointcut; + private final int myOrder; + + HookDescriptor(IPointcut thePointcut, int theOrder) { + myPointcut = thePointcut; + myOrder = theOrder; + } + + IPointcut getPointcut() { + return myPointcut; + } + + int getOrder() { + return myOrder; + } + + } + +} diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java index 6b3b5e082fa..14a0d7fa459 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java @@ -27,39 +27,13 @@ import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.util.ReflectionUtil; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ListMultimap; -import com.google.common.collect.Multimaps; import org.apache.commons.lang3.Validate; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; -import org.apache.commons.lang3.reflect.MethodUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.lang.annotation.Annotation; -import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.*; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Predicate; -import java.util.stream.Collectors; +import java.util.Optional; -public class InterceptorService implements IInterceptorService, IInterceptorBroadcaster { - private static final Logger ourLog = LoggerFactory.getLogger(InterceptorService.class); - private final List myInterceptors = new ArrayList<>(); - private final ListMultimap myGlobalInvokers = ArrayListMultimap.create(); - private final ListMultimap myAnonymousInvokers = ArrayListMultimap.create(); - private final Object myRegistryMutex = new Object(); - private final ThreadLocal> myThreadlocalInvokers = new ThreadLocal<>(); - private String myName; - private boolean myThreadlocalInvokersEnabled = true; +public class InterceptorService extends BaseInterceptorService implements IInterceptorService, IInterceptorBroadcaster { /** * Constructor which uses a default name of "default" @@ -74,28 +48,14 @@ public class InterceptorService implements IInterceptorService, IInterceptorBroa * @param theName The name for this registry (useful for troubleshooting) */ public InterceptorService(String theName) { - super(); - myName = theName; + super(theName); } - /** - * Are threadlocal interceptors enabled on this registry (defaults to true) - */ - public boolean isThreadlocalInvokersEnabled() { - return myThreadlocalInvokersEnabled; + @Override + protected Optional scanForHook(Method nextMethod) { + return findAnnotation(nextMethod, Hook.class).map(t -> new HookDescriptor(t.value(), t.order())); } - /** - * Are threadlocal interceptors enabled on this registry (defaults to true) - */ - public void setThreadlocalInvokersEnabled(boolean theThreadlocalInvokersEnabled) { - myThreadlocalInvokersEnabled = theThreadlocalInvokersEnabled; - } - - @VisibleForTesting - List getGlobalInterceptorsForUnitTest() { - return myInterceptors; - } @Override @VisibleForTesting @@ -103,309 +63,14 @@ public class InterceptorService implements IInterceptorService, IInterceptorBroa registerAnonymousInterceptor(thePointcut, Interceptor.DEFAULT_ORDER, theInterceptor); } - public void setName(String theName) { - myName = theName; - } - @Override public void registerAnonymousInterceptor(Pointcut thePointcut, int theOrder, IAnonymousInterceptor theInterceptor) { Validate.notNull(thePointcut); Validate.notNull(theInterceptor); - synchronized (myRegistryMutex) { - - myAnonymousInvokers.put(thePointcut, new AnonymousLambdaInvoker(thePointcut, theInterceptor, theOrder)); - if (!isInterceptorAlreadyRegistered(theInterceptor)) { - myInterceptors.add(theInterceptor); - } - } + BaseInvoker invoker = new AnonymousLambdaInvoker(thePointcut, theInterceptor, theOrder); + registerAnonymousInterceptor(thePointcut, theInterceptor, invoker); } - @Override - public List getAllRegisteredInterceptors() { - synchronized (myRegistryMutex) { - List retVal = new ArrayList<>(); - retVal.addAll(myInterceptors); - return Collections.unmodifiableList(retVal); - } - } - - @Override - @VisibleForTesting - public void unregisterAllInterceptors() { - synchronized (myRegistryMutex) { - myAnonymousInvokers.clear(); - myGlobalInvokers.clear(); - myInterceptors.clear(); - } - } - - @Override - public void unregisterInterceptors(@Nullable Collection theInterceptors) { - if (theInterceptors != null) { - theInterceptors.forEach(t -> unregisterInterceptor(t)); - } - } - - @Override - public void registerInterceptors(@Nullable Collection theInterceptors) { - if (theInterceptors != null) { - theInterceptors.forEach(t -> registerInterceptor(t)); - } - } - - @Override - public void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction) { - unregisterInterceptorsIf(theShouldUnregisterFunction, myGlobalInvokers); - unregisterInterceptorsIf(theShouldUnregisterFunction, myAnonymousInvokers); - } - - private void unregisterInterceptorsIf(Predicate theShouldUnregisterFunction, ListMultimap theGlobalInvokers) { - theGlobalInvokers.entries().removeIf(t->theShouldUnregisterFunction.test(t.getValue().getInterceptor())); - } - - @Override - public boolean registerThreadLocalInterceptor(Object theInterceptor) { - if (!myThreadlocalInvokersEnabled) { - return false; - } - ListMultimap invokers = getThreadLocalInvokerMultimap(); - scanInterceptorAndAddToInvokerMultimap(theInterceptor, invokers); - return !invokers.isEmpty(); - - } - - @Override - public void unregisterThreadLocalInterceptor(Object theInterceptor) { - if (myThreadlocalInvokersEnabled) { - ListMultimap invokers = getThreadLocalInvokerMultimap(); - invokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); - if (invokers.isEmpty()) { - myThreadlocalInvokers.remove(); - } - } - } - - private ListMultimap getThreadLocalInvokerMultimap() { - ListMultimap invokers = myThreadlocalInvokers.get(); - if (invokers == null) { - invokers = Multimaps.synchronizedListMultimap(ArrayListMultimap.create()); - myThreadlocalInvokers.set(invokers); - } - return invokers; - } - - @Override - public boolean registerInterceptor(Object theInterceptor) { - synchronized (myRegistryMutex) { - - if (isInterceptorAlreadyRegistered(theInterceptor)) { - return false; - } - - List addedInvokers = scanInterceptorAndAddToInvokerMultimap(theInterceptor, myGlobalInvokers); - if (addedInvokers.isEmpty()) { - ourLog.warn("Interceptor registered with no valid hooks - Type was: {}", theInterceptor.getClass().getName()); - return false; - } - - // Add to the global list - myInterceptors.add(theInterceptor); - sortByOrderAnnotation(myInterceptors); - - return true; - } - } - - private boolean isInterceptorAlreadyRegistered(Object theInterceptor) { - for (Object next : myInterceptors) { - if (next == theInterceptor) { - return true; - } - } - return false; - } - - @Override - public boolean unregisterInterceptor(Object theInterceptor) { - synchronized (myRegistryMutex) { - boolean removed = myInterceptors.removeIf(t -> t == theInterceptor); - removed |= myGlobalInvokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); - removed |= myAnonymousInvokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); - return removed; - } - } - - private void sortByOrderAnnotation(List theObjects) { - IdentityHashMap interceptorToOrder = new IdentityHashMap<>(); - for (Object next : theObjects) { - Interceptor orderAnnotation = next.getClass().getAnnotation(Interceptor.class); - int order = orderAnnotation != null ? orderAnnotation.order() : 0; - interceptorToOrder.put(next, order); - } - - theObjects.sort((a, b) -> { - Integer orderA = interceptorToOrder.get(a); - Integer orderB = interceptorToOrder.get(b); - return orderA - orderB; - }); - } - - @Override - public Object callHooksAndReturnObject(Pointcut thePointcut, HookParams theParams) { - assert haveAppropriateParams(thePointcut, theParams); - assert thePointcut.getReturnType() != void.class; - - return doCallHooks(thePointcut, theParams, null); - } - - @Override - public boolean hasHooks(Pointcut thePointcut) { - return myGlobalInvokers.containsKey(thePointcut) - || myAnonymousInvokers.containsKey(thePointcut) - || hasThreadLocalHooks(thePointcut); - } - - private boolean hasThreadLocalHooks(Pointcut thePointcut) { - ListMultimap hooks = myThreadlocalInvokersEnabled ? myThreadlocalInvokers.get() : null; - return hooks != null && hooks.containsKey(thePointcut); - } - - @Override - public boolean callHooks(Pointcut thePointcut, HookParams theParams) { - assert haveAppropriateParams(thePointcut, theParams); - assert thePointcut.getReturnType() == void.class || thePointcut.getReturnType() == boolean.class; - - Object retValObj = doCallHooks(thePointcut, theParams, true); - return (Boolean) retValObj; - } - - private Object doCallHooks(Pointcut thePointcut, HookParams theParams, Object theRetVal) { - List invokers = getInvokersForPointcut(thePointcut); - - /* - * Call each hook in order - */ - for (BaseInvoker nextInvoker : invokers) { - Object nextOutcome = nextInvoker.invoke(theParams); - Class pointcutReturnType = thePointcut.getReturnType(); - if (pointcutReturnType.equals(boolean.class)) { - Boolean nextOutcomeAsBoolean = (Boolean) nextOutcome; - if (Boolean.FALSE.equals(nextOutcomeAsBoolean)) { - ourLog.trace("callHooks({}) for invoker({}) returned false", thePointcut, nextInvoker); - theRetVal = false; - break; - } - } else if (pointcutReturnType.equals(void.class) == false) { - if (nextOutcome != null) { - theRetVal = nextOutcome; - break; - } - } - } - - return theRetVal; - } - - @VisibleForTesting - List getInterceptorsWithInvokersForPointcut(Pointcut thePointcut) { - return getInvokersForPointcut(thePointcut) - .stream() - .map(BaseInvoker::getInterceptor) - .collect(Collectors.toList()); - } - - /** - * Returns an ordered list of invokers for the given pointcut. Note that - * a new and stable list is returned to.. do whatever you want with it. - */ - private List getInvokersForPointcut(Pointcut thePointcut) { - List invokers; - - synchronized (myRegistryMutex) { - List globalInvokers = myGlobalInvokers.get(thePointcut); - List anonymousInvokers = myAnonymousInvokers.get(thePointcut); - List threadLocalInvokers = null; - if (myThreadlocalInvokersEnabled) { - ListMultimap pointcutToInvokers = myThreadlocalInvokers.get(); - if (pointcutToInvokers != null) { - threadLocalInvokers = pointcutToInvokers.get(thePointcut); - } - } - invokers = union(globalInvokers, anonymousInvokers, threadLocalInvokers); - } - - return invokers; - } - - /** - * First argument must be the global invoker list!! - */ - @SafeVarargs - private final List union(List... theInvokersLists) { - List haveOne = null; - boolean haveMultiple = false; - for (List nextInvokerList : theInvokersLists) { - if (nextInvokerList == null || nextInvokerList.isEmpty()) { - continue; - } - - if (haveOne == null) { - haveOne = nextInvokerList; - } else { - haveMultiple = true; - } - } - - if (haveOne == null) { - return Collections.emptyList(); - } - - List retVal; - - if (haveMultiple == false) { - - // The global list doesn't need to be sorted every time since it's sorted on - // insertion each time. Doing so is a waste of cycles.. - if (haveOne == theInvokersLists[0]) { - retVal = haveOne; - } else { - retVal = new ArrayList<>(haveOne); - retVal.sort(Comparator.naturalOrder()); - } - - } else { - - retVal = Arrays - .stream(theInvokersLists) - .filter(t -> t != null) - .flatMap(t -> t.stream()) - .sorted() - .collect(Collectors.toList()); - - } - - return retVal; - } - - /** - * Only call this when assertions are enabled, it's expensive - */ - boolean haveAppropriateParams(Pointcut thePointcut, HookParams theParams) { - Validate.isTrue(theParams.getParamsForType().values().size() == thePointcut.getParameterTypes().size(), "Wrong number of params for pointcut %s - Wanted %s but found %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), theParams.getParamsForType().values().stream().map(t -> t != null ? t.getClass().getSimpleName() : "null").sorted().collect(Collectors.toList())); - - List wantedTypes = new ArrayList<>(thePointcut.getParameterTypes()); - - ListMultimap, Object> givenTypes = theParams.getParamsForType(); - for (Class nextTypeClass : givenTypes.keySet()) { - String nextTypeName = nextTypeClass.getName(); - for (Object nextParamValue : givenTypes.get(nextTypeClass)) { - Validate.isTrue(nextParamValue == null || nextTypeClass.isAssignableFrom(nextParamValue.getClass()), "Invalid params for pointcut %s - %s is not of type %s", thePointcut.name(), nextParamValue != null ? nextParamValue.getClass() : "null", nextTypeClass); - Validate.isTrue(wantedTypes.remove(nextTypeName), "Invalid params for pointcut %s - Wanted %s but found %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), nextTypeName); - } - } - - return true; - } private class AnonymousLambdaInvoker extends BaseInvoker { private final IAnonymousInterceptor myHook; @@ -424,191 +89,5 @@ public class InterceptorService implements IInterceptorService, IInterceptorBroa } } - private abstract static class BaseInvoker implements Comparable { - - private final int myOrder; - private final Object myInterceptor; - - BaseInvoker(Object theInterceptor, int theOrder) { - myInterceptor = theInterceptor; - myOrder = theOrder; - } - - public Object getInterceptor() { - return myInterceptor; - } - - abstract Object invoke(HookParams theParams); - - @Override - public int compareTo(BaseInvoker theInvoker) { - return myOrder - theInvoker.myOrder; - } - } - - private static class HookInvoker extends BaseInvoker { - - private final Method myMethod; - private final Class[] myParameterTypes; - private final int[] myParameterIndexes; - private final Pointcut myPointcut; - - /** - * Constructor - */ - private HookInvoker(Hook theHook, @Nonnull Object theInterceptor, @Nonnull Method theHookMethod, int theOrder) { - super(theInterceptor, theOrder); - myPointcut = theHook.value(); - myParameterTypes = theHookMethod.getParameterTypes(); - myMethod = theHookMethod; - - Class returnType = theHookMethod.getReturnType(); - if (myPointcut.getReturnType().equals(boolean.class)) { - Validate.isTrue(boolean.class.equals(returnType) || void.class.equals(returnType), "Method does not return boolean or void: %s", theHookMethod); - } else if (myPointcut.getReturnType().equals(void.class)) { - Validate.isTrue(void.class.equals(returnType), "Method does not return void: %s", theHookMethod); - } else { - Validate.isTrue(myPointcut.getReturnType().isAssignableFrom(returnType) || void.class.equals(returnType), "Method does not return %s or void: %s", myPointcut.getReturnType(), theHookMethod); - } - - myParameterIndexes = new int[myParameterTypes.length]; - Map, AtomicInteger> typeToCount = new HashMap<>(); - for (int i = 0; i < myParameterTypes.length; i++) { - AtomicInteger counter = typeToCount.computeIfAbsent(myParameterTypes[i], t -> new AtomicInteger(0)); - myParameterIndexes[i] = counter.getAndIncrement(); - } - - myMethod.setAccessible(true); - } - - @Override - public String toString() { - return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) - .append("method", myMethod) - .toString(); - } - - public Pointcut getPointcut() { - return myPointcut; - } - - /** - * @return Returns true/false if the hook method returns a boolean, returns true otherwise - */ - @Override - Object invoke(HookParams theParams) { - - Object[] args = new Object[myParameterTypes.length]; - for (int i = 0; i < myParameterTypes.length; i++) { - Class nextParamType = myParameterTypes[i]; - if (nextParamType.equals(Pointcut.class)) { - args[i] = myPointcut; - } else { - int nextParamIndex = myParameterIndexes[i]; - Object nextParamValue = theParams.get(nextParamType, nextParamIndex); - args[i] = nextParamValue; - } - } - - // Invoke the method - try { - return myMethod.invoke(getInterceptor(), args); - } catch (InvocationTargetException e) { - Throwable targetException = e.getTargetException(); - if (myPointcut.isShouldLogAndSwallowException(targetException)) { - ourLog.error("Exception thrown by interceptor: " + targetException.toString(), targetException); - return null; - } - - if (targetException instanceof RuntimeException) { - throw ((RuntimeException) targetException); - } else { - throw new InternalErrorException("Failure invoking interceptor for pointcut(s) " + getPointcut(), targetException); - } - } catch (Exception e) { - throw new InternalErrorException(e); - } - - } - - } - - private static List scanInterceptorAndAddToInvokerMultimap(Object theInterceptor, ListMultimap theInvokers) { - Class interceptorClass = theInterceptor.getClass(); - int typeOrder = determineOrder(interceptorClass); - - List addedInvokers = scanInterceptorForHookMethods(theInterceptor, typeOrder); - - // Invoke the REGISTERED pointcut for any added hooks - addedInvokers.stream() - .filter(t -> Pointcut.INTERCEPTOR_REGISTERED.equals(t.getPointcut())) - .forEach(t -> t.invoke(new HookParams())); - - // Register the interceptor and its various hooks - for (HookInvoker nextAddedHook : addedInvokers) { - Pointcut nextPointcut = nextAddedHook.getPointcut(); - if (nextPointcut.equals(Pointcut.INTERCEPTOR_REGISTERED)) { - continue; - } - theInvokers.put(nextPointcut, nextAddedHook); - } - - // Make sure we're always sorted according to the order declared in - // @Order - for (Pointcut nextPointcut : theInvokers.keys()) { - List nextInvokerList = theInvokers.get(nextPointcut); - nextInvokerList.sort(Comparator.naturalOrder()); - } - - return addedInvokers; - } - - /** - * @return Returns a list of any added invokers - */ - private static List scanInterceptorForHookMethods(Object theInterceptor, int theTypeOrder) { - ArrayList retVal = new ArrayList<>(); - for (Method nextMethod : ReflectionUtil.getDeclaredMethods(theInterceptor.getClass(), true)) { - Optional hook = findAnnotation(nextMethod, Hook.class); - - if (hook.isPresent()) { - int methodOrder = theTypeOrder; - int methodOrderAnnotation = hook.get().order(); - if (methodOrderAnnotation != Interceptor.DEFAULT_ORDER) { - methodOrder = methodOrderAnnotation; - } - - retVal.add(new HookInvoker(hook.get(), theInterceptor, nextMethod, methodOrder)); - } - } - - return retVal; - } - - private static Optional findAnnotation(AnnotatedElement theObject, Class theHookClass) { - T annotation; - if (theObject instanceof Method) { - annotation = MethodUtils.getAnnotation((Method) theObject, theHookClass, true, true); - } else { - annotation = theObject.getAnnotation(theHookClass); - } - return Optional.ofNullable(annotation); - } - - private static int determineOrder(Class theInterceptorClass) { - int typeOrder = Interceptor.DEFAULT_ORDER; - Optional typeOrderAnnotation = findAnnotation(theInterceptorClass, Interceptor.class); - if (typeOrderAnnotation.isPresent()) { - typeOrder = typeOrderAnnotation.get().order(); - } - return typeOrder; - } - - private static String toErrorString(List theParameterTypes) { - return theParameterTypes - .stream() - .sorted() - .collect(Collectors.joining(",")); - } } From 1d1ebcf5e8fc80829b105f9b715aedcd0ed76dfd Mon Sep 17 00:00:00 2001 From: Frank Tao <38163583+frankjtao@users.noreply.github.com> Date: Sat, 23 Jan 2021 17:53:18 -0500 Subject: [PATCH 04/12] Improved DateQuery and removed 'OR' in the SQL (#2302) * improved DateQuery and removed 'OR' in the SQL * improved DateQuery and removed 'OR' in the SQL * Date handling cleanup (#2316) * Date handling cleanup * Fix codecov * One more fix * Fix tests Co-authored-by: James Agnew --- .../ca/uhn/fhir/rest/param/DateParam.java | 1 + .../dao/predicate/PredicateBuilderDate.java | 74 +- .../predicate/PredicateBuilderReference.java | 125 +- .../fhir/jpa/search/builder/QueryStack.java | 75 +- .../predicate/DatePredicateBuilder.java | 61 +- ...rceDaoR4FilterLegacySearchBuilderTest.java | 1264 +++++++++++++++++ .../dao/r4/FhirResourceDaoR4FilterTest.java | 4 + ...rResourceDaoR4LegacySearchBuilderTest.java | 4 +- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 5 +- .../jpa/dao/r4/PartitioningSqlR4Test.java | 12 +- 10 files changed, 1471 insertions(+), 154 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterLegacySearchBuilderTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java index 8be700cc402..6c713472328 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java @@ -50,6 +50,7 @@ public class DateParam extends BaseParamWithPrefix implements /*IQuer * Constructor */ public DateParam() { + super(); } /** diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index a86d7f8d450..0895b33f55a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -126,6 +126,9 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi if (theParam instanceof DateParam) { DateParam date = (DateParam) theParam; if (!date.isEmpty()) { + if (theOperation == SearchFilterParser.CompareOperation.ne) { + date = new DateParam(ParamPrefixEnum.EQUAL, date.getValueAsString()); + } DateRangeParam range = new DateRangeParam(date); p = createPredicateDateFromRange(theBuilder, theFrom, @@ -152,6 +155,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi return theDateParam == null || theDateParam.getPrecision().ordinal() == TemporalPrecisionEnum.DAY.ordinal(); } + @SuppressWarnings("unchecked") private Predicate createPredicateDateFromRange(CriteriaBuilder theBuilder, From theFrom, DateRangeParam theRange, @@ -191,36 +195,60 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } if (operation == SearchFilterParser.CompareOperation.lt) { - if (lowerBoundInstant == null) { - throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); - } - //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - lb = theBuilder.lessThan(theFrom.get(lowValueField), genericLowerBound); - } else if (operation == SearchFilterParser.CompareOperation.le) { - if (upperBoundInstant == null) { - throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); - } - //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - lb = theBuilder.lessThanOrEqualTo(theFrom.get(highValueField), genericUpperBound); - } else if (operation == SearchFilterParser.CompareOperation.gt) { - if (upperBoundInstant == null) { - throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); - } - lb = theBuilder.greaterThan(theFrom.get(highValueField), genericUpperBound); - } else if (operation == SearchFilterParser.CompareOperation.ge) { - if (lowerBoundInstant == null) { - throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); + // use lower bound first + if (lowerBoundInstant != null) { + // the value has been reduced one in this case + lb = theBuilder.lessThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); + } else { + if (upperBoundInstant != null) { + ub = theBuilder.lessThanOrEqualTo(theFrom.get(lowValueField), genericUpperBound); + } else { + throw new InvalidRequestException("lowerBound and upperBound value not correctly specified for compare theOperation"); } - lb = theBuilder.greaterThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); - } else if (operation == SearchFilterParser.CompareOperation.ne) { + } + } else if (operation == SearchFilterParser.CompareOperation.le) { + // use lower bound first + if (lowerBoundInstant != null) { + lb = theBuilder.lessThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); + } else { + if (upperBoundInstant != null) { + ub = theBuilder.lessThanOrEqualTo(theFrom.get(lowValueField), genericUpperBound); + } else { + throw new InvalidRequestException("lowerBound and upperBound value not correctly specified for compare theOperation"); + } + } + } else if (operation == SearchFilterParser.CompareOperation.gt) { + // use upper bound first, e.g value between 6 and 10 + // gt7 true, 10>7, gt11 false, 10>11 false, gt5 true, 10>5 + if (upperBoundInstant != null) { + ub = theBuilder.greaterThanOrEqualTo(theFrom.get(highValueField), genericUpperBound); + } else { + if (lowerBoundInstant != null) { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get(highValueField), genericLowerBound); + } else { + throw new InvalidRequestException("upperBound and lowerBound value not correctly specified for compare theOperation"); + } + } + } else if (operation == SearchFilterParser.CompareOperation.ge) { + // use upper bound first, e.g value between 6 and 10 + // gt7 true, 10>7, gt11 false, 10>11 false, gt5 true, 10>5 + if (upperBoundInstant != null) { + ub = theBuilder.greaterThanOrEqualTo(theFrom.get(highValueField), genericUpperBound);; + } else { + if (lowerBoundInstant != null) { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get(highValueField), genericLowerBound); + } else { + throw new InvalidRequestException("upperBound and lowerBound value not correctly specified for compare theOperation"); + } + } + } else if (operation == SearchFilterParser.CompareOperation.ne) { if ((lowerBoundInstant == null) || (upperBoundInstant == null)) { throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } lt = theBuilder.lessThan(theFrom.get(lowValueField), genericLowerBound); gt = theBuilder.greaterThan(theFrom.get(highValueField), genericUpperBound); - lb = theBuilder.or(lt, - gt); + lb = theBuilder.or(lt, gt); } else if ((operation == SearchFilterParser.CompareOperation.eq) || (operation == null)) { if (lowerBoundInstant != null) { gt = theBuilder.greaterThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java index 2012fe91baf..a973b5f52c2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java @@ -102,6 +102,7 @@ import java.util.ListIterator; import java.util.Set; import java.util.stream.Collectors; +import static ca.uhn.fhir.jpa.search.builder.QueryStack.fromOperation; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.apache.commons.lang3.StringUtils.trim; @@ -267,12 +268,12 @@ class PredicateBuilderReference extends BasePredicateBuilder { private Predicate addPredicateReferenceWithChain(String theResourceName, String theParamName, List theList, From theJoin, List theCodePredicates, ReferenceParam theReferenceParam, RequestDetails theRequest, RequestPartitionId theRequestPartitionId) { /* - * Which resource types can the given chained parameter actually link to? This might be a list - * where the chain is unqualified, as in: Observation?subject.identifier=(...) - * since subject can link to several possible target types. - * - * If the user has qualified the chain, as in: Observation?subject:Patient.identifier=(...) - * this is just a simple 1-entry list. + * Which resource types can the given chained parameter actually link to? This might be a list + * where the chain is unqualified, as in: Observation?subject.identifier=(...) + * since subject can link to several possible target types. + * + * If the user has qualified the chain, as in: Observation?subject:Patient.identifier=(...) + * this is just a simple 1-entry list. */ final List> resourceTypes = determineCandidateResourceTypesForChain(theResourceName, theParamName, theReferenceParam); @@ -593,7 +594,14 @@ class PredicateBuilderReference extends BasePredicateBuilder { switch (nextParamDef.getParamType()) { case DATE: for (List nextAnd : theAndOrParams) { - myPredicateBuilder.addPredicateDate(theResourceName, nextParamDef, nextAnd, null, theRequestPartitionId); + // FT: 2021-01-18 use operation 'gt', 'ge', 'le' or 'lt' + // to create the predicateDate instead of generic one with operation = null + SearchFilterParser.CompareOperation operation = null; + if (nextAnd.size() > 0) { + DateParam param = (DateParam) nextAnd.get(0); + operation = ca.uhn.fhir.jpa.search.builder.QueryStack.toOperation(param.getPrefix()); + } + myPredicateBuilder.addPredicateDate(theResourceName, nextParamDef, nextAnd, operation, theRequestPartitionId); } break; case QUANTITY: @@ -716,67 +724,58 @@ class PredicateBuilderReference extends BasePredicateBuilder { private Predicate processFilterParameter(SearchFilterParser.FilterParameter theFilter, String theResourceName, RequestDetails theRequest, RequestPartitionId theRequestPartitionId) { + if (theFilter.getParamPath().getName().equals(Constants.PARAM_SOURCE)) { + TokenParam param = new TokenParam(); + param.setValueAsQueryToken(null, null, null, theFilter.getValue()); + return addPredicateSource(Collections.singletonList(param), theFilter.getOperation(), theRequest); + } else if (theFilter.getParamPath().getName().equals(IAnyResource.SP_RES_ID)) { + TokenParam param = new TokenParam(); + param.setValueAsQueryToken(null, + null, + null, + theFilter.getValue()); + return myPredicateBuilder.addPredicateResourceId(Collections.singletonList(Collections.singletonList(param)), myResourceName, theFilter.getOperation(), theRequestPartitionId); + } else if (theFilter.getParamPath().getName().equals(IAnyResource.SP_RES_LANGUAGE)) { + return addPredicateLanguage(Collections.singletonList(Collections.singletonList(new StringParam(theFilter.getValue()))), + theFilter.getOperation()); + } + RuntimeSearchParam searchParam = mySearchParamRegistry.getActiveSearchParam(theResourceName, theFilter.getParamPath().getName()); if (searchParam == null) { throw new InvalidRequestException("Invalid search parameter specified, " + theFilter.getParamPath().getName() + ", for resource type " + theResourceName); - } else if (searchParam.getName().equals(IAnyResource.SP_RES_ID)) { - if (searchParam.getParamType() == RestSearchParameterTypeEnum.TOKEN) { - TokenParam param = new TokenParam(); - param.setValueAsQueryToken(null, - null, - null, - theFilter.getValue()); - return myPredicateBuilder.addPredicateResourceId(Collections.singletonList(Collections.singletonList(param)), myResourceName, theFilter.getOperation(), theRequestPartitionId); - } else { - throw new InvalidRequestException("Unexpected search parameter type encountered, expected token type for _id search"); - } - } else if (searchParam.getName().equals(IAnyResource.SP_RES_LANGUAGE)) { - if (searchParam.getParamType() == RestSearchParameterTypeEnum.STRING) { - return addPredicateLanguage(Collections.singletonList(Collections.singletonList(new StringParam(theFilter.getValue()))), - theFilter.getOperation()); - } else { - throw new InvalidRequestException("Unexpected search parameter type encountered, expected string type for language search"); - } - } else if (searchParam.getName().equals(Constants.PARAM_SOURCE)) { - if (searchParam.getParamType() == RestSearchParameterTypeEnum.TOKEN) { - TokenParam param = new TokenParam(); - param.setValueAsQueryToken(null, null, null, theFilter.getValue()); - return addPredicateSource(Collections.singletonList(param), theFilter.getOperation(), theRequest); - } else { - throw new InvalidRequestException("Unexpected search parameter type encountered, expected token type for _id search"); - } - } else { - RestSearchParameterTypeEnum typeEnum = searchParam.getParamType(); - if (typeEnum == RestSearchParameterTypeEnum.URI) { - return myPredicateBuilder.addPredicateUri(theResourceName, searchParam, Collections.singletonList(new UriParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.STRING) { - return myPredicateBuilder.addPredicateString(theResourceName, searchParam, Collections.singletonList(new StringParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.DATE) { - return myPredicateBuilder.addPredicateDate(theResourceName, searchParam, Collections.singletonList(new DateParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.NUMBER) { - return myPredicateBuilder.addPredicateNumber(theResourceName, searchParam, Collections.singletonList(new NumberParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.REFERENCE) { - String paramName = theFilter.getParamPath().getName(); - SearchFilterParser.CompareOperation operation = theFilter.getOperation(); - String resourceType = null; // The value can either have (Patient/123) or not have (123) a resource type, either way it's not needed here - String chain = (theFilter.getParamPath().getNext() != null) ? theFilter.getParamPath().getNext().toString() : null; - String value = theFilter.getValue(); - ReferenceParam referenceParam = new ReferenceParam(resourceType, chain, value); - return addPredicate(theResourceName, paramName, Collections.singletonList(referenceParam), operation, theRequest, theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.QUANTITY) { - return myPredicateBuilder.addPredicateQuantity(theResourceName, searchParam, Collections.singletonList(new QuantityParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); - } else if (typeEnum == RestSearchParameterTypeEnum.COMPOSITE) { - throw new InvalidRequestException("Composite search parameters not currently supported with _filter clauses"); - } else if (typeEnum == RestSearchParameterTypeEnum.TOKEN) { - TokenParam param = new TokenParam(); - param.setValueAsQueryToken(null, - null, - null, - theFilter.getValue()); - return myPredicateBuilder.addPredicateToken(theResourceName, searchParam, Collections.singletonList(param), theFilter.getOperation(), theRequestPartitionId); - } } + + RestSearchParameterTypeEnum typeEnum = searchParam.getParamType(); + if (typeEnum == RestSearchParameterTypeEnum.URI) { + return myPredicateBuilder.addPredicateUri(theResourceName, searchParam, Collections.singletonList(new UriParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.STRING) { + return myPredicateBuilder.addPredicateString(theResourceName, searchParam, Collections.singletonList(new StringParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.DATE) { + return myPredicateBuilder.addPredicateDate(theResourceName, searchParam, Collections.singletonList(new DateParam(fromOperation(theFilter.getOperation()), theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.NUMBER) { + return myPredicateBuilder.addPredicateNumber(theResourceName, searchParam, Collections.singletonList(new NumberParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.REFERENCE) { + String paramName = theFilter.getParamPath().getName(); + SearchFilterParser.CompareOperation operation = theFilter.getOperation(); + String resourceType = null; // The value can either have (Patient/123) or not have (123) a resource type, either way it's not needed here + String chain = (theFilter.getParamPath().getNext() != null) ? theFilter.getParamPath().getNext().toString() : null; + String value = theFilter.getValue(); + ReferenceParam referenceParam = new ReferenceParam(resourceType, chain, value); + return addPredicate(theResourceName, paramName, Collections.singletonList(referenceParam), operation, theRequest, theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.QUANTITY) { + return myPredicateBuilder.addPredicateQuantity(theResourceName, searchParam, Collections.singletonList(new QuantityParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + } else if (typeEnum == RestSearchParameterTypeEnum.COMPOSITE) { + throw new InvalidRequestException("Composite search parameters not currently supported with _filter clauses"); + } else if (typeEnum == RestSearchParameterTypeEnum.TOKEN) { + TokenParam param = new TokenParam(); + param.setValueAsQueryToken(null, + null, + null, + theFilter.getValue()); + return myPredicateBuilder.addPredicateToken(theResourceName, searchParam, Collections.singletonList(param), theFilter.getOperation(), theRequestPartitionId); + } + return null; } 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 13d853aa356..a84d44bf5d4 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 @@ -89,6 +89,9 @@ import com.healthmarketscience.sqlbuilder.OrderObject; import com.healthmarketscience.sqlbuilder.SelectQuery; import com.healthmarketscience.sqlbuilder.Subquery; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn; +import org.apache.commons.collections4.BidiMap; +import org.apache.commons.collections4.bidimap.DualHashBidiMap; +import org.apache.commons.collections4.bidimap.UnmodifiableBidiMap; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.tuple.Pair; @@ -112,12 +115,29 @@ import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; public class QueryStack { private static final Logger ourLog = LoggerFactory.getLogger(QueryStack.class); + private static final BidiMap ourCompareOperationToParamPrefix; + + static { + DualHashBidiMap compareOperationToParamPrefix = new DualHashBidiMap<>(); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.ap, ParamPrefixEnum.APPROXIMATE); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.eq, ParamPrefixEnum.EQUAL); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.gt, ParamPrefixEnum.GREATERTHAN); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.ge, ParamPrefixEnum.GREATERTHAN_OR_EQUALS); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.lt, ParamPrefixEnum.LESSTHAN); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.le, ParamPrefixEnum.LESSTHAN_OR_EQUALS); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.ne, ParamPrefixEnum.NOT_EQUAL); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.eb, ParamPrefixEnum.ENDS_BEFORE); + compareOperationToParamPrefix.put(SearchFilterParser.CompareOperation.sa, ParamPrefixEnum.STARTS_AFTER); + ourCompareOperationToParamPrefix = UnmodifiableBidiMap.unmodifiableBidiMap(compareOperationToParamPrefix); + } + private final ModelConfig myModelConfig; private final FhirContext myFhirContext; private final SearchQueryBuilder mySqlBuilder; @@ -175,7 +195,6 @@ public class QueryStack { mySqlBuilder.addSortDate(resourceTablePredicateBuilder.getColumnLastUpdated(), theAscending); } - public void addSortOnNumber(String theResourceName, String theParamName, boolean theAscending) { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); NumberPredicateBuilder sortPredicateBuilder = mySqlBuilder.addNumberPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); @@ -217,7 +236,6 @@ public class QueryStack { mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnTargetResourceId(), theAscending); } - public void addSortOnString(String theResourceName, String theParamName, boolean theAscending) { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); StringPredicateBuilder sortPredicateBuilder = mySqlBuilder.addStringPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); @@ -246,7 +264,6 @@ public class QueryStack { mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValue(), theAscending); } - @SuppressWarnings("unchecked") private PredicateBuilderCacheLookupResult createOrReusePredicateBuilder(PredicateBuilderTypeEnum theType, DbColumn theSourceJoinColumn, String theParamName, Supplier theFactoryMethod) { boolean cacheHit = false; @@ -299,7 +316,6 @@ public class QueryStack { return orCondidtion; } - private Condition createPredicateCompositePart(@Nullable DbColumn theSourceJoinColumn, String theResourceName, RuntimeSearchParam theParam, IQueryParameterType theParamValue, RequestPartitionId theRequestPartitionId) { switch (theParam.getParamType()) { @@ -310,7 +326,7 @@ public class QueryStack { return createPredicateToken(theSourceJoinColumn, theResourceName, theParam, Collections.singletonList(theParamValue), null, theRequestPartitionId); } case DATE: { - return createPredicateDate(theSourceJoinColumn, theResourceName, theParam, Collections.singletonList(theParamValue), null, theRequestPartitionId); + return createPredicateDate(theSourceJoinColumn, theResourceName, theParam, Collections.singletonList(theParamValue), toOperation(((DateParam) theParamValue).getPrefix()), theRequestPartitionId); } case QUANTITY: { return createPredicateQuantity(theSourceJoinColumn, theResourceName, theParam, Collections.singletonList(theParamValue), null, theRequestPartitionId); @@ -320,7 +336,6 @@ public class QueryStack { throw new InvalidRequestException("Don't know how to handle composite parameter with type of " + theParam.getParamType()); } - public Condition createPredicateCoords(@Nullable DbColumn theSourceJoinColumn, String theResourceName, RuntimeSearchParam theSearchParam, @@ -363,7 +378,7 @@ public class QueryStack { List codePredicates = new ArrayList<>(); for (IQueryParameterType nextOr : theList) { - Condition p = predicateBuilder.createPredicateDateWithoutIdentityPredicate(nextOr, theResourceName, paramName, predicateBuilder, theOperation, theRequestPartitionId); + Condition p = predicateBuilder.createPredicateDateWithoutIdentityPredicate(nextOr, predicateBuilder, theOperation); codePredicates.add(p); } @@ -429,7 +444,7 @@ public class QueryStack { } else if (typeEnum == RestSearchParameterTypeEnum.STRING) { return theQueryStack3.createPredicateString(null, theResourceName, searchParam, Collections.singletonList(new StringParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); } else if (typeEnum == RestSearchParameterTypeEnum.DATE) { - return theQueryStack3.createPredicateDate(null, theResourceName, searchParam, Collections.singletonList(new DateParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); + return theQueryStack3.createPredicateDate(null, theResourceName, searchParam, Collections.singletonList(new DateParam(fromOperation(theFilter.getOperation()), theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); } else if (typeEnum == RestSearchParameterTypeEnum.NUMBER) { return theQueryStack3.createPredicateNumber(null, theResourceName, searchParam, Collections.singletonList(new NumberParam(theFilter.getValue())), theFilter.getOperation(), theRequestPartitionId); } else if (typeEnum == RestSearchParameterTypeEnum.REFERENCE) { @@ -976,7 +991,15 @@ public class QueryStack { switch (nextParamDef.getParamType()) { case DATE: for (List nextAnd : theAndOrParams) { - andPredicates.add(createPredicateDate(theSourceJoinColumn, theResourceName, nextParamDef, nextAnd, null, theRequestPartitionId)); + // FT: 2021-01-18 use operation 'gt', 'ge', 'le' or 'lt' + // to create the predicateDate instead of generic one with operation = null + SearchFilterParser.CompareOperation operation = null; + if (nextAnd.size() > 0) { + DateParam param = (DateParam) nextAnd.get(0); + operation = toOperation(param.getPrefix()); + } + andPredicates.add(createPredicateDate(theSourceJoinColumn, theResourceName, nextParamDef, nextAnd, operation, theRequestPartitionId)); + //andPredicates.add(createPredicateDate(theSourceJoinColumn, theResourceName, nextParamDef, nextAnd, null, theRequestPartitionId)); } break; case QUANTITY: @@ -1204,29 +1227,19 @@ public class QueryStack { } public static SearchFilterParser.CompareOperation toOperation(ParamPrefixEnum thePrefix) { - if (thePrefix != null) { - switch (thePrefix) { - case APPROXIMATE: - return SearchFilterParser.CompareOperation.ap; - case EQUAL: - return SearchFilterParser.CompareOperation.eq; - case GREATERTHAN: - return SearchFilterParser.CompareOperation.gt; - case GREATERTHAN_OR_EQUALS: - return SearchFilterParser.CompareOperation.ge; - case LESSTHAN: - return SearchFilterParser.CompareOperation.lt; - case LESSTHAN_OR_EQUALS: - return SearchFilterParser.CompareOperation.le; - case NOT_EQUAL: - return SearchFilterParser.CompareOperation.ne; - case ENDS_BEFORE: - return SearchFilterParser.CompareOperation.eb; - case STARTS_AFTER: - return SearchFilterParser.CompareOperation.sa; - } + SearchFilterParser.CompareOperation retVal = null; + if (thePrefix != null && ourCompareOperationToParamPrefix.containsValue(thePrefix)) { + retVal = ourCompareOperationToParamPrefix.getKey(thePrefix); } - return SearchFilterParser.CompareOperation.eq; + return defaultIfNull(retVal, SearchFilterParser.CompareOperation.eq); + } + + public static ParamPrefixEnum fromOperation(SearchFilterParser.CompareOperation thePrefix) { + ParamPrefixEnum retVal = null; + if (thePrefix != null && ourCompareOperationToParamPrefix.containsKey(thePrefix)) { + retVal = ourCompareOperationToParamPrefix.get(thePrefix); + } + return defaultIfNull(retVal, ParamPrefixEnum.EQUAL); } private static String getChainedPart(String parameter) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/DatePredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/DatePredicateBuilder.java index af57521f211..36eee2123fd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/DatePredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/DatePredicateBuilder.java @@ -20,7 +20,6 @@ package ca.uhn.fhir.jpa.search.builder.predicate; * #L% */ -import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.dao.predicate.SearchFilterParser; import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; @@ -64,16 +63,16 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder { public Condition createPredicateDateWithoutIdentityPredicate(IQueryParameterType theParam, - String theResourceName, - String theParamName, DatePredicateBuilder theFrom, - SearchFilterParser.CompareOperation theOperation, - RequestPartitionId theRequestPartitionId) { + SearchFilterParser.CompareOperation theOperation) { Condition p; if (theParam instanceof DateParam) { DateParam date = (DateParam) theParam; if (!date.isEmpty()) { + if (theOperation == SearchFilterParser.CompareOperation.ne) { + date = new DateParam(ParamPrefixEnum.EQUAL, date.getValueAsString()); + } DateRangeParam range = new DateRangeParam(date); p = createPredicateDateFromRange(theFrom, range, theOperation); } else { @@ -96,6 +95,8 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder { private Condition createPredicateDateFromRange(DatePredicateBuilder theFrom, DateRangeParam theRange, SearchFilterParser.CompareOperation theOperation) { + + Date lowerBoundInstant = theRange.getLowerBoundAsInstant(); Date upperBoundInstant = theRange.getUpperBoundAsInstant(); @@ -103,16 +104,17 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder { DateParam upperBound = theRange.getUpperBound(); Integer lowerBoundAsOrdinal = theRange.getLowerBoundAsDateInteger(); Integer upperBoundAsOrdinal = theRange.getUpperBoundAsDateInteger(); - Comparable genericLowerBound; - Comparable genericUpperBound; - /** + Comparable genericLowerBound; + Comparable genericUpperBound; + + /* * If all present search parameters are of DAY precision, and {@link ca.uhn.fhir.jpa.model.entity.ModelConfig#getUseOrdinalDatesForDayPrecisionSearches()} is true, * then we attempt to use the ordinal field for date comparisons instead of the date field. */ boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound) && myDaoConfig.getModelConfig().getUseOrdinalDatesForDayPrecisionSearches(); Condition lt; - Condition gt = null; + Condition gt; Condition lb = null; Condition ub = null; DatePredicateBuilder.ColumnEnum lowValueField; @@ -130,28 +132,24 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder { genericUpperBound = upperBoundInstant; } - if (theOperation == SearchFilterParser.CompareOperation.lt) { - if (lowerBoundInstant == null) { - throw new InvalidRequestException("lowerBound value not correctly specified for compare theOperation"); + if (theOperation == SearchFilterParser.CompareOperation.lt || theOperation == SearchFilterParser.CompareOperation.le) { + // use lower bound first + if (lowerBoundInstant != null) { + lb = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericLowerBound); + } else if (upperBoundInstant != null) { + ub = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound); + } else { + throw new InvalidRequestException("lowerBound and upperBound value not correctly specified for comparing " + theOperation); } - //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - lb = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN, genericLowerBound); - } else if (theOperation == SearchFilterParser.CompareOperation.le) { - if (upperBoundInstant == null) { - throw new InvalidRequestException("upperBound value not correctly specified for compare theOperation"); + } else if (theOperation == SearchFilterParser.CompareOperation.gt || theOperation == SearchFilterParser.CompareOperation.ge) { + // use upper bound first, e.g value between 6 and 10 + if (upperBoundInstant != null) { + ub = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericUpperBound); + } else if (lowerBoundInstant != null) { + lb = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound); + } else { + throw new InvalidRequestException("upperBound and lowerBound value not correctly specified for compare theOperation"); } - //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - lb = theFrom.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound); - } else if (theOperation == SearchFilterParser.CompareOperation.gt) { - if (upperBoundInstant == null) { - throw new InvalidRequestException("upperBound value not correctly specified for compare theOperation"); - } - lb = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN, genericUpperBound); - } else if (theOperation == SearchFilterParser.CompareOperation.ge) { - if (lowerBoundInstant == null) { - throw new InvalidRequestException("lowerBound value not correctly specified for compare theOperation"); - } - lb = theFrom.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound); } else if (theOperation == SearchFilterParser.CompareOperation.ne) { if ((lowerBoundInstant == null) || (upperBoundInstant == null)) { @@ -160,7 +158,10 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder { lt = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN, genericLowerBound); gt = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN, genericUpperBound); lb = ComboCondition.or(lt, gt); - } else if ((theOperation == SearchFilterParser.CompareOperation.eq) || (theOperation == null)) { + } else if ((theOperation == SearchFilterParser.CompareOperation.eq) + || (theOperation == SearchFilterParser.CompareOperation.sa) + || (theOperation == SearchFilterParser.CompareOperation.eb) + || (theOperation == null)) { if (lowerBoundInstant != null) { gt = theFrom.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound); lt = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterLegacySearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterLegacySearchBuilderTest.java new file mode 100644 index 00000000000..fd6bb9fdab9 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterLegacySearchBuilderTest.java @@ -0,0 +1,1264 @@ +package ca.uhn.fhir.jpa.dao.r4; + +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.IResourceProvenanceDao; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import org.hamcrest.Matchers; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.CarePlan; +import org.hl7.fhir.r4.model.DateType; +import org.hl7.fhir.r4.model.DecimalType; +import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.RiskAssessment; +import org.hl7.fhir.r4.model.ValueSet; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +@SuppressWarnings({"Duplicates"}) +public class FhirResourceDaoR4FilterLegacySearchBuilderTest extends BaseJpaR4Test { + + private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4FilterLegacySearchBuilderTest.class); + @Autowired + private IResourceProvenanceDao myResourceProvenanceDao; + + @AfterEach + public void after() { + myDaoConfig.setFilterParameterEnabled(new DaoConfig().isFilterParameterEnabled()); + myDaoConfig.setUseLegacySearchBuilder(false); + } + + @BeforeEach + public void before() { + myDaoConfig.setFilterParameterEnabled(true); + myDaoConfig.setUseLegacySearchBuilder(true); + } + + @Test + public void testMalformedFilter() { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name eq smith))")); + try { + myPatientDao.search(map); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Error parsing _filter syntax: Expression did not terminate at 13", e.getMessage()); + } + } + + @Test + public void testBrackets() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name eq smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("(name eq smith) or (name eq jones)")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1, id2)); + + } + + @Test + public void testStringComparatorEq() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name eq smi")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name eq smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + } + + @Test + public void testReferenceComparatorEq() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + IIdType ptId = myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + p = new Patient(); + p.addName().setFamily("Smith").addGiven("John2"); + p.setActive(true); + IIdType ptId2 = myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + p = new Patient(); + p.addName().setFamily("Smith").addGiven("John3"); + p.setActive(true); + IIdType ptId3 = myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + CarePlan cp = new CarePlan(); + cp.getSubject().setReference(ptId.getValue()); + String cpId = myCarePlanDao.create(cp).getId().toUnqualifiedVersionless().getValue(); + + cp = new CarePlan(); + cp.addActivity().getDetail().addPerformer().setReference(ptId2.getValue()); + String cpId2 = myCarePlanDao.create(cp).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("subject eq " + ptId.getValue())); + found = toUnqualifiedVersionlessIdValues(myCarePlanDao.search(map)); + assertThat(found, containsInAnyOrder(cpId)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("subject eq " + ptId.getIdPart())); + found = toUnqualifiedVersionlessIdValues(myCarePlanDao.search(map)); + assertThat(found, containsInAnyOrder(cpId)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("(subject eq " + ptId.getIdPart() + ") or (performer eq " + ptId2.getValue() + ")")); + found = toUnqualifiedVersionlessIdValues(myCarePlanDao.search(map)); + assertThat(found, containsInAnyOrder(cpId, cpId2)); + + } + + @Test + public void testSourceComparatorEq() { + myDaoConfig.setStoreMetaSourceInformation(DaoConfig.StoreMetaSourceInformationEnum.SOURCE_URI_AND_REQUEST_ID); + + Patient p = new Patient(); + p.getMeta().setSource("http://source"); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String ptId = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Smith").addGiven("John2"); + p.setActive(true); + myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + SearchParameterMap map; + List found; + + runInTransaction(() -> { + ourLog.info("Provenance:\n * {}", myResourceProvenanceDao.findAll().stream().map(t -> t.toString()).collect(Collectors.joining("\n * "))); + }); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("_source eq http://source")); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, containsInAnyOrder(ptId)); + + } + + @Test + public void testFilterDisabled() { + myDaoConfig.setFilterParameterEnabled(false); + + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name eq smith")); + try { + myPatientDao.search(map); + } catch (InvalidRequestException e) { + assertEquals("_filter parameter is disabled on this server", e.getMessage()); + } + } + + /** + * Use the Encounter DAO to find things that would match a Patient + */ + @Test + public void testRetrieveDifferentTypeEq() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + String idVal = id1.split("/")[1]; + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam(String.format("status eq active or _id eq %s", idVal))); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myEncounterDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam(String.format("status eq inactive or _id eq %s", idVal))); + found = toUnqualifiedVersionlessIdValues(myEncounterDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam(String.format("status eq inactive or _id eq Patient/FOO"))); + found = toUnqualifiedVersionlessIdValues(myEncounterDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam(String.format("_id eq %s", idVal))); + found = toUnqualifiedVersionlessIdValues(myEncounterDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testStringComparatorNe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ne smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + assertThat(found, containsInAnyOrder(Matchers.not(id1))); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ne jones")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + assertThat(found, containsInAnyOrder(Matchers.not(id2))); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given ne john")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + assertThat(found, containsInAnyOrder(Matchers.not(id1))); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given ne frank")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + assertThat(found, containsInAnyOrder(Matchers.not(id2))); + + } + + @Test + public void testReferenceComparatorNe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + IIdType ptId = myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + p = new Patient(); + p.addName().setFamily("Smith").addGiven("John2"); + p.setActive(true); + IIdType ptId2 = myPatientDao.create(p).getId().toUnqualifiedVersionless(); + + CarePlan cp = new CarePlan(); + cp.getSubject().setReference(ptId.getValue()); + myCarePlanDao.create(cp).getId().toUnqualifiedVersionless().getValue(); + + cp = new CarePlan(); + cp.getSubject().setReference(ptId2.getValue()); + cp.addActivity().getDetail().addPerformer().setReference(ptId2.getValue()); + String cpId2 = myCarePlanDao.create(cp).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("subject ne " + ptId.getValue())); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myCarePlanDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, containsInAnyOrder(cpId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("(subject ne " + ptId.getIdPart() + ") and (performer ne " + ptId2.getValue() + ")")); + found = toUnqualifiedVersionlessIdValues(myCarePlanDao.search(map)); + assertThat(found, empty()); + + } + + + @Test + public void testLanguageComparatorEq() { + + Patient p = new Patient(); + p.setLanguage("en"); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("_language eq en")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + } + + + + @Test + public void testStringComparatorCo() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name co smi")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name co smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given co frank")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family co jones")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + } + + @Test + public void testStringComparatorSw() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name sw smi")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name sw mi")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given sw fr")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + } + + @Test + public void testStringComparatorEw() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ew ith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("name ew it")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given ew nk")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + } + + @Test + public void testStringComparatorGt() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family gt jones")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family gt arthur")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1, id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given gt john")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testStringComparatorLt() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family lt smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family lt walker")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1, id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given lt frank")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testStringComparatorGe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ge jones")); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, containsInAnyOrder(id1, id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ge justin")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family ge arthur")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1, id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("given ge jon")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testStringComparatorLe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + p = new Patient(); + p.addName().setFamily("Jones").addGiven("Frank"); + p.setActive(false); + String id2 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family le smith")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1, id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family le jones")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("family le jackson")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testDateComparatorEq() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate eq 1955-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + } + + @Test + public void testDateComparatorNe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate ne 1955-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate ne 1995-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + } + + @Test + public void testDateComparatorGt() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate gt 1954-12-31")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate gt 1955-01-01")); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, empty()); + + } + + @Test + public void testDateComparatorLt() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate lt 1955-01-02")); + myCaptureQueriesListener.clear(); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate lt 1955-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testDateComparatorGe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate ge 1955-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate ge 1954-12-31")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate ge 1955-01-02")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testDateComparatorLe() { + + Patient p = new Patient(); + p.addName().setFamily("Smith").addGiven("John"); + p.setBirthDateElement(new DateType("1955-01-01")); + p.setActive(true); + String id1 = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate le 1955-01-01")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate le 1954-12-31")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, empty()); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("birthdate le 1955-01-02")); + found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(found, containsInAnyOrder(id1)); + + } + + @Test + public void testNumericComparatorEq() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.1")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testNumericComparatorNe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.1")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + } + + @Test + public void testNumericComparatorGt() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability gt 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability gt 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testNumericComparatorLt() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability lt 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability lt 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, empty()); + + } + + @Test + public void testNumericComparatorGe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ge 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ge 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + } + + @Test + public void testNumericComparatorLe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability le 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability le 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + } + + @Test + public void testSearchUriEq() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url eq http://hl7.org/foo/baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url eq http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url eq http://hl7.org/foo/bar/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriNe() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ne http://hl7.org/foo/baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ne http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ne http://hl7.org/foo/baz and url ne http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriCo() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url co http://hl7.org/foo"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url co baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url co http://hl7.org/foo/bat"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriGt() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url gt http://hl7.org/foo"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url gt http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url gt http://hl7.org/foo/baza"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriLt() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url lt http://hl7.org/foo"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url lt http://hl7.org/foo/baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url lt http://hl7.org/foo/bara"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId2)); + + } + + @Test + public void testSearchUriGe() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ge http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ge http://hl7.org/foo/baza"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriLe() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url le http://hl7.org/foo/baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url le http://hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url lt http://hl7.org/foo/baza"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + } + + @Test + public void testSearchUriSw() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url sw http://hl7.org"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1, vsId2)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url sw hl7.org/foo/bar"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testSearchUriEw() { + + ValueSet vs1 = new ValueSet(); + vs1.setUrl("http://hl7.org/foo/baz"); + IIdType vsId1 = myValueSetDao.create(vs1, mySrd).getId().toUnqualifiedVersionless(); + + ValueSet vs2 = new ValueSet(); + vs2.setUrl("http://hl7.org/foo/bar"); + IIdType vsId2 = myValueSetDao.create(vs2, mySrd).getId().toUnqualifiedVersionless(); + + IBundleProvider result; + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ew baz"))); + assertThat(toUnqualifiedVersionlessIds(result), containsInAnyOrder(vsId1)); + + result = myValueSetDao.search(SearchParameterMap.newSynchronous().add(Constants.PARAM_FILTER, + new StringParam("url ew ba"))); + assertThat(toUnqualifiedVersionlessIds(result), empty()); + + } + + @Test + public void testUnknownSearchParam() { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("foo eq smith")); + try { + myPatientDao.search(map); + } catch (InvalidRequestException e) { + assertEquals("Invalid search parameter specified, foo, for resource type Patient", e.getMessage()); + } + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java index 1a707e0b16a..eacf7d652e9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java @@ -703,7 +703,9 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test { map = new SearchParameterMap(); map.setLoadSynchronous(true); map.add(Constants.PARAM_FILTER, new StringParam("birthdate gt 1955-01-01")); + myCaptureQueriesListener.clear(); found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); assertThat(found, empty()); } @@ -723,7 +725,9 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test { map = new SearchParameterMap(); map.setLoadSynchronous(true); map.add(Constants.PARAM_FILTER, new StringParam("birthdate lt 1955-01-02")); + myCaptureQueriesListener.clear(); found = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(0); assertThat(found, containsInAnyOrder(id1)); map = new SearchParameterMap(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java index 52d2b219fc7..bcd03a8640d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java @@ -4172,7 +4172,7 @@ public class FhirResourceDaoR4LegacySearchBuilderTest extends BaseJpaR4Test { String searchQuery = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search query:\n{}", searchQuery); assertEquals(1, countMatches(searchQuery.toLowerCase(), "join"), searchQuery); - assertEquals(1, countMatches(searchQuery.toLowerCase(), "sp_value_low_date_ordinal>='20200605'"), searchQuery); + assertEquals(1, countMatches(searchQuery.toLowerCase(), "sp_value_high_date_ordinal>='20200605'"), searchQuery); assertEquals(1, countMatches(searchQuery.toLowerCase(), "sp_value_low_date_ordinal<='20200606'"), searchQuery); } @@ -4192,7 +4192,7 @@ public class FhirResourceDaoR4LegacySearchBuilderTest extends BaseJpaR4Test { assertEquals(0, countMatches(searchQuery.toLowerCase(), "partition"), searchQuery); assertEquals(2, countMatches(searchQuery.toLowerCase(), "join"), searchQuery); assertEquals(2, countMatches(searchQuery.toLowerCase(), "hash_identity"), searchQuery); - assertEquals(4, countMatches(searchQuery.toLowerCase(), "sp_value_low"), searchQuery); + assertEquals(2, countMatches(searchQuery.toLowerCase(), "sp_value_low"), searchQuery); } // Period search diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 411ae3d4b37..0078f7998b9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -4369,7 +4369,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { String searchQuery = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search query:\n{}", searchQuery); assertEquals(1, countMatches(searchQuery.toLowerCase(), "join"), searchQuery); - assertEquals(1, countMatches(searchQuery.toLowerCase(), "t0.sp_value_low_date_ordinal >= '20200605'"), searchQuery); + assertEquals(1, countMatches(searchQuery.toLowerCase(), "t0.sp_value_high_date_ordinal >= '20200605'"), searchQuery); assertEquals(1, countMatches(searchQuery.toLowerCase(), "t0.sp_value_low_date_ordinal <= '20200606'"), searchQuery); } @@ -4389,7 +4389,8 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { assertEquals(0, countMatches(searchQuery.toLowerCase(), "partition"), searchQuery); assertEquals(2, countMatches(searchQuery.toLowerCase(), "join"), searchQuery); assertEquals(2, countMatches(searchQuery.toLowerCase(), "hash_identity"), searchQuery); - assertEquals(4, countMatches(searchQuery.toLowerCase(), "sp_value_low"), searchQuery); + // - query is changed 'or' is removed + assertEquals(2, countMatches(searchQuery.toLowerCase(), "sp_value_low"), searchQuery); } // Period search 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 371b65346d2..50390a69aa3 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 @@ -1702,7 +1702,9 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); assertEquals(0, StringUtils.countMatches(searchSql, "PARTITION_ID")); - assertEquals(2, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + // NOTE: the query is changed, only one SP_VALUE_LOW and SP_VALUE_HIGH + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_HIGH")); } @@ -1783,7 +1785,9 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID")); - assertEquals(2, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + // NOTE: the query is changed, only one SP_VALUE_LOW and SP_VALUE_HIGH + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_HIGH")); } @@ -1861,7 +1865,9 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID")); - assertEquals(2, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + // NOTE: the query is changed, only one SP_VALUE_LOW and SP_VALUE_HIGH + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); + assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_HIGH")); } From c55410e6b6f2c9e1ae30bdfef47dae036aed046b Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 23 Jan 2021 17:54:51 -0500 Subject: [PATCH 05/12] Add changelog for #2302 --- .../fhir/changelog/5_3_0/2302-streamline-date-query-sql.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2302-streamline-date-query-sql.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2302-streamline-date-query-sql.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2302-streamline-date-query-sql.yaml new file mode 100644 index 00000000000..198d8ed2234 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2302-streamline-date-query-sql.yaml @@ -0,0 +1,5 @@ +--- +type: perf +issue: 2302 +title: The JPA server generated SQL for date search parameters has been streamlined to avoid the use of + redundant OR expressions that slow down performance in some cases. From 42d12403f96d429e2ef0785e6c265c53dc5c56c2 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 23 Jan 2021 18:50:27 -0500 Subject: [PATCH 06/12] Try to restore codecov --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 04c4ea0b33d..b026fee6b54 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -52,7 +52,7 @@ jobs: inputs: targetPath: '$(Build.ArtifactStagingDirectory)/' artifactName: 'full_logs.zip' - - script: bash <(curl https://codecov.io/bash) -C $(Build.SourceVersion) + - script: bash <(curl https://codecov.io/bash) -t $(CODECOV_TOKEN) displayName: 'codecov' - task: PublishTestResults@2 inputs: From 66e6cab21f6830d00e2a5a4891f2dfd6e646fea2 Mon Sep 17 00:00:00 2001 From: Frank Tao <38163583+frankjtao@users.noreply.github.com> Date: Sun, 24 Jan 2021 09:56:45 -0500 Subject: [PATCH 07/12] Fixed xml.bind conflict between jakakax and javax (#2320) --- hapi-fhir-jpaserver-model/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hapi-fhir-jpaserver-model/pom.xml b/hapi-fhir-jpaserver-model/pom.xml index 20f587e4ab0..c3be04b6b5e 100644 --- a/hapi-fhir-jpaserver-model/pom.xml +++ b/hapi-fhir-jpaserver-model/pom.xml @@ -79,6 +79,10 @@ javax.activation javax.activation-api + + javax.xml.bind + jaxb-api + From 9e5d0790fb18e2fcbd044f598a5dd526ef8378ee Mon Sep 17 00:00:00 2001 From: Mark Iantorno Date: Sun, 24 Jan 2021 11:49:53 -0500 Subject: [PATCH 08/12] Just cleaning up the README a little --- README.md | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 351737d7722..8ae46ea24ad 100644 --- a/README.md +++ b/README.md @@ -3,10 +3,17 @@ HAPI FHIR HAPI FHIR - Java API for HL7 FHIR Clients and Servers -[![Build Status](https://dev.azure.com/hapifhir/HAPI%20FHIR/_apis/build/status/jamesagnew.hapi-fhir?branchName=master)](https://dev.azure.com/hapifhir/HAPI%20FHIR/_build/latest?definitionId=1&branchName=master) -[![codecov](https://codecov.io/gh/jamesagnew/hapi-fhir/branch/master/graph/badge.svg)](https://codecov.io/gh/jamesagnew/hapi-fhir) -[![Maven Central](https://maven-badges.herokuapp.com/maven-central/ca.uhn.hapi.fhir/hapi-fhir-base/badge.svg)](http://search.maven.org/#search|ga|1|ca.uhn.hapi.fhir) -[![License](https://img.shields.io/badge/license-apache%202.0-60C060.svg)](https://hapifhir.io/hapi-fhir/license.html) +[![License][Badge-License]][Link-License] + +## CI/CD +| CI Status (master) | SNAPSHOT Pipeline | Current Release | +| :---: | :---: | :---: | +| [![Build Status][Badge-AzurePipelineMaster]][Link-AzurePipelinesMaster] | [![Build Status][Badge-AzureReleaseSnapshot]][Link-AzurePipelinesSnapshot] | [![Release Artifacts][Badge-MavenCentral]][Link-MavenCentral] | + +## Test Coverage +[![codecov][Badge-CodeCov]][Link-CodeCov] + +## Documentation and wiki Complete project documentation is available here: http://hapifhir.io @@ -16,4 +23,23 @@ http://hapi.fhir.org/ This project is Open Source, licensed under the Apache Software License 2.0. -Please see [this wiki page](https://github.com/jamesagnew/hapi-fhir/wiki/Getting-Help) for information on where to get help with HAPI FHIR. Please see [Smile CDR](https://smilecdr.com) for information on commercial support. +Please see [this wiki page][Link-wiki] for information on where to get help with HAPI FHIR. + +Please see [Smile CDR][Link-SmileCDR] for information on commercial support. + +[Link-AzurePipelines]: https://dev.azure.com/hapifhir/HAPI%20FHIR/_build +[Link-AzurePipelinesMaster]: https://dev.azure.com/hapifhir/HAPI%20FHIR/_build?definitionId=2 +[Link-AzurePipelinesSnapshot]: https://dev.azure.com/hapifhir/HAPI%20FHIR/_build?definitionId=3 +[Link-MavenCentral]: http://search.maven.org/#search|ga|1|ca.uhn.hapi.fhir +[Link-CodeCov]: https://codecov.io/gh/hapifhir/hapi-fhir +[Link-wiki]: https://github.com/jamesagnew/hapi-fhir/wiki/Getting-Help +[Link-SmileCDR]: https://smilecdr.com +[Link-License]: https://hapifhir.io/hapi-fhir/license.html + +[Badge-AzurePipelineMaster]: https://dev.azure.com/hapifhir/HAPI%20FHIR/_apis/build/status/hapifhir.hapi-fhir?branchName=refs%2Fpull%2F2319%2Fmerge +[Badge-AzureReleaseSnapshot]: https://dev.azure.com/hapifhir/HAPI%20FHIR/_apis/build/status/SNAPSHOT%20pipeline?branchName=master +[Badge-MavenCentral]: https://maven-badges.herokuapp.com/maven-central/ca.uhn.hapi.fhir/hapi-fhir-base/badge.svg +[Badge-CodeCov]: https://codecov.io/gh/hapifhir/hapi-fhir/branch/master/graph/badge.svg?token=zHfnKfQB9X +[Badge-License]: https://img.shields.io/badge/license-apache%202.0-60C060.svg + + From 5d2e830b156f5ebf3d48436cee39e7a1cf42fe66 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 24 Jan 2021 12:09:50 -0500 Subject: [PATCH 09/12] Fix android tests (#2321) --- hapi-fhir-android/pom.xml | 5 +++++ .../ca/uhn/fhir/android/client/GenericClientDstu3IT.java | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-android/pom.xml b/hapi-fhir-android/pom.xml index f39f6d9ae41..73db7d5cb06 100644 --- a/hapi-fhir-android/pom.xml +++ b/hapi-fhir-android/pom.xml @@ -103,12 +103,17 @@ org.apache.maven.plugins maven-failsafe-plugin + + org.slf4j:slf4j-android + + true it integration-test + verify diff --git a/hapi-fhir-android/src/test/java/ca/uhn/fhir/android/client/GenericClientDstu3IT.java b/hapi-fhir-android/src/test/java/ca/uhn/fhir/android/client/GenericClientDstu3IT.java index 1c5cb01529f..cf1242ca8d5 100644 --- a/hapi-fhir-android/src/test/java/ca/uhn/fhir/android/client/GenericClientDstu3IT.java +++ b/hapi-fhir-android/src/test/java/ca/uhn/fhir/android/client/GenericClientDstu3IT.java @@ -143,7 +143,7 @@ public class GenericClientDstu3IT { .returnBundle(Bundle.class) .execute(); - assertEquals("http://example.com/fhir/Patient?_format=json", capt.getAllValues().get(idx).url().toString()); + assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(idx).url().toString()); idx++; } @@ -178,7 +178,7 @@ public class GenericClientDstu3IT { Request request = capt.getAllValues().get(0); ourLog.info(request.headers().toString()); - assertEquals("http://example.com/fhir/Binary?_format=json", request.url().toString()); + assertEquals("http://example.com/fhir/Binary", request.url().toString()); validateUserAgent(capt); assertEquals(Constants.CT_FHIR_JSON_NEW + ";charset=utf-8", request.body().contentType().toString().toLowerCase().replace(" ", "")); @@ -252,7 +252,7 @@ public class GenericClientDstu3IT { assertNotNull(outcome.getResource()); assertEquals("
FINAL VALUE
", ((Patient) outcome.getResource()).getText().getDivAsString()); - assertEquals("http://example.com/fhir/Patient?_format=json", capt.getAllValues().get(0).url().toString()); + assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(0).url().toString()); } From 86ea0d60ef14616e8baa9482ed46a09b25c5cc83 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 25 Jan 2021 08:19:29 -0500 Subject: [PATCH 10/12] reduce _count to the maximum page size rather than throwing an error. (#2319) * reduce _count to the maximum page size rather than throwing an error. Also when setting the paging provider on a restful server, automatically set the default page size and max page sized of the restful server from the paging provider. * reword log message * fixing this paging issue has uncovered all sorts of interesting things, including this long hidden bug! * fix tests in sub-optimal way * fix NPE * fix test * fixed a couple of bugs in SearchCoordinatorSvcImpl that surfaced when i added a default page size to RestfulServer --- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 7 +++--- .../jpa/search/SearchCoordinatorSvcImpl.java | 4 ++-- .../jpa/dao/r4/ConsentEventsDaoR4Test.java | 2 +- ...sentInterceptorResourceProviderR4Test.java | 3 +++ .../r4/ResourceProviderInterceptorR4Test.java | 22 ------------------- .../uhn/fhir/rest/server/RestfulServer.java | 8 ++++++- 6 files changed, 17 insertions(+), 29 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 5b55793ed12..7cbdd76c2a7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1315,11 +1315,12 @@ public abstract class BaseHapiFhirResourceDao extends B theParams.setOffset(offset); } - final Integer count = RestfulServerUtils.extractCountParameter(theRequest); + Integer count = RestfulServerUtils.extractCountParameter(theRequest); if (count != null) { Integer maxPageSize = theRequest.getServer().getMaximumPageSize(); - if (maxPageSize != null) { - Validate.inclusiveBetween(1, theRequest.getServer().getMaximumPageSize(), count, "Count must be positive integer and less than " + maxPageSize); + if (maxPageSize != null && count > maxPageSize) { + ourLog.info("Reducing {} from {} to {} which is the maximum allowable page size.", Constants.PARAM_COUNT, count, maxPageSize); + count = maxPageSize; } theParams.setCount(count); } else if (theRequest.getServer().getDefaultPageSize() != null) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index 80328cf2f73..6786f9ff002 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -251,7 +251,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { ourLog.trace("Search entity marked as finished with {} results", search.getNumFound()); break; } - if (search.getNumFound() >= theTo) { + if ((search.getNumFound() - search.getNumBlocked()) >= theTo) { ourLog.trace("Search entity has {} results so far", search.getNumFound()); break; } @@ -1091,7 +1091,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { int minWanted = 0; if (myParams.getCount() != null) { minWanted = myParams.getCount(); - minWanted = Math.max(minWanted, myPagingProvider.getMaximumPageSize()); + minWanted = Math.min(minWanted, myPagingProvider.getMaximumPageSize()); minWanted += currentlyLoaded; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java index 0db292467f8..8cf3c06258e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java @@ -140,7 +140,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { } } assertEquals(myObservationIdsEvenOnly.subList(10, 25), returnedIdValues, "Wrong response from " + outcome.getClass()); - assertEquals(2, hitCount.get()); + assertEquals(3, hitCount.get()); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ConsentInterceptorResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ConsentInterceptorResourceProviderR4Test.java index 3fbd8e3e15a..2c92497b1a1 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ConsentInterceptorResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ConsentInterceptorResourceProviderR4Test.java @@ -65,6 +65,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.blankOrNullString; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -126,6 +127,7 @@ public class ConsentInterceptorResourceProviderR4Test extends BaseResourceProvid .execute(); List resources = BundleUtil.toListOfResources(myFhirCtx, result); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); + assertThat(returnedIdValues, hasSize(15)); assertEquals(myObservationIdsEvenOnly.subList(0, 15), returnedIdValues); // Fetch the next page @@ -135,6 +137,7 @@ public class ConsentInterceptorResourceProviderR4Test extends BaseResourceProvid .execute(); resources = BundleUtil.toListOfResources(myFhirCtx, result); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); + assertThat(returnedIdValues, hasSize(10)); assertEquals(myObservationIdsEvenOnly.subList(15, 25), returnedIdValues); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java index 907d9baf064..8d02586a564 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java @@ -13,8 +13,6 @@ import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; -import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.interceptor.ServerOperationInterceptorAdapter; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.collect.Lists; @@ -52,7 +50,6 @@ import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; @@ -428,23 +425,4 @@ public class ResourceProviderInterceptorR4Test extends BaseResourceProviderR4Tes } } - public static void verifyDaoInterceptor(IServerInterceptor theDaoInterceptor) { - ArgumentCaptor ardCaptor; - ArgumentCaptor opTypeCaptor; - ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class); - opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class); - verify(theDaoInterceptor, atLeast(1)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture()); -// boolean good = false; -// for (int i = 0; i < opTypeCaptor.getAllValues().size(); i++) { -// if (RestOperationTypeEnum.CREATE.equals(opTypeCaptor.getAllValues().get(i))) { -// if ("Patient".equals(ardCaptor.getValue().getResourceType())) { -// if (ardCaptor.getValue().getResource() != null) { -// good = true; -// } -// } -// } -// } -// assertTrue(good); - } - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index e1e44a4b1c4..ee9b1cb1da8 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -675,10 +675,16 @@ public class RestfulServer extends HttpServlet implements IRestfulServernull to use no paging (which is the default) + * Sets the paging provider to use, or null to use no paging (which is the default). + * This will set defaultPageSize and maximumPageSize from the paging provider. */ public void setPagingProvider(IPagingProvider thePagingProvider) { myPagingProvider = thePagingProvider; + if (myPagingProvider != null) { + setDefaultPageSize(myPagingProvider.getDefaultPageSize()); + setMaximumPageSize(myPagingProvider.getMaximumPageSize()); + } + } @Override From e696262c4a32b0ae8d2601bac0d7ecab5f805fa0 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 25 Jan 2021 13:08:15 -0500 Subject: [PATCH 11/12] Update snapshot-pipeline.yml for Azure Pipelines Add CI,ERRORPRONE profiles. --- snapshot-pipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snapshot-pipeline.yml b/snapshot-pipeline.yml index 98ace477f68..7f43be66e0a 100644 --- a/snapshot-pipeline.yml +++ b/snapshot-pipeline.yml @@ -85,5 +85,5 @@ steps: inputs: mavenPomFile: '$(System.DefaultWorkingDirectory)/pom.xml' goals: deploy - options: '--settings $(System.DefaultWorkingDirectory)/settings.xml -P DIST,ALLMODULES' + options: '--settings $(System.DefaultWorkingDirectory)/settings.xml -P DIST,ALLMODULES,CI,ERRORPRONE' publishJUnitResults: false \ No newline at end of file From e80e592d804865e472a7712e6218675101717fe6 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 25 Jan 2021 14:31:36 -0500 Subject: [PATCH 12/12] Skip tests --- snapshot-pipeline.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/snapshot-pipeline.yml b/snapshot-pipeline.yml index 7f43be66e0a..ad886c91ca9 100644 --- a/snapshot-pipeline.yml +++ b/snapshot-pipeline.yml @@ -85,5 +85,5 @@ steps: inputs: mavenPomFile: '$(System.DefaultWorkingDirectory)/pom.xml' goals: deploy - options: '--settings $(System.DefaultWorkingDirectory)/settings.xml -P DIST,ALLMODULES,CI,ERRORPRONE' - publishJUnitResults: false \ No newline at end of file + options: '--settings $(System.DefaultWorkingDirectory)/settings.xml -P DIST,ALLMODULES -Dmaven.test.skip' + publishJUnitResults: false