From 7010ddd715655cf0a80715ddbc703a4f7689cd9d Mon Sep 17 00:00:00 2001 From: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:30:20 -0400 Subject: [PATCH] Allow hsearch sorted offset searches (#3864) * Start direct HSearch path * Support no HSearch * Spike out the direct resource query * Implement hsearch fast load * Fix last master merge in issues * Implement revision requests * Test direct resources (no IDs query) sorting * Use mock to count freetext searches to avoid implementing interface in test * Remove fixme * Make listener optional as it is used only for tests * Provide new dependency * Widen fast path test scope and fix previously untested configurations * Make method transactional as it can be called from outside a TX (at least testObservationLastNAllParamsPopulated does) * Update test validation * Allow hsearch sorted offset searches * Add changelog Co-authored-by: Michael Buckley Co-authored-by: juan.marchionatto --- ...en-offset-greater-than-zero-specified.yaml | 6 +++ .../fhir/jpa/dao/FulltextSearchSvcImpl.java | 2 +- ...esourceDaoR4SearchWithElasticSearchIT.java | 38 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3863-hsearch-sort-doesnt-work-when-offset-greater-than-zero-specified.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3863-hsearch-sort-doesnt-work-when-offset-greater-than-zero-specified.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3863-hsearch-sort-doesnt-work-when-offset-greater-than-zero-specified.yaml new file mode 100644 index 00000000000..407b63fcd3c --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3863-hsearch-sort-doesnt-work-when-offset-greater-than-zero-specified.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 3863 +title: "Previously a lucene/elastic enabled sorted search including offset greater than zero was not sorting results. + This has now been fixed." + diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java index cb16f8f7e09..fa66b09a7a7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java @@ -420,7 +420,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { .select(this::buildResourceSelectClause) .where(f -> buildWhereClause(f, theResourceType, theParams, null)); - if (theParams.getSort() != null && offset == 0) { + if (theParams.getSort() != null) { query.sort( f -> myExtendedFulltextSortHelper.getSortClauses(f, theParams.getSort(), theResourceType) ); } diff --git a/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java index dc4cf72387f..c3c33256104 100644 --- a/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java +++ b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java @@ -1504,6 +1504,30 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl assertNotFind("when one predicate matches each object", "/Observation" + "?value-quantity=0.06|" + UCUM_CODESYSTEM_URL + "|10*3/L"); } + + @Nested + @Disabled // These conversions are not supported by the library we use + public class TemperatureUnitConversions { + + @Test + public void celsiusToFahrenheit() { + withObservationWithQuantity(37.5, UCUM_CODESYSTEM_URL, "Cel" ); + + assertFind( "when eq UCUM 99.5 degF", "/Observation?value-quantity=99.5|" + UCUM_CODESYSTEM_URL + "|degF"); + assertNotFind( "when eq UCUM 101.1 degF", "/Observation?value-quantity=101.1|" + UCUM_CODESYSTEM_URL + "|degF"); + assertNotFind( "when eq UCUM 97.8 degF", "/Observation?value-quantity=97.8|" + UCUM_CODESYSTEM_URL + "|degF"); + } + +// @Test + public void fahrenheitToCelsius() { + withObservationWithQuantity(99.5, UCUM_CODESYSTEM_URL, "degF" ); + + assertFind( "when eq UCUM 37.5 Cel", "/Observation?value-quantity=99.5|" + UCUM_CODESYSTEM_URL + "|Cel"); + assertNotFind( "when eq UCUM 38.1 Cel", "/Observation?value-quantity=101.1|" + UCUM_CODESYSTEM_URL + "|Cel"); + assertNotFind( "when eq UCUM 36.9 Cel", "/Observation?value-quantity=97.8|" + UCUM_CODESYSTEM_URL + "|Cel"); + } + } + } @@ -2094,6 +2118,20 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl assertThat(getResultIds(result), contains(raId3, raId2, raId1)); } + @Test + public void sortWithOffset() { + String raId1 = createRiskAssessmentWithPredictionProbability(0.23).getIdPart(); + String raId2 = createRiskAssessmentWithPredictionProbability(0.38).getIdPart(); + String raId3 = createRiskAssessmentWithPredictionProbability(0.76).getIdPart(); + + myCaptureQueriesListener.clear(); + IBundleProvider result = myTestDaoSearch.searchForBundleProvider("/RiskAssessment?_sort=-probability&_offset=1"); + + assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "we build the bundle with no sql"); + // requested profile (uri) descending so order should be id2, id1 + assertThat(getResultIds(result), contains(raId2, raId1)); + } + } @Nested