From 6594e4f76bb8ec59518175ad1f97e8bf7ac57d2d Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 14 Nov 2024 11:48:24 -0500 Subject: [PATCH 01/10] added test and fixed performance --- ...9-reduce-memory-overhead-for-searches.yaml | 8 +++ .../jpa/search/builder/SearchBuilder.java | 39 +++++++++-- .../builder/sql/SearchQueryExecutor.java | 3 + .../FhirResourceDaoR4SearchOptimizedTest.java | 69 ++++++++++++++++++- .../jpa/api/config/JpaStorageSettings.java | 2 +- 5 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6469-reduce-memory-overhead-for-searches.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6469-reduce-memory-overhead-for-searches.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6469-reduce-memory-overhead-for-searches.yaml new file mode 100644 index 00000000000..173bc12c1b5 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6469-reduce-memory-overhead-for-searches.yaml @@ -0,0 +1,8 @@ +--- +type: perf +issue: 6469 +title: "Searching for a large number of resources can use a lot of + memory, due to the nature of deduplication of results in memory. + We will instead push this responsibility to the db to save + reduce this overhead. +" diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 9a28921f1ae..9b2560c0514 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -638,7 +638,8 @@ public class SearchBuilder implements ISearchBuilder { boolean theCountOnlyFlag, RequestDetails theRequest, List thePidList, - List theSearchQueryExecutors) { + List theSearchQueryExecutors + ) { SearchQueryBuilder sqlBuilder = new SearchQueryBuilder( myContext, myStorageSettings, @@ -719,9 +720,13 @@ public class SearchBuilder implements ISearchBuilder { } /* - * If offset is present, we want deduplicate the results by using GROUP BY + * If offset is present, we want to deduplicate the results by using GROUP BY; + * OR + * if the MaxResultsToFetch is null, we are requesting "everything", + * so we'll let the db do the deduplication (instead of in-memory) */ - if (theOffset != null) { + // todo - if the limit is not present, we should group + if (theOffset != null || myMaxResultsToFetch == null) { queryStack3.addGrouping(); queryStack3.setUseAggregate(true); } @@ -2403,7 +2408,14 @@ public class SearchBuilder implements ISearchBuilder { if (nextLong != null) { JpaPid next = JpaPid.fromId(nextLong); - if (myPidSet.add(next) && doNotSkipNextPidForEverything()) { + // TODO - check if it's the unlimited case + if (doNotSkipNextPidForEverything() && !myPidSet.contains(next)) { + if (myMaxResultsToFetch != null) { + // we only add to the map if we aren't fetching "everything"; + // otherwise, we let the de-duplication happen in the database + // (see createChunkedQueryNormalSearch above) + myPidSet.add(next); + } myNext = next; myNonSkipCount++; break; @@ -2478,6 +2490,25 @@ public class SearchBuilder implements ISearchBuilder { } } + /** + * Determine if the next value should be skipped or not. + * + * We skip if: + * * we are in everything mode + * AND + * * we've already seen next (and we add it to our list of seen ids) + * OR + * * we've already seen the result and we're fetching everything; we + * don't add it to the map in this case because we might be seeing + * millions of records and don't want to actually fill up our map; + * the database will do the deduplication for us. + */ + private boolean shouldSkip(JpaPid next) { + return !doNotSkipNextPidForEverything() && ( + (myMaxResultsToFetch == null && myPidSet.contains(next)) + || (!myPidSet.add(next))); + } + private Integer calculateMaxResultsToFetch() { if (myParams.getLoadSynchronousUpTo() != null) { return myParams.getLoadSynchronousUpTo(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java index f636ab7eb4c..08499e47749 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java @@ -55,6 +55,8 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { private ScrollableResultsIterator myResultSet; private Long myNext; + private Integer myMaxResultsToFetch; + /** * Constructor */ @@ -62,6 +64,7 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { Validate.notNull(theGeneratedSql, "theGeneratedSql must not be null"); myGeneratedSql = theGeneratedSql; myQueryInitialized = false; + myMaxResultsToFetch = theMaxResultsToFetch; } /** diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java index e03d3725746..b2ce52e759f 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java @@ -15,6 +15,7 @@ import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.jpa.util.QueryParameterUtils; +import ca.uhn.fhir.jpa.util.SqlQuery; import ca.uhn.fhir.rest.api.SearchTotalModeEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.SummaryEnum; @@ -33,6 +34,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.BaseResource; import org.hl7.fhir.r4.model.BodyStructure; import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Coding; @@ -65,14 +67,18 @@ import java.util.Locale; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.leftPad; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.fail; import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -556,6 +562,67 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { } + /** + * We want to use the db to deduplicate in the "fetch everything" + * case because it's more memory efficient. + */ + @Test + public void search_whenPastPreFetchLimit_usesDBToDeduplicate() { + // setup + IBundleProvider results; + List queries; + List ids; + + create200Patients(); + + myCaptureQueriesListener.clear(); + // set the prefetch thresholds low so we don't need to + // search for tons of resources + myStorageSettings.setSearchPreFetchThresholds(List.of(5, 10, -1)); + + // basic search map + SearchParameterMap map = new SearchParameterMap(); + map.setSort(new SortSpec(BaseResource.SP_RES_LAST_UPDATED)); + + // test + results = myPatientDao.search(map, null); + String uuid = results.getUuid(); + ourLog.debug("** Search returned UUID: {}", uuid); + assertNotNull(results); + ids = toUnqualifiedVersionlessIdValues(results, 0, 9, true); + assertEquals(9, ids.size()); + + // first search was < 10 (our max pre-fetch value); so we should + // expect no "group by" queries (we deduplicate in memory) + queries = findGroupByQueries(); + assertTrue(queries.isEmpty()); + myCaptureQueriesListener.clear(); + + ids = toUnqualifiedVersionlessIdValues(results, 10, 100, true); + assertEquals(90, ids.size()); + + // we are now requesting > 10 results, meaning we should be using the + // database to deduplicate any values not fetched yet; + // so we *do* expect to see a "group by" query + queries = findGroupByQueries(); + assertFalse(queries.isEmpty()); + assertEquals(1, queries.size()); + SqlQuery query = queries.get(0); + String sql = query.getSql(true, false); + // we expect a "GROUP BY t0.RES_ID" (but we'll be ambiguous about the table + // name, just in case) + Pattern p = Pattern.compile("GROUP BY .+\\.RES_ID"); + Matcher m = p.matcher(sql); + assertTrue(m.find()); + } + + private List findGroupByQueries() { + List queries = myCaptureQueriesListener.getSelectQueries(); + queries = queries.stream().filter(q -> q.getSql(true, false).toLowerCase().contains("group by")) + .collect(Collectors.toList()); + return queries; + } + @Test public void testFetchMoreThanFirstPageSizeInFirstPage() { create200Patients(); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java index 9b9e19034d0..cad57a018f1 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java @@ -189,7 +189,7 @@ public class JpaStorageSettings extends StorageSettings { // start with a tiny number so our first page always loads quickly. // If they fetch the second page, fetch more. // Use prime sizes to avoid empty next links. - private List mySearchPreFetchThresholds = Arrays.asList(13, 503, 2003, -1); + private List mySearchPreFetchThresholds = Arrays.asList(13, 503, 2003, 1000003, -1); private List myWarmCacheEntries = new ArrayList<>(); private boolean myEnforceReferenceTargetTypes = true; private ClientIdStrategyEnum myResourceClientIdStrategy = ClientIdStrategyEnum.ALPHANUMERIC; From 0be9b67036a3d46ce19c8db73dadf2390f307355 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 14 Nov 2024 13:18:24 -0500 Subject: [PATCH 02/10] spotless --- .../ca/uhn/fhir/jpa/search/builder/SearchBuilder.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 9b2560c0514..06322a0a660 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -638,8 +638,7 @@ public class SearchBuilder implements ISearchBuilder { boolean theCountOnlyFlag, RequestDetails theRequest, List thePidList, - List theSearchQueryExecutors - ) { + List theSearchQueryExecutors) { SearchQueryBuilder sqlBuilder = new SearchQueryBuilder( myContext, myStorageSettings, @@ -2504,9 +2503,8 @@ public class SearchBuilder implements ISearchBuilder { * the database will do the deduplication for us. */ private boolean shouldSkip(JpaPid next) { - return !doNotSkipNextPidForEverything() && ( - (myMaxResultsToFetch == null && myPidSet.contains(next)) - || (!myPidSet.add(next))); + return !doNotSkipNextPidForEverything() + && ((myMaxResultsToFetch == null && myPidSet.contains(next)) || (!myPidSet.add(next))); } private Integer calculateMaxResultsToFetch() { From b242620bb809d585f6fc8a6c5e04be090366944b Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 14 Nov 2024 13:33:45 -0500 Subject: [PATCH 03/10] cleanup --- .../jpa/search/builder/SearchBuilder.java | 23 ++----------------- .../builder/sql/SearchQueryExecutor.java | 3 --- .../jpa/api/config/JpaStorageSettings.java | 3 ++- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 06322a0a660..56df4b84019 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -724,7 +724,6 @@ public class SearchBuilder implements ISearchBuilder { * if the MaxResultsToFetch is null, we are requesting "everything", * so we'll let the db do the deduplication (instead of in-memory) */ - // todo - if the limit is not present, we should group if (theOffset != null || myMaxResultsToFetch == null) { queryStack3.addGrouping(); queryStack3.setUseAggregate(true); @@ -2407,12 +2406,12 @@ public class SearchBuilder implements ISearchBuilder { if (nextLong != null) { JpaPid next = JpaPid.fromId(nextLong); - // TODO - check if it's the unlimited case if (doNotSkipNextPidForEverything() && !myPidSet.contains(next)) { if (myMaxResultsToFetch != null) { // we only add to the map if we aren't fetching "everything"; // otherwise, we let the de-duplication happen in the database - // (see createChunkedQueryNormalSearch above) + // (see createChunkedQueryNormalSearch above), becuase it saves + // memory that way myPidSet.add(next); } myNext = next; @@ -2489,24 +2488,6 @@ public class SearchBuilder implements ISearchBuilder { } } - /** - * Determine if the next value should be skipped or not. - * - * We skip if: - * * we are in everything mode - * AND - * * we've already seen next (and we add it to our list of seen ids) - * OR - * * we've already seen the result and we're fetching everything; we - * don't add it to the map in this case because we might be seeing - * millions of records and don't want to actually fill up our map; - * the database will do the deduplication for us. - */ - private boolean shouldSkip(JpaPid next) { - return !doNotSkipNextPidForEverything() - && ((myMaxResultsToFetch == null && myPidSet.contains(next)) || (!myPidSet.add(next))); - } - private Integer calculateMaxResultsToFetch() { if (myParams.getLoadSynchronousUpTo() != null) { return myParams.getLoadSynchronousUpTo(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java index 08499e47749..f636ab7eb4c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java @@ -55,8 +55,6 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { private ScrollableResultsIterator myResultSet; private Long myNext; - private Integer myMaxResultsToFetch; - /** * Constructor */ @@ -64,7 +62,6 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { Validate.notNull(theGeneratedSql, "theGeneratedSql must not be null"); myGeneratedSql = theGeneratedSql; myQueryInitialized = false; - myMaxResultsToFetch = theMaxResultsToFetch; } /** diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java index cad57a018f1..ab5cf7e20e1 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java @@ -188,7 +188,8 @@ public class JpaStorageSettings extends StorageSettings { // start with a tiny number so our first page always loads quickly. // If they fetch the second page, fetch more. - // Use prime sizes to avoid empty next links. + // we'll only fetch (by default) up to 1 million records, because after that, deduplication in local memory is + // prohibitive private List mySearchPreFetchThresholds = Arrays.asList(13, 503, 2003, 1000003, -1); private List myWarmCacheEntries = new ArrayList<>(); private boolean myEnforceReferenceTargetTypes = true; From 1d2bde268ecc5ff963a509c69d3b0219d29dec63 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Tue, 19 Nov 2024 13:53:09 -0500 Subject: [PATCH 04/10] code review fix --- .../jpa/search/builder/SearchBuilder.java | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 56df4b84019..4bbe54670c9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -198,6 +198,26 @@ public class SearchBuilder implements ISearchBuilder { private String mySearchUuid; private int myFetchSize; private Integer myMaxResultsToFetch; + + /** + * Set of PIDs of results that have already been returned in a search. + * + * Searches use pre-fetch thresholds to avoid returning every result in the db + * (see {@link JpaStorageSettings mySearchPreFetchThresholds}). These threshold values + * dictate the usage of this set. + * + * Results from searches returning *less* than a prefetch threshold are put into this set + * for 2 purposes: + * 1) skipping already seen resources. ie, client requesting next "page" of + * results should skip previously returned results + * 2) deduplication of returned results. ie, searches can return duplicate resources (due to + * sort and filter criteria), so this set will be used to avoid returning duplicate results. + * + * NOTE: if a client requests *more* resources than *all* prefetch thresholds, + * we push the work of "deduplication" to the database. No newly seen resource + * will be stored in this set (to avoid this set exploding in size and the JVM running out memory). + * We will, however, still use it to skip previously seen results. + */ private Set myPidSet; private boolean myHasNextIteratorQuery = false; private RequestPartitionId myRequestPartitionId; @@ -2408,10 +2428,12 @@ public class SearchBuilder implements ISearchBuilder { JpaPid next = JpaPid.fromId(nextLong); if (doNotSkipNextPidForEverything() && !myPidSet.contains(next)) { if (myMaxResultsToFetch != null) { - // we only add to the map if we aren't fetching "everything"; - // otherwise, we let the de-duplication happen in the database - // (see createChunkedQueryNormalSearch above), becuase it saves - // memory that way + /* + * We only add to the map if we aren't fetching "everything"; + * otherwise, we let the de-duplication happen in the database + * (see createChunkedQueryNormalSearch above), because it + * saves memory that way. + */ myPidSet.add(next); } myNext = next; From 2d9db1ce74c05a12b729c0192d1150b808a06644 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 21 Nov 2024 13:15:31 -0500 Subject: [PATCH 05/10] spotless --- .../main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 4bbe54670c9..a69137cc436 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -219,6 +219,7 @@ public class SearchBuilder implements ISearchBuilder { * We will, however, still use it to skip previously seen results. */ private Set myPidSet; + private boolean myHasNextIteratorQuery = false; private RequestPartitionId myRequestPartitionId; From 18635f727e05a30f8d79d908f75ac4cbfe5efa37 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Fri, 22 Nov 2024 16:54:31 -0500 Subject: [PATCH 06/10] fixing counts --- .../ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java | 1 + .../ca/uhn/fhir/jpa/search/builder/SearchBuilder.java | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java index 317b8fa2105..6b947683e9a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java @@ -130,6 +130,7 @@ public class SynchronousSearchSvcImpl implements ISynchronousSearchSvc { List> contentAndTerms = theParams.get(Constants.PARAM_CONTENT); List> textAndTerms = theParams.get(Constants.PARAM_TEXT); + // TODO - this count query should not be grouped count = theSb.createCountQuery( theParams, theSearchUuid, theRequestDetails, theRequestPartitionId); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index e457349252b..3103b0452eb 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -762,7 +762,7 @@ public class SearchBuilder implements ISearchBuilder { * if the MaxResultsToFetch is null, we are requesting "everything", * so we'll let the db do the deduplication (instead of in-memory) */ - if (theOffset != null || myMaxResultsToFetch == null) { + if (theOffset != null || (myMaxResultsToFetch == null && !theCountOnlyFlag)) { queryStack3.addGrouping(); queryStack3.setUseAggregate(true); } @@ -2507,7 +2507,11 @@ public class SearchBuilder implements ISearchBuilder { } } - mySearchRuntimeDetails.setFoundMatchesCount(myPidSet.size()); + if (myMaxResultsToFetch == null) { + mySearchRuntimeDetails.setFoundIndexMatchesCount(myNonSkipCount); + } else { + mySearchRuntimeDetails.setFoundMatchesCount(myPidSet.size()); + } } finally { // search finished - fire hooks From 43c8341d9c6fb4346ad5e226449c77b8e6d1af31 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 25 Nov 2024 10:32:38 -0500 Subject: [PATCH 07/10] fixing tests --- .../jpa/search/SynchronousSearchSvcImpl.java | 1 - ...rResourceDaoR4ComboNonUniqueParamTest.java | 17 ++-- .../jpa/dao/r4/PartitioningSqlR4Test.java | 90 +++++++++---------- 3 files changed, 51 insertions(+), 57 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java index 6b947683e9a..317b8fa2105 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SynchronousSearchSvcImpl.java @@ -130,7 +130,6 @@ public class SynchronousSearchSvcImpl implements ISynchronousSearchSvc { List> contentAndTerms = theParams.get(Constants.PARAM_CONTENT); List> textAndTerms = theParams.get(Constants.PARAM_TEXT); - // TODO - this count query should not be grouped count = theSb.createCountQuery( theParams, theSearchUuid, theRequestDetails, theRequestPartitionId); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java index 0b7dcd5878a..cfc2fb36cc2 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java @@ -165,7 +165,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); assertThat(myCaptureQueriesListener.getSelectQueries().stream().map(t -> t.getSql(true, false)).toList()).contains( - "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '-2634469377090377342')" + "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '-2634469377090377342') GROUP BY t0.RES_ID" ); logCapturedMessages(); @@ -291,7 +291,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); String sql = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false); - String expected = "SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_IDX_CMB_TOK_NU t0 ON (t1.RES_ID = t0.RES_ID) INNER JOIN HFJ_SPIDX_DATE t2 ON (t1.RES_ID = t2.RES_ID) WHERE ((t0.HASH_COMPLETE = '-2634469377090377342') AND ((t2.HASH_IDENTITY = '5247847184787287691') AND (((t2.SP_VALUE_LOW_DATE_ORDINAL >= '20210202') AND (t2.SP_VALUE_LOW_DATE_ORDINAL <= '20210202')) AND ((t2.SP_VALUE_HIGH_DATE_ORDINAL <= '20210202') AND (t2.SP_VALUE_HIGH_DATE_ORDINAL >= '20210202')))))"; + String expected = "SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_IDX_CMB_TOK_NU t0 ON (t1.RES_ID = t0.RES_ID) INNER JOIN HFJ_SPIDX_DATE t2 ON (t1.RES_ID = t2.RES_ID) WHERE ((t0.HASH_COMPLETE = '-2634469377090377342') AND ((t2.HASH_IDENTITY = '5247847184787287691') AND (((t2.SP_VALUE_LOW_DATE_ORDINAL >= '20210202') AND (t2.SP_VALUE_LOW_DATE_ORDINAL <= '20210202')) AND ((t2.SP_VALUE_HIGH_DATE_ORDINAL <= '20210202') AND (t2.SP_VALUE_HIGH_DATE_ORDINAL >= '20210202'))))) GROUP BY t1.RES_ID"; assertEquals(expected, sql); logCapturedMessages(); @@ -323,7 +323,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); String sql = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '7545664593829342272') AND ((t1.HASH_NORM_PREFIX = '6206712800146298788') AND (t1.SP_VALUE_NORMALIZED LIKE 'JAY%')))"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '7545664593829342272') AND ((t1.HASH_NORM_PREFIX = '6206712800146298788') AND (t1.SP_VALUE_NORMALIZED LIKE 'JAY%'))) GROUP BY t0.RES_ID"; assertEquals(expected, sql); logCapturedMessages(); @@ -363,7 +363,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '7196518367857292879')"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '7196518367857292879') GROUP BY t0.RES_ID"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); logCapturedMessages(); @@ -398,7 +398,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '2591238402961312979')"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '2591238402961312979') GROUP BY t0.RES_ID"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); } @@ -461,9 +461,8 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains("Patient/A"); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_IDX_CMB_TOK_NU t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND (t1.HASH_COMPLETE = '-8088946700286918311'))"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_IDX_CMB_TOK_NU t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND (t1.HASH_COMPLETE = '-8088946700286918311')) GROUP BY t0.RES_ID"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); - } /** @@ -497,7 +496,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains("Patient/A"); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND ((t1.HASH_NORM_PREFIX = '-3664262414674370905') AND (t1.SP_VALUE_NORMALIZED LIKE 'JONES%')))"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND ((t1.HASH_NORM_PREFIX = '-3664262414674370905') AND (t1.SP_VALUE_NORMALIZED LIKE 'JONES%'))) GROUP BY t0.RES_ID"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); } @@ -520,7 +519,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains("Observation/O1"); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE IN ('2445648980345828396','-6884698528022589694','-8034948665712960724') )"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE IN ('2445648980345828396','-6884698528022589694','-8034948665712960724') ) GROUP BY t0.RES_ID"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); logCapturedMessages(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java index 737c764724b..851b0304d6f 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java @@ -1289,7 +1289,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { // Only the read columns should be used, no criteria use partition assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); } // Read in null Partition @@ -1342,7 +1342,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { // Only the read columns should be used, no criteria use partition assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); // If this switches to 2 that would be fine + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(4); // If this switches to 2 that would be fine } // Read in null Partition @@ -1397,7 +1397,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { // Only the read columns should be used, no criteria use partition assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); } // Read in null Partition @@ -1475,10 +1475,8 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); assertEquals(0, searchOutcome.size()); } - } - @Test public void testSearch_MissingParamString_SearchAllPartitions() { myPartitionSettings.setIncludePartitionInSearchHashes(false); @@ -1500,7 +1498,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_MISSING = 'true'")); } @@ -1517,7 +1515,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_MISSING = 'false'")); } } @@ -1626,7 +1624,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "HFJ_RES_PARAM_PRESENT")); assertEquals(1, StringUtils.countMatches(searchSql, "HASH_PRESENCE = '1919227773735728687'")); } @@ -1653,7 +1651,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true)); String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); assertThat(StringUtils.countMatches(searchSql, "t0.PARTITION_ID = '1'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HFJ_RES_PARAM_PRESENT")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HASH_PRESENCE = '-3438137196820602023'")).as(searchSql).isEqualTo(1); @@ -1681,7 +1679,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true)); String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); assertThat(StringUtils.countMatches(searchSql, "t0.PARTITION_ID = '1'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HFJ_RES_PARAM_PRESENT")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HASH_PRESENCE = '1919227773735728687'")).as(searchSql).isEqualTo(1); @@ -1707,7 +1705,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); assertThat(StringUtils.countMatches(searchSql, "t0.PARTITION_ID IS NULL")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HFJ_RES_PARAM_PRESENT")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "HASH_PRESENCE = '1919227773735728687'")).as(searchSql).isEqualTo(1); @@ -1732,7 +1730,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); } @Test @@ -1752,7 +1750,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); } @Test @@ -1822,7 +1820,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(2, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // Date OR param @@ -1838,7 +1836,7 @@ 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, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // Date AND param @@ -1854,7 +1852,7 @@ 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, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // DateRangeParam @@ -1870,14 +1868,12 @@ 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, "PARTITION_ID")); // 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")); - } - @Test public void testSearch_DateParam_SearchSpecificPartitions() { myPartitionSettings.setIncludePartitionInSearchHashes(false); @@ -1907,7 +1903,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); assertThat(StringUtils.countMatches(searchSql, "SP_VALUE_LOW")).as(searchSql).isEqualTo(2); // Date OR param @@ -1923,7 +1919,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // Date AND param @@ -1939,7 +1935,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // DateRangeParam @@ -1955,7 +1951,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); // 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")); @@ -1987,7 +1983,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(2, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // Date OR param @@ -2003,7 +1999,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // Date AND param @@ -2019,7 +2015,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(4, StringUtils.countMatches(searchSql, "SP_VALUE_LOW")); // DateRangeParam @@ -2035,7 +2031,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); // 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")); @@ -2105,7 +2101,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); ourLog.info("Search SQL:\n{}", searchSql); assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); } @@ -2129,7 +2125,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_NORMALIZED")); } @@ -2152,7 +2148,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); searchSql = searchSql.toUpperCase(); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID IS NULL")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_NORMALIZED")); } @@ -2176,7 +2172,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_NORMALIZED")); } @@ -2209,7 +2205,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true)); String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); assertThat(searchSql).contains("PARTITION_ID IN ('1','2')"); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); } // Match two partitions including null @@ -2226,7 +2222,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); assertThat(searchSql).contains("PARTITION_ID IS NULL"); assertThat(searchSql).contains("PARTITION_ID = '1'"); - assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(4, StringUtils.countMatches(searchSql, "PARTITION_ID")); } } @@ -2287,7 +2283,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); searchSql = searchSql.toUpperCase(); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID IS NULL")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_NORMALIZED")); } @@ -2315,7 +2311,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "SP_VALUE_NORMALIZED")); } @@ -2372,7 +2368,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); // And with another param @@ -2389,7 +2385,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(1); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); assertThat(StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, ".HASH_SYS_AND_VALUE =")).as(searchSql).isEqualTo(1); @@ -2413,7 +2409,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID IS NULL")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); @@ -2441,7 +2437,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2463,7 +2459,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2489,7 +2485,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", searchSql); assertEquals(2, StringUtils.countMatches(searchSql, "JOIN")); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2514,7 +2510,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String 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, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2539,7 +2535,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2687,7 +2683,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { assertThat(StringUtils.countMatches(searchSql, "t0.PARTITION_ID = '1'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "t0.SRC_PATH = 'Observation.subject'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, "t0.TARGET_RESOURCE_ID = '" + patientId.getIdPartAsLong() + "'")).as(searchSql).isEqualTo(1); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); // Same query, different partition addReadPartition(2); @@ -2724,7 +2720,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { assertEquals(1, StringUtils.countMatches(searchSql, "t0.PARTITION_ID IS NULL")); assertEquals(1, StringUtils.countMatches(searchSql, "t0.SRC_PATH = 'Observation.subject'")); assertEquals(1, StringUtils.countMatches(searchSql, "t0.TARGET_RESOURCE_ID = '" + patientId.getIdPartAsLong() + "'")); - assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(3, StringUtils.countMatches(searchSql, "PARTITION_ID")); // Same query, different partition addReadPartition(2); @@ -2759,7 +2755,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true)); String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); assertThat(StringUtils.countMatches(searchSql.toUpperCase(Locale.US), "PARTITION_ID = '1'")).as(searchSql).isEqualTo(1); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); // Same query, different partition addReadPartition(2); @@ -2829,7 +2825,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); assertThat(StringUtils.countMatches(searchSql.toUpperCase(Locale.US), "PARTITION_ID IS NULL")).as(searchSql).isEqualTo(1); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(3); // Same query, different partition addReadPartition(2); From 5d396b1f6400e6e26cace1485dcc9529603b49f5 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 25 Nov 2024 13:52:27 -0500 Subject: [PATCH 08/10] fixing everything tests --- .../uhn/fhir/jpa/search/builder/SearchBuilder.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 3103b0452eb..4814f9f87ce 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -124,6 +124,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.repository.query.Param; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -2452,7 +2453,8 @@ public class SearchBuilder implements ISearchBuilder { if (nextLong != null) { JpaPid next = JpaPid.fromId(nextLong); - if (doNotSkipNextPidForEverything() && !myPidSet.contains(next)) { + + if (!myPidSet.contains(next)) { if (myMaxResultsToFetch != null) { /* * We only add to the map if we aren't fetching "everything"; @@ -2462,9 +2464,11 @@ public class SearchBuilder implements ISearchBuilder { */ myPidSet.add(next); } - myNext = next; - myNonSkipCount++; - break; + if (doNotSkipNextPidForEverything()) { + myNext = next; + myNonSkipCount++; + break; + } } else { mySkipCount++; } From 168ee41abc6c6a4261ceac4b0fd40d84da39d4df Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 25 Nov 2024 13:52:44 -0500 Subject: [PATCH 09/10] spotless --- .../main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 4814f9f87ce..8d9dcf409d6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -124,7 +124,6 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.repository.query.Param; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.transaction.support.TransactionSynchronizationManager; From c7a6cd98131916920002a21604e6cd4aa5c5c802 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 25 Nov 2024 16:27:09 -0500 Subject: [PATCH 10/10] intermittent fix --- .../dstu3/ResourceProviderDstu3Test.java | 155 +++++++++--------- 1 file changed, 77 insertions(+), 78 deletions(-) diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index a4bc55eef46..47eb77ebaa8 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -1,10 +1,5 @@ package ca.uhn.fhir.jpa.provider.dstu3; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.assertFalse; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.dao.data.ISearchDao; @@ -30,9 +25,7 @@ import ca.uhn.fhir.rest.gclient.StringClientParam; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.NumberParam; import ca.uhn.fhir.rest.param.ParamPrefixEnum; -import ca.uhn.fhir.rest.param.StringAndListParam; -import ca.uhn.fhir.rest.param.StringOrListParam; -import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.server.IPagingProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; @@ -133,6 +126,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; @@ -161,7 +155,11 @@ import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -184,10 +182,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(null); mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(QueryParameterUtils.DEFAULT_SYNC_SIZE); mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(false); - } - @Test public void testSearchBySourceTransactionId() { @@ -1485,53 +1481,51 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(ids.toString()); } - @Test - public void testEverythingInstanceWithContentFilter() { - Patient pt1 = new Patient(); - pt1.addName().setFamily("Everything").addGiven("Arthur"); - IIdType ptId1 = myPatientDao.create(pt1, mySrd).getId().toUnqualifiedVersionless(); - Patient pt2 = new Patient(); - pt2.addName().setFamily("Everything").addGiven("Arthur"); - IIdType ptId2 = myPatientDao.create(pt2, mySrd).getId().toUnqualifiedVersionless(); + @Test + public void testEverythingInstanceWithContentFilter() { + Patient pt1 = new Patient(); + pt1.addName().setFamily("Everything").addGiven("Arthur"); + IIdType ptId1 = myPatientDao.create(pt1, mySrd).getId().toUnqualifiedVersionless(); - Device dev1 = new Device(); - dev1.setManufacturer("Some Manufacturer"); - IIdType devId1 = myDeviceDao.create(dev1, mySrd).getId().toUnqualifiedVersionless(); + Patient pt2 = new Patient(); + pt2.addName().setFamily("Everything").addGiven("Arthur"); + IIdType ptId2 = myPatientDao.create(pt2, mySrd).getId().toUnqualifiedVersionless(); - Device dev2 = new Device(); - dev2.setManufacturer("Some Manufacturer 2"); - myDeviceDao.create(dev2, mySrd).getId().toUnqualifiedVersionless(); + Device dev1 = new Device(); + dev1.setManufacturer("Some Manufacturer"); + IIdType devId1 = myDeviceDao.create(dev1, mySrd).getId().toUnqualifiedVersionless(); - Observation obs1 = new Observation(); - obs1.getText().setDivAsString("
OBSTEXT1
"); - obs1.getSubject().setReferenceElement(ptId1); - obs1.getCode().addCoding().setCode("CODE1"); - obs1.setValue(new StringType("obsvalue1")); - obs1.getDevice().setReferenceElement(devId1); - IIdType obsId1 = myObservationDao.create(obs1, mySrd).getId().toUnqualifiedVersionless(); + Device dev2 = new Device(); + dev2.setManufacturer("Some Manufacturer 2"); + myDeviceDao.create(dev2, mySrd).getId().toUnqualifiedVersionless(); - Observation obs2 = new Observation(); - obs2.getSubject().setReferenceElement(ptId1); - obs2.getCode().addCoding().setCode("CODE2"); - obs2.setValue(new StringType("obsvalue2")); - IIdType obsId2 = myObservationDao.create(obs2, mySrd).getId().toUnqualifiedVersionless(); + // create an observation that links to Dev1 and Patient1 + Observation obs1 = new Observation(); + obs1.getText().setDivAsString("
OBSTEXT1
"); + obs1.getSubject().setReferenceElement(ptId1); + obs1.getCode().addCoding().setCode("CODE1"); + obs1.setValue(new StringType("obsvalue1")); + obs1.getDevice().setReferenceElement(devId1); + IIdType obsId1 = myObservationDao.create(obs1, mySrd).getId().toUnqualifiedVersionless(); - Observation obs3 = new Observation(); - obs3.getSubject().setReferenceElement(ptId2); - obs3.getCode().addCoding().setCode("CODE3"); - obs3.setValue(new StringType("obsvalue3")); - IIdType obsId3 = myObservationDao.create(obs3, mySrd).getId().toUnqualifiedVersionless(); + Observation obs2 = new Observation(); + obs2.getSubject().setReferenceElement(ptId1); + obs2.getCode().addCoding().setCode("CODE2"); + obs2.setValue(new StringType("obsvalue2")); + IIdType obsId2 = myObservationDao.create(obs2, mySrd).getId().toUnqualifiedVersionless(); - List actual; - StringAndListParam param; + Observation obs3 = new Observation(); + obs3.getSubject().setReferenceElement(ptId2); + obs3.getCode().addCoding().setCode("CODE3"); + obs3.setValue(new StringType("obsvalue3")); + IIdType obsId3 = myObservationDao.create(obs3, mySrd).getId().toUnqualifiedVersionless(); - ourLog.info("Pt1:{} Pt2:{} Obs1:{} Obs2:{} Obs3:{}", ptId1.getIdPart(), ptId2.getIdPart(), obsId1.getIdPart(), obsId2.getIdPart(), obsId3.getIdPart()); + List actual; - param = new StringAndListParam(); - param.addAnd(new StringOrListParam().addOr(new StringParam("obsvalue1"))); + ourLog.info("Pt1:{} Pt2:{} Obs1:{} Obs2:{} Obs3:{}", ptId1.getIdPart(), ptId2.getIdPart(), obsId1.getIdPart(), obsId2.getIdPart(), obsId3.getIdPart()); - //@formatter:off + //@formatter:off Parameters response = myClient .operation() .onInstance(ptId1) @@ -1540,10 +1534,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .execute(); //@formatter:on - actual = toUnqualifiedVersionlessIds((Bundle) response.getParameter().get(0).getResource()); - assertThat(actual).containsExactlyInAnyOrder(ptId1, obsId1, devId1); - - } + actual = toUnqualifiedVersionlessIds((Bundle) response.getParameter().get(0).getResource()); + assertThat(actual).containsExactlyInAnyOrder(ptId1, obsId1, devId1); + } /** * See #147"Patient" @@ -2497,6 +2490,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { assertEquals(1, resp.getTotal()); } +// @Disabled @Test public void testMetaOperations() { String methodName = "testMetaOperations"; @@ -2627,35 +2621,40 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testEverythingWithNoPagingProvider() { - myRestServer.setPagingProvider(null); + IPagingProvider pagingProvider = myRestServer.getPagingProvider(); + try { + myRestServer.setPagingProvider(null); - Patient p = new Patient(); - p.setActive(true); - String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + Patient p = new Patient(); + p.setActive(true); + String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); - for (int i = 0; i < 20; i++) { - Observation o = new Observation(); - o.getSubject().setReference(pid); - o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); - myObservationDao.create(o); + for (int i = 0; i < 20; i++) { + Observation o = new Observation(); + o.getSubject().setReference(pid); + o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); + myObservationDao.create(o); + } + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); + + Bundle response = myClient + .operation() + .onInstance(new IdType(pid)) + .named("everything") + .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) + .returnResourceType(Bundle.class) + .useHttpGet() + .execute(); + + assertThat(response.getEntry()).hasSize(10); + assertNull(response.getTotalElement().getValue()); + assertNull(response.getLink("next")); + } finally { + myRestServer.setPagingProvider(pagingProvider); } - - mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); - mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); - mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); - - Bundle response = myClient - .operation() - .onInstance(new IdType(pid)) - .named("everything") - .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) - .returnResourceType(Bundle.class) - .useHttpGet() - .execute(); - - assertThat(response.getEntry()).hasSize(10); - assertNull(response.getTotalElement().getValue()); - assertNull(response.getLink("next")); } @Test