From 5586b325cea8c89b665ae0a8e19ebc83b66abe03 Mon Sep 17 00:00:00 2001 From: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Date: Thu, 28 Jul 2022 19:09:31 -0400 Subject: [PATCH] Issue 3857 elastic lucene search returns only 50 results when offset0 and count100 (#3858) * Add logback definition to project to stop debugging all output * Fix limit definition to include _offset=0 case * Move test required resources from previous test module * Add changelog Co-authored-by: juan.marchionatto --- ...x-50-entries-for-offset-count-request.yaml | 6 ++ .../fhir/jpa/dao/FulltextSearchSvcImpl.java | 5 +- ...esourceDaoR4SearchWithElasticSearchIT.java | 15 ++++ .../src/test/resources/logback-test.xml | 70 +++++++++++++++++++ .../test/resources/r4/expand-multi-cs.json | 0 .../resources/r4/expand-multi-vs-all.json | 0 6 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3857-hsearch-returns-max-50-entries-for-offset-count-request.yaml create mode 100644 hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/logback-test.xml rename {hapi-fhir-jpaserver-test-utilities => hapi-fhir-jpaserver-elastic-test-utilities}/src/test/resources/r4/expand-multi-cs.json (100%) rename {hapi-fhir-jpaserver-test-utilities => hapi-fhir-jpaserver-elastic-test-utilities}/src/test/resources/r4/expand-multi-vs-all.json (100%) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3857-hsearch-returns-max-50-entries-for-offset-count-request.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3857-hsearch-returns-max-50-entries-for-offset-count-request.yaml new file mode 100644 index 00000000000..f7d4cc11a0e --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3857-hsearch-returns-max-50-entries-for-offset-count-request.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 3857 +title: "Previously a lucene/elastic enabled search including offset=0 and count > 50 was returning only 50 resources + 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 ca5b6cba348..cb16f8f7e09 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 @@ -405,10 +405,11 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { @Override @Transactional(readOnly = true) public List searchForResources(String theResourceType, SearchParameterMap theParams) { - int offset = 0; int limit = DEFAULT_MAX_PAGE_SIZE; + int offset = 0; + int limit = theParams.getCount() == null ? DEFAULT_MAX_PAGE_SIZE : theParams.getCount(); + if (theParams.getOffset() != null && theParams.getOffset() != 0) { offset = theParams.getOffset(); - limit = theParams.getCount() == null ? DEFAULT_MAX_PAGE_SIZE : theParams.getCount(); // indicate param was already processed, otherwise queries DB to process it theParams.setOffset(null); } 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 d217667954f..748790cc30e 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 @@ -1874,6 +1874,21 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl // also validate no extra SQL queries were executed assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "bundle was built with no sql"); } + + @Test + public void offsetAndCountReturnsMoreThan50() { + for (int i = 0; i < 60; i++) { + myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-" + i))); + } + + myCaptureQueriesListener.clear(); + List resultIds = myTestDaoSearch.searchForIds("Observation?_offset=0&_count=100"); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + assertEquals(60, resultIds.size()); + // also validate no extra SQL queries were executed + assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "bundle was built with no sql"); + } } @Nested diff --git a/hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/logback-test.xml new file mode 100644 index 00000000000..4b90b3c8130 --- /dev/null +++ b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/logback-test.xml @@ -0,0 +1,70 @@ + + + + %d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] %msg%n + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/resources/r4/expand-multi-cs.json b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/r4/expand-multi-cs.json similarity index 100% rename from hapi-fhir-jpaserver-test-utilities/src/test/resources/r4/expand-multi-cs.json rename to hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/r4/expand-multi-cs.json diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/resources/r4/expand-multi-vs-all.json b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/r4/expand-multi-vs-all.json similarity index 100% rename from hapi-fhir-jpaserver-test-utilities/src/test/resources/r4/expand-multi-vs-all.json rename to hapi-fhir-jpaserver-elastic-test-utilities/src/test/resources/r4/expand-multi-vs-all.json