fixing test

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java

@@ -55,6 +55,7 @@ import ca.uhn.fhir.jpa.model.search.SearchBuilderLoadIncludesParameters;
 import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails;
 import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage;
 import ca.uhn.fhir.jpa.search.SearchConstants;
+import ca.uhn.fhir.jpa.search.builder.models.ListWrappingSearchQueryExecutor;
 import ca.uhn.fhir.jpa.search.builder.sql.GeneratedSql;
 import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder;
 import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryExecutor;
@@ -459,10 +460,15 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 		} else {
 			// do everything in the database.
 			createChunkedQuery(
-					theParams, sort, theOffset, theMaximumResults, theCountOnlyFlag, theRequest, null,
+				theParams,
+				sort,
+				theOffset,
+				theMaximumResults,
+				theCountOnlyFlag,
+				theRequest,
+				null,
 				queries
 			);
-//			query.ifPresent(queries::add);
 		}
 
 		return queries;
@@ -552,10 +558,16 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 		if (thePids.size() < getMaximumPageSize()) {
 			normalizeIdListForLastNInClause(thePids);
 		}
-//		Optional<SearchQueryExecutor> query =
+		createChunkedQuery(
-				createChunkedQuery(theParams, sort, theOffset, thePids.size(), theCount, theRequest, thePids,
+			theParams,
-					theQueries);
+			sort,
-//		query.ifPresent(t -> theQueries.add(t));
+			theOffset,
+			thePids.size(),
+			theCount,
+			theRequest,
+			thePids,
+			theQueries
+		);
 	}
 
 	/**
@@ -563,7 +575,7 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 	 *
 	 * @param theTargetPids
 	 */
-	private void extractTargetPidsFromIdParams(HashSet<Long> theTargetPids) {
+	private void extractTargetPidsFromIdParams(HashSet<Long> theTargetPids, List<ISearchQueryExecutor> theSearchQueryExecutors) {
 		// get all the IQueryParameterType objects
 		// for _id -> these should all be StringParam values
 		HashSet<String> ids = new HashSet<>();
@@ -587,15 +599,17 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 		// this will throw if an id is not found
 		Map<String, JpaPid> idToPid = myIdHelperService.resolveResourcePersistentIds(
 				myRequestPartitionId, myResourceName, new ArrayList<>(ids));
-		if (myAlsoIncludePids == null) {
-			myAlsoIncludePids = new ArrayList<>();
-		}
 
 		// add the pids to targetPids
 		for (JpaPid pid : idToPid.values()) {
-			myAlsoIncludePids.add(pid);
 			theTargetPids.add(pid.getId());
 		}
+
+		// add the target pids to our executors as the first
+		// results iterator to go through
+		theSearchQueryExecutors.add(new ListWrappingSearchQueryExecutor(
+			new ArrayList<>(theTargetPids)
+		));
 	}
 
 	private void createChunkedQuery(
@@ -641,7 +655,7 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 		if (myParams.getEverythingMode() != null) {
 			HashSet<Long> targetPids = new HashSet<>();
 			if (myParams.get(IAnyResource.SP_RES_ID) != null) {
-				extractTargetPidsFromIdParams(targetPids);
+				extractTargetPidsFromIdParams(targetPids, theSearchQueryExecutors);
 			} else {
 				// For Everything queries, we make the query root by the ResourceLink table, since this query
 				// is basically a reverse-include search. For type/Everything (as opposed to instance/Everything)
@@ -661,33 +675,10 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 				Object[] args = allTargetsSql.getBindVariables().toArray(new Object[0]);
 
 				List<Long> output = jdbcTemplate.query(sql, args, new SingleColumnRowMapper<>(Long.class));
-//				if (myAlsoIncludePids == null) {
-//					myAlsoIncludePids = new ArrayList<>(output.size());
-//				}
-//				myAlsoIncludePids.addAll(JpaPid.fromLongList(output));
 
-				theSearchQueryExecutors.add(new ISearchQueryExecutor() {
+				// we add the search to the list of search executors as the first
-					private final Iterator<Long> myIterator = output.iterator();
+				// search executor/resultsiterator
-
+				theSearchQueryExecutors.add(new ListWrappingSearchQueryExecutor(output));
-					@Override
-					public void close() {
-
-					}
-
-					@Override
-					public boolean hasNext() {
-						return myIterator.hasNext();
-					}
-
-					@Override
-					public Long next() {
-						return myIterator.next();
-					}
-				});
-
-//				theSearchQueryExecutors.add(
-//					mySqlBuilderFactory.newSearchQueryExecutor(allTargetsSql, myMaxResultsToFetch)
-//				);
 			}
 
 			List<String> typeSourceResources = new ArrayList<>();
@@ -785,14 +776,10 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 		 * Now perform the search
 		 */
 		GeneratedSql generatedSql = sqlBuilder.generate(theOffset, myMaxResultsToFetch);
-		if (generatedSql.isMatchNothing()) {
+		if (!generatedSql.isMatchNothing()) {
-//			return Optional.empty();
+			SearchQueryExecutor executor = mySqlBuilderFactory.newSearchQueryExecutor(generatedSql, myMaxResultsToFetch);
-			return;
+			theSearchQueryExecutors.add(executor);
 		}
-
-		SearchQueryExecutor executor = mySqlBuilderFactory.newSearchQueryExecutor(generatedSql, myMaxResultsToFetch);
-		theSearchQueryExecutors.add(executor);
-//		return Optional.of(executor);
 	}
 
 	private Collection<String> extractTypeSourceResourcesFromParams() {
@@ -2025,120 +2012,108 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 					}
 
 					/*
-					 * assigns the results iterator.
+					 * assigns the results iterator
-					 * Can also assign and populate myAlsoIncludePids.
+					 * and populates the myQueryList.
-					 * Specifically in type/$everything mode
-					 * (ie, /Patient/$everything)
 					 */
 					initializeIteratorQuery(myOffset, myMaxResultsToFetch);
 
-					// but if it doesn't, we'll set an empty list here
+					// if it's null, we'll set an empty list here
 					if (myAlsoIncludePids == null) {
 						myAlsoIncludePids = new ArrayList<>();
 					}
 				}
 
 				if (myNext == null) {
-					// we first consume any alsoIncludePids
+					// no next means we need a new query (if one is available)
-					for (JpaPid next : myAlsoIncludePids) {
+					while (myResultsIterator.hasNext() || !myQueryList.isEmpty()) {
-						if (next != null)
+						// Update iterator with next chunk if necessary.
-							if (myPidSet.add(next)) {
+						if (!myResultsIterator.hasNext()) {
-								myNext = next;
+							retrieveNextIteratorQuery();
+
+							// if our new results iterator is also empty
+							// we're done here
+							if (!myResultsIterator.hasNext()) {
 								break;
 							}
-					}
+						}
 
-					if (myNext == null) {
+						Long nextLong = myResultsIterator.next();
-						// no next means we need a new query (if one is available)
+						if (myHavePerfTraceFoundIdHook) {
-						while (myResultsIterator.hasNext() || !myQueryList.isEmpty()) {
+							HookParams params = new HookParams()
-							// Update iterator with next chunk if necessary.
+								.add(Integer.class, System.identityHashCode(this))
-							if (!myResultsIterator.hasNext()) {
+								.add(Object.class, nextLong);
-								retrieveNextIteratorQuery();
+							CompositeInterceptorBroadcaster.doCallHooks(
+								myInterceptorBroadcaster,
+								myRequest,
+								Pointcut.JPA_PERFTRACE_SEARCH_FOUND_ID,
+								params);
+						}
 
-								// if our new results iterator is also empty
+						if (nextLong != null) {
-								// we're done here
+							JpaPid next = JpaPid.fromId(nextLong);
-								if (!myResultsIterator.hasNext()) {
+							if (myPidSet.add(next)) {
-									break;
+								myNext = next;
-								}
+								myNonSkipCount++;
+								break;
+							} else {
+								mySkipCount++;
 							}
+						}
 
-							Long nextLong = myResultsIterator.next();
+						if (!myResultsIterator.hasNext()) {
-							if (myHavePerfTraceFoundIdHook) {
+							if (myMaxResultsToFetch != null
-								HookParams params = new HookParams()
+								&& (mySkipCount + myNonSkipCount == myMaxResultsToFetch)) {
-										.add(Integer.class, System.identityHashCode(this))
+								if (mySkipCount > 0 && myNonSkipCount == 0) {
-										.add(Object.class, nextLong);
+
-								CompositeInterceptorBroadcaster.doCallHooks(
+									StorageProcessingMessage message = new StorageProcessingMessage();
+									String msg = "Pass completed with no matching results seeking rows "
+										+ myPidSet.size() + "-" + mySkipCount
+										+ ". This indicates an inefficient query! Retrying with new max count of "
+										+ myMaxResultsToFetch;
+									ourLog.warn(msg);
+									message.setMessage(msg);
+									HookParams params = new HookParams()
+										.add(RequestDetails.class, myRequest)
+										.addIfMatchesType(ServletRequestDetails.class, myRequest)
+										.add(StorageProcessingMessage.class, message);
+									CompositeInterceptorBroadcaster.doCallHooks(
 										myInterceptorBroadcaster,
 										myRequest,
-										Pointcut.JPA_PERFTRACE_SEARCH_FOUND_ID,
+										Pointcut.JPA_PERFTRACE_WARNING,
 										params);
-							}
 
-							if (nextLong != null) {
+									myMaxResultsToFetch += 1000;
-								JpaPid next = JpaPid.fromId(nextLong);
+									initializeIteratorQuery(myOffset, myMaxResultsToFetch);
+								}
+							}
+						}
+					}
+				}
+
+				if (myNext == null) {
+					// if we got here, it means the current PjaPid has already been processed
+					// and we will decide (here) if we need to fetch related resources recursively
+					if (myFetchIncludesForEverythingOperation) {
+						myIncludesIterator = new IncludesIterator(myPidSet, myRequest);
+						myFetchIncludesForEverythingOperation = false;
+					}
+					if (myIncludesIterator != null) {
+						while (myIncludesIterator.hasNext()) {
+							JpaPid next = myIncludesIterator.next();
+							if (next != null)
 								if (myPidSet.add(next)) {
 									myNext = next;
-									myNonSkipCount++;
 									break;
-								} else {
-									mySkipCount++;
 								}
-							}
-
-							if (!myResultsIterator.hasNext()) {
-								if (myMaxResultsToFetch != null
-										&& (mySkipCount + myNonSkipCount == myMaxResultsToFetch)) {
-									if (mySkipCount > 0 && myNonSkipCount == 0) {
-
-										StorageProcessingMessage message = new StorageProcessingMessage();
-										String msg = "Pass completed with no matching results seeking rows "
-												+ myPidSet.size() + "-" + mySkipCount
-												+ ". This indicates an inefficient query! Retrying with new max count of "
-												+ myMaxResultsToFetch;
-										ourLog.warn(msg);
-										message.setMessage(msg);
-										HookParams params = new HookParams()
-												.add(RequestDetails.class, myRequest)
-												.addIfMatchesType(ServletRequestDetails.class, myRequest)
-												.add(StorageProcessingMessage.class, message);
-										CompositeInterceptorBroadcaster.doCallHooks(
-												myInterceptorBroadcaster,
-												myRequest,
-												Pointcut.JPA_PERFTRACE_WARNING,
-												params);
-
-										myMaxResultsToFetch += 1000;
-										initializeIteratorQuery(myOffset, myMaxResultsToFetch);
-									}
-								}
-							}
 						}
-					}
+						if (myNext == null) {
-
-					if (myNext == null) {
-						// if we got here, it means the current PjaPid has already been processed
-						// and we will decide (here) if we need to fetch related resources recursively
-						if (myFetchIncludesForEverythingOperation) {
-							myIncludesIterator = new IncludesIterator(myPidSet, myRequest);
-							myFetchIncludesForEverythingOperation = false;
-						}
-						if (myIncludesIterator != null) {
-							while (myIncludesIterator.hasNext()) {
-								JpaPid next = myIncludesIterator.next();
-								if (next != null)
-									if (myPidSet.add(next)) {
-										myNext = next;
-										break;
-									}
-							}
-							if (myNext == null) {
-								myNext = NO_MORE;
-							}
-						} else {
 							myNext = NO_MORE;
 						}
+					} else {
+						myNext = NO_MORE;
 					}
-				} // if we need to fetch the next result
+				}
+
 
 				mySearchRuntimeDetails.setFoundMatchesCount(myPidSet.size());
 
@@ -2151,21 +2126,21 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
 
 			if (myFirst) {
 				HookParams params = new HookParams()
-						.add(RequestDetails.class, myRequest)
+					.add(RequestDetails.class, myRequest)
-						.addIfMatchesType(ServletRequestDetails.class, myRequest)
+					.addIfMatchesType(ServletRequestDetails.class, myRequest)
-						.add(SearchRuntimeDetails.class, mySearchRuntimeDetails);
+					.add(SearchRuntimeDetails.class, mySearchRuntimeDetails);
 				CompositeInterceptorBroadcaster.doCallHooks(
-						myInterceptorBroadcaster, myRequest, Pointcut.JPA_PERFTRACE_SEARCH_FIRST_RESULT_LOADED, params);
+					myInterceptorBroadcaster, myRequest, Pointcut.JPA_PERFTRACE_SEARCH_FIRST_RESULT_LOADED, params);
 				myFirst = false;
 			}
 
 			if (NO_MORE.equals(myNext)) {
 				HookParams params = new HookParams()
-						.add(RequestDetails.class, myRequest)
+					.add(RequestDetails.class, myRequest)
-						.addIfMatchesType(ServletRequestDetails.class, myRequest)
+					.addIfMatchesType(ServletRequestDetails.class, myRequest)
-						.add(SearchRuntimeDetails.class, mySearchRuntimeDetails);
+					.add(SearchRuntimeDetails.class, mySearchRuntimeDetails);
 				CompositeInterceptorBroadcaster.doCallHooks(
-						myInterceptorBroadcaster, myRequest, Pointcut.JPA_PERFTRACE_SEARCH_SELECT_COMPLETE, params);
+					myInterceptorBroadcaster, myRequest, Pointcut.JPA_PERFTRACE_SEARCH_SELECT_COMPLETE, params);
 			}
 		}
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/models/ListWrappingSearchQueryExecutor.java

@@ -0,0 +1,33 @@
+package ca.uhn.fhir.jpa.search.builder.models;
+
+import ca.uhn.fhir.jpa.search.builder.ISearchQueryExecutor;
+
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * An implementation of ISearchQueryExecutor that wraps a simple list.
+ */
+public class ListWrappingSearchQueryExecutor implements ISearchQueryExecutor {
+
+	private final Iterator<Long> myIterator;
+
+	public ListWrappingSearchQueryExecutor(List<Long> theList) {
+		myIterator = theList.iterator();
+	}
+
+	@Override
+	public void close() {
+
+	}
+
+	@Override
+	public boolean hasNext() {
+		return myIterator.hasNext();
+	}
+
+	@Override
+	public Long next() {
+		return myIterator.next();
+	}
+}

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientEverythingPaginationR4Test.java

@@ -101,8 +101,10 @@ public class PatientEverythingPaginationR4Test extends BaseResourceProviderR4Tes
 	@ValueSource(booleans = { true, false })
 	public void testEverythingPagination_LastPage(boolean theProvideCountBool) throws IOException {
 		// setup
-		myStorageSettings.setSearchPreFetchThresholds(Arrays.asList(10, 50,-1));
+		List<Integer> prefetchThreshold = Arrays.asList(10, 50,-1);
-		// 3 pages
+		myStorageSettings.setSearchPreFetchThresholds(prefetchThreshold);
+
+		// 3 pages @ 50
 		int total = 154;
 		createPatients(total);
 		Set<String> ids = new HashSet<>();
@@ -119,7 +121,11 @@ public class PatientEverythingPaginationR4Test extends BaseResourceProviderR4Tes
 
 		// first page
 		List<Patient> patientsPage = BundleUtil.toListOfResourcesOfType(myFhirContext, bundle, Patient.class);
-//		assertEquals(50, patientsPage.size());
+		if (theProvideCountBool) {
+			assertEquals(50, patientsPage.size());
+		} else {
+			assertEquals(10, patientsPage.size());
+		}
 		for (Patient p : patientsPage) {
 			assertTrue(ids.add(p.getId()));
 		}
@@ -134,24 +140,19 @@ public class PatientEverythingPaginationR4Test extends BaseResourceProviderR4Tes
 			for (Patient p : patientsPage) {
 				assertTrue(ids.add(p.getId()));
 			}
-//			assertEquals(50, patientsPage.size());
 			nextUrl = BundleUtil.getLinkUrlOfType(myFhirContext, bundle, LINK_NEXT);
+			if (nextUrl != null) {
+				if (theProvideCountBool) {
+					assertEquals(50, patientsPage.size());
+				} else {
+					assertEquals(10, patientsPage.size());
+				}
+			} else {
+				assertEquals(4, patientsPage.size());
+			}
 		} while (nextUrl != null);
 		assertNull(nextUrl);
 
-		// last page
-//		nextUrl = BundleUtil.getLinkUrlOfType(myFhirContext, bundle, LINK_NEXT);
-//		assertNotNull(nextUrl);
-//		bundle = fetchBundle(nextUrl);
-//		nextUrl = BundleUtil.getLinkUrlOfType(myFhirContext, bundle, LINK_NEXT);
-//		assertNull(nextUrl);
-//		patientsPage = BundleUtil.toListOfResourcesOfType(myFhirContext, bundle, Patient.class);
-//
-//		assertEquals(4, patientsPage.size());
-//		for (Patient p : patientsPage) {
-//			assertTrue(ids.add(p.getId()));
-//		}
-
 		assertEquals(total, ids.size());
 	}