Add forced offset search JPA interceptor (#2756)

* Add support for offset searches

* Tests working

* Add changelog

* Test fixes

* Test fix

* More test fixes

* Add commit counter

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2756-add-forced-offset-search-interceptor.yaml

@@ -0,0 +1,7 @@
+---
+type: add
+issue: 2756
+title: "A new interceptor has been addeed to the JPA server called `ForceOffsetSearchModeInterceptor`. This
+  interceptor forces all searches to be offset searches, instead of relying on the query cache. This means
+  that FHIR search operations will never result in any database write, which can be good for highly
+  concurrent servers."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java

@@ -145,11 +145,17 @@ public class Search implements ICachedSearchDetails, Serializable {
 	private byte[] mySearchParameterMap;
 
 	/**
-	 * This isn't currently persisted in the DB as it's only used for history. We could
+	 * This isn't currently persisted in the DB as it's only used for offset mode. We could
 	 * change this if needed in the future.
 	 */
 	@Transient
 	private Integer myOffset;
+	/**
+	 * This isn't currently persisted in the DB as it's only used for offset mode. We could
+	 * change this if needed in the future.
+	 */
+	@Transient
+	private Integer mySizeModeSize;
 
 	/**
 	 * Constructor
@@ -158,6 +164,10 @@ public class Search implements ICachedSearchDetails, Serializable {
 		super();
 	}
 
+	public Integer getSizeModeSize() {
+		return mySizeModeSize;
+	}
+
 	@Override
 	public String toString() {
 		return new ToStringBuilder(this)
@@ -396,14 +406,14 @@ public class Search implements ICachedSearchDetails, Serializable {
 		mySearchQueryStringHash = null;
 	}
 
-	public void setOffset(Integer theOffset) {
-		myOffset = theOffset;
-	}
-
 	public Integer getOffset() {
 		return myOffset;
 	}
 
+	public void setOffset(Integer theOffset) {
+		myOffset = theOffset;
+	}
+
 	@Nonnull
 	public static String createSearchQueryStringForStorage(@Nonnull String theSearchQueryString, @Nonnull RequestPartitionId theRequestPartitionId) {
 		String searchQueryString = theSearchQueryString;

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/ForceOffsetSearchModeInterceptor.java

@@ -0,0 +1,55 @@
+package ca.uhn.fhir.jpa.interceptor;
+
+/*-
+ * #%L
+ * HAPI FHIR JPA Server
+ * %%
+ * Copyright (C) 2014 - 2021 Smile CDR, Inc.
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+
+import ca.uhn.fhir.interceptor.api.Hook;
+import ca.uhn.fhir.interceptor.api.Interceptor;
+import ca.uhn.fhir.interceptor.api.Pointcut;
+import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import org.apache.commons.lang3.Validate;
+
+/**
+ * This interceptor for the HAPI FHIR JPA server forces all queries to
+ * be performed as offset queries. This means that the query cache will
+ * not be used and searches will never result in any writes to the
+ * database.
+ */
+@Interceptor
+public class ForceOffsetSearchModeInterceptor {
+
+	private Integer myDefaultCount = 100;
+
+	public void setDefaultCount(Integer theDefaultCount) {
+		Validate.notNull(theDefaultCount, "theDefaultCount must not be null");
+		myDefaultCount = theDefaultCount;
+	}
+
+	@Hook(Pointcut.STORAGE_PRESEARCH_REGISTERED)
+	public void storagePreSearchRegistered(SearchParameterMap theMap) {
+		if (theMap.getOffset() == null) {
+			theMap.setOffset(0);
+		}
+		if (theMap.getCount() == null) {
+			theMap.setCount(myDefaultCount);
+		}
+	}
+
+}

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

@@ -307,6 +307,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 
 		final String queryString = theParams.toNormalizedQueryString(myContext);
 		ourLog.debug("Registering new search {}", searchUuid);
+
 		Search search = new Search();
 		populateSearchEntity(theParams, theResourceType, searchUuid, queryString, search, theRequestPartitionId);
 
@@ -317,13 +318,15 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 			.addIfMatchesType(ServletRequestDetails.class, theRequestDetails)
 			.add(SearchParameterMap.class, theParams);
 		CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESEARCH_REGISTERED, params);
+
 		Class<? extends IBaseResource> resourceTypeClass = myContext.getResourceDefinition(theResourceType).getImplementingClass();
 		final ISearchBuilder sb = mySearchBuilderFactory.newSearchBuilder(theCallingDao, theResourceType, resourceTypeClass);
 		sb.setFetchSize(mySyncSize);
 
 		final Integer loadSynchronousUpTo = getLoadSynchronousUpToOrNull(theCacheControlDirective);
+		boolean isOffsetQuery = theParams.isOffsetQuery();
 
-		if (theParams.isLoadSynchronous() || loadSynchronousUpTo != null) {
+		if (theParams.isLoadSynchronous() || loadSynchronousUpTo != null || isOffsetQuery) {
 			ourLog.debug("Search {} is loading in synchronous mode", searchUuid);
 			return executeQuery(theResourceType, theParams, theRequestDetails, searchUuid, sb, loadSynchronousUpTo, theRequestPartitionId);
 		}
@@ -466,6 +469,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 		// Execute the query and make sure we return distinct results
 		TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager);
 		txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED);
+		txTemplate.setReadOnly(theParams.isLoadSynchronous() || theParams.isOffsetQuery());
 		return txTemplate.execute(t -> {
 
 			// Load the results synchronously
@@ -554,6 +558,10 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 			resources = InterceptorUtil.fireStoragePreshowResource(resources, theRequestDetails, myInterceptorBroadcaster);
 
 			SimpleBundleProvider bundleProvider = new SimpleBundleProvider(resources);
+			if (theParams.isOffsetQuery()) {
+				bundleProvider.setCurrentPageOffset(theParams.getOffset());
+				bundleProvider.setCurrentPageSize(theParams.getCount());
+			}
 
 			if (wantCount) {
 				bundleProvider.setSize(count.intValue());

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/BaseCaptureQueriesListener.java

@@ -22,17 +22,20 @@ package ca.uhn.fhir.jpa.util;
 
 import net.ttddyy.dsproxy.ExecutionInfo;
 import net.ttddyy.dsproxy.QueryInfo;
+import net.ttddyy.dsproxy.listener.MethodExecutionContext;
 import net.ttddyy.dsproxy.proxy.ParameterSetOperation;
 import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
 
+import javax.annotation.Nullable;
 import java.util.Collections;
 import java.util.List;
 import java.util.Queue;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 import static org.apache.commons.lang3.StringUtils.trim;
 
-public abstract class BaseCaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution {
+public abstract class BaseCaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution, ProxyDataSourceBuilder.SingleMethodExecution {
 
 	private boolean myCaptureQueryStackTrace = false;
 
@@ -88,4 +91,34 @@ public abstract class BaseCaptureQueriesListener implements ProxyDataSourceBuild
 
 	protected abstract Queue<SqlQuery> provideQueryList();
 
+	@Nullable
+	protected abstract AtomicInteger provideCommitCounter();
+
+	@Nullable
+	protected abstract AtomicInteger provideRollbackCounter();
+
+	@Override
+	public void execute(MethodExecutionContext executionContext) {
+				AtomicInteger counter = null;
+		switch (executionContext.getMethod().getName()) {
+			case "commit":
+				counter = provideCommitCounter();
+				break;
+			case "rollback":
+				counter = provideRollbackCounter();
+				break;
+		}
+
+		if (counter != null) {
+			counter.incrementAndGet();
+		}
+	}
+
+	public int countCommits() {
+		return provideCommitCounter().get();
+	}
+
+	public int countRollbacks() {
+		return provideRollbackCounter().get();
+	}
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java

@@ -35,6 +35,7 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Queue;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -52,6 +53,8 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe
 	private static final int CAPACITY = 1000;
 	private static final Logger ourLog = LoggerFactory.getLogger(CircularQueueCaptureQueriesListener.class);
 	private Queue<SqlQuery> myQueries;
+	private AtomicInteger myCommitCounter;
+	private AtomicInteger myRollbackCounter;
 
 	/**
 	 * Constructor
@@ -65,11 +68,23 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe
 		return myQueries;
 	}
 
+	@Override
+	protected AtomicInteger provideCommitCounter() {
+		return myCommitCounter;
+	}
+
+	@Override
+	protected AtomicInteger provideRollbackCounter() {
+		return myRollbackCounter;
+	}
+
 	/**
 	 * Clear all stored queries
 	 */
 	public void clear() {
 		myQueries.clear();
+		myCommitCounter.set(0);
+		myRollbackCounter.set(0);
 	}
 
 	/**
@@ -77,6 +92,8 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe
 	 */
 	public void startCollecting() {
 		myQueries = Queues.synchronizedQueue(new CircularFifoQueue<>(CAPACITY));
+		myCommitCounter = new AtomicInteger(0);
+		myRollbackCounter = new AtomicInteger(0);
 	}
 
 	/**
@@ -84,6 +101,8 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe
 	 */
 	public void stopCollecting() {
 		myQueries = null;
+		myCommitCounter = null;
+		myRollbackCounter = null;
 	}
 
 	/**
@@ -115,6 +134,14 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe
 		return getQueriesStartingWith(theStart, null);
 	}
 
+	public int getCommitCount() {
+		return myCommitCounter.get();
+	}
+
+	public int getRollbackCount() {
+		return myRollbackCounter.get();
+	}
+
 	/**
 	 * Returns all SELECT queries executed on the current thread - Index 0 is oldest
 	 */

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CurrentThreadCaptureQueriesListener.java

@@ -27,11 +27,14 @@ import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Queue;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 public class CurrentThreadCaptureQueriesListener extends BaseCaptureQueriesListener {
 
 	private static final ThreadLocal<Queue<SqlQuery>> ourQueues = new ThreadLocal<>();
+	private static final ThreadLocal<AtomicInteger> ourCommits = new ThreadLocal<>();
+	private static final ThreadLocal<AtomicInteger> ourRollbacks = new ThreadLocal<>();
 	private static final Logger ourLog = LoggerFactory.getLogger(CurrentThreadCaptureQueriesListener.class);
 
 	@Override
@@ -39,18 +42,31 @@ public class CurrentThreadCaptureQueriesListener extends BaseCaptureQueriesListe
 		return ourQueues.get();
 	}
 
+	@Override
+	protected AtomicInteger provideCommitCounter() {
+		return ourCommits.get();
+	}
+
+	@Override
+	protected AtomicInteger provideRollbackCounter() {
+		return ourRollbacks.get();
+	}
+
 	/**
 	 * Get the current queue of items and stop collecting
 	 */
 	public static SqlQueryList getCurrentQueueAndStopCapturing() {
 		Queue<SqlQuery> retVal = ourQueues.get();
 		ourQueues.remove();
+		ourCommits.remove();
+		ourRollbacks.remove();
 		if (retVal == null) {
 			return new SqlQueryList();
 		}
 		return new SqlQueryList(retVal);
 	}
 
+
 	/**
 	 * Starts capturing queries for the current thread.
 	 * <p>
@@ -62,6 +78,8 @@ public class CurrentThreadCaptureQueriesListener extends BaseCaptureQueriesListe
 	 */
 	public static void startCapturing() {
 		ourQueues.set(new ArrayDeque<>());
+		ourCommits.set(new AtomicInteger(0));
+		ourRollbacks.set(new AtomicInteger(0));
 	}
 
 	/**

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java

@@ -127,6 +127,7 @@ public class TestR4Config extends BaseJavaConfigR4 {
 			.afterQuery(captureQueriesListener())
 			.afterQuery(new CurrentThreadCaptureQueriesListener())
 			.countQuery(singleQueryCountHolder())
+			.afterMethod(captureQueriesListener())
 			.build();
 
 		return dataSource;

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java

@@ -5,11 +5,13 @@ import ca.uhn.fhir.jpa.api.model.HistoryCountModeEnum;
 import ca.uhn.fhir.jpa.model.entity.ModelConfig;
 import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.partition.SystemRequestDetails;
+import ca.uhn.fhir.jpa.provider.r4.BaseResourceProviderR4Test;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.util.SqlQuery;
 import ca.uhn.fhir.rest.api.SortSpec;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.param.ReferenceParam;
+import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.util.BundleBuilder;
 import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IIdType;
@@ -46,10 +48,11 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 
-public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
+public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test {
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4QueryCountTest.class);
 
 	@AfterEach
@@ -66,8 +69,10 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 		myDaoConfig.setTagStorageMode(new DaoConfig().getTagStorageMode());
 	}
 
+	@Override
 	@BeforeEach
-	public void before() {
+	public void before() throws Exception {
+		super.before();
 		myInterceptorRegistry.registerInterceptor(myInterceptor);
 	}
 
@@ -534,6 +539,92 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 	}
 
 
+	@Test
+	public void testSearchUsingOffsetMode_Explicit() {
+		for (int i = 0; i < 10; i++) {
+			createPatient(withId("A" + i), withActiveTrue());
+		}
+
+		SearchParameterMap map = new SearchParameterMap();
+		map.setLoadSynchronousUpTo(5);
+		map.setOffset(0);
+		map.add("active", new TokenParam("true"));
+
+		// First page
+		myCaptureQueriesListener.clear();
+		Bundle outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.offset(0)
+			.count(5)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), containsInAnyOrder(
+			"Patient/A0", "Patient/A1", "Patient/A2", "Patient/A3", "Patient/A4"
+		));
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(2, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertThat(outcome.getLink("next").getUrl(), containsString("Patient?_count=5&_offset=5&active=true"));
+
+		// Second page
+		myCaptureQueriesListener.clear();
+		outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.offset(5)
+			.count(5)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), containsInAnyOrder(
+			"Patient/A5", "Patient/A6", "Patient/A7", "Patient/A8", "Patient/A9"
+		));
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(2, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("offset '5'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertThat(outcome.getLink("next").getUrl(), containsString("Patient?_count=5&_offset=10&active=true"));
+
+		// Third page (no results)
+
+		myCaptureQueriesListener.clear();
+		outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.offset(10)
+			.count(5)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), empty());
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(1, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("offset '10'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+
+	}
+
+
 	@Test
 	public void testSearchUsingForcedIdReference() {
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/ForceOffsetSearchModeInterceptorTest.java

@@ -0,0 +1,158 @@
+package ca.uhn.fhir.jpa.interceptor;
+
+import ca.uhn.fhir.jpa.provider.r4.BaseResourceProviderR4Test;
+import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.Patient;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.empty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class ForceOffsetSearchModeInterceptorTest extends BaseResourceProviderR4Test {
+
+	private ForceOffsetSearchModeInterceptor mySvc;
+	private Integer myInitialDefaultPageSize;
+
+	@Override
+	@BeforeEach
+	public void before() throws Exception {
+		super.before();
+
+		mySvc = new ForceOffsetSearchModeInterceptor();
+		ourRestServer.registerInterceptor(mySvc);
+		myInitialDefaultPageSize = ourRestServer.getDefaultPageSize();
+	}
+
+	@Override
+	@AfterEach
+	public void after() throws Exception {
+		super.after();
+
+		ourRestServer.unregisterInterceptor(mySvc);
+		ourRestServer.setDefaultPageSize(myInitialDefaultPageSize);
+	}
+
+	@Test
+	public void testSearch_NoExplcitCount() {
+		ourRestServer.setDefaultPageSize(5);
+
+		for (int i = 0; i < 10; i++) {
+			createPatient(withId("A" + i), withActiveTrue());
+		}
+
+		// First page
+		myCaptureQueriesListener.clear();
+		Bundle outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), containsInAnyOrder(
+			"Patient/A0", "Patient/A1", "Patient/A2", "Patient/A3", "Patient/A4"
+		));
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(2, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertThat(outcome.getLink("next").getUrl(), containsString("Patient?_count=5&_offset=5&active=true"));
+
+		// Second page
+		myCaptureQueriesListener.clear();
+		outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.offset(5)
+			.count(5)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), containsInAnyOrder(
+			"Patient/A5", "Patient/A6", "Patient/A7", "Patient/A8", "Patient/A9"
+		));
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(2, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("offset '5'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertThat(outcome.getLink("next").getUrl(), containsString("Patient?_count=5&_offset=10&active=true"));
+
+		// Third page (no results)
+
+		myCaptureQueriesListener.clear();
+		Bundle outcome3 = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.offset(10)
+			.count(5)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome3).toString(), toUnqualifiedVersionlessIdValues(outcome3), empty());
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(1, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '5'"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("offset '10'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+
+		assertNull(outcome3.getLink("next"), () -> outcome3.getLink("next").getUrl());
+
+	}
+
+
+	@Test
+	public void testSearch_WithExplicitCount() {
+		ourRestServer.setDefaultPageSize(5);
+
+		for (int i = 0; i < 10; i++) {
+			createPatient(withId("A" + i), withActiveTrue());
+		}
+
+		// First page
+		myCaptureQueriesListener.clear();
+		Bundle outcome = myClient
+			.search()
+			.forResource("Patient")
+			.where(Patient.ACTIVE.exactly().code("true"))
+			.count(7)
+			.returnBundle(Bundle.class)
+			.execute();
+		assertThat(toUnqualifiedVersionlessIdValues(outcome).toString(), toUnqualifiedVersionlessIdValues(outcome), containsInAnyOrder(
+			"Patient/A0", "Patient/A1", "Patient/A2", "Patient/A3", "Patient/A4", "Patient/A5", "Patient/A6"
+		));
+		myCaptureQueriesListener.logSelectQueries();
+		assertEquals(2, myCaptureQueriesListener.countSelectQueries());
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("SELECT t0.RES_ID FROM HFJ_SPIDX_TOKEN t0"));
+		assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false), containsString("limit '7'"));
+		assertEquals(0, myCaptureQueriesListener.countInsertQueries());
+		assertEquals(0, myCaptureQueriesListener.countUpdateQueries());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertThat(outcome.getLink("next").getUrl(), containsString("Patient?_count=7&_offset=7&active=true"));
+	}
+
+
+}

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java

@@ -631,6 +631,15 @@ public class SearchParameterMap implements Serializable {
 		}
 	}
 
+	/**
+	 * Returns true if {@link #getOffset()} and {@link #getCount()} both return a non null response
+	 *
+	 * @since 5.5.0
+	 */
+	public boolean isOffsetQuery() {
+		return getOffset() != null && getCount() != null;
+	}
+
 	public enum EverythingModeEnum {
 		/*
 		 * Don't reorder! We rely on the ordinals

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java

@@ -88,6 +88,25 @@ public interface IBundleProvider {
 		return null;
 	}
 
+	/**
+	 * If the results in this bundle were produced using an offset query (as opposed to a query using
+	 * continuation pointers, page IDs, etc.) the page offset can be returned here. The server
+	 * should then attempt to form paging links that use <code>_offset</code> instead of
+	 * opaque page IDs.
+	 */
+	default Integer getCurrentPageOffset() {
+		return null;
+	}
+
+	/**
+	 * If {@link #getCurrentPageOffset()} returns a non-null value, this method must also return
+	 * the actual page size used
+	 */
+	default Integer getCurrentPageSize() {
+		return null;
+	}
+
+
 	/**
 	 * Returns the instant as of which this result was created. The
 	 * result of this value is used to populate the <code>lastUpdated</code>
@@ -194,4 +213,5 @@ public interface IBundleProvider {
 		Validate.notNull(retVal, "size() returned null");
 		return retVal;
 	}
+
 }

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/SimpleBundleProvider.java

@@ -37,6 +37,8 @@ public class SimpleBundleProvider implements IBundleProvider {
 	private Integer myPreferredPageSize;
 	private Integer mySize;
 	private IPrimitiveType<Date> myPublished = InstantDt.withCurrentTime();
+	private Integer myCurrentPageOffset;
+	private Integer myCurrentPageSize;
 
 	/**
 	 * Constructor
@@ -74,6 +76,36 @@ public class SimpleBundleProvider implements IBundleProvider {
 		setSize(theSize);
 	}
 
+	/**
+	 * @since 5.5.0
+	 */
+	@Override
+	public Integer getCurrentPageOffset() {
+		return myCurrentPageOffset;
+	}
+
+	/**
+	 * @since 5.5.0
+	 */
+	public void setCurrentPageOffset(Integer theCurrentPageOffset) {
+		myCurrentPageOffset = theCurrentPageOffset;
+	}
+
+	/**
+	 * @since 5.5.0
+	 */
+	@Override
+	public Integer getCurrentPageSize() {
+		return myCurrentPageSize;
+	}
+
+	/**
+	 * @since 5.5.0
+	 */
+	public void setCurrentPageSize(Integer theCurrentPageSize) {
+		myCurrentPageSize = theCurrentPageSize;
+	}
+
 	/**
 	 * Returns the results stored in this provider
 	 */

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java

@@ -47,6 +47,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 
+import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
@@ -141,7 +142,16 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
 	IBaseResource createBundleFromBundleProvider(IRestfulServer<?> theServer, RequestDetails theRequest, Integer theLimit, String theLinkSelf, Set<Include> theIncludes,
 																IBundleProvider theResult, int theOffset, BundleTypeEnum theBundleType, EncodingEnum theLinkEncoding, String theSearchId) {
 		IVersionSpecificBundleFactory bundleFactory = theServer.getFhirContext().newBundleFactory();
-		final Integer requestOffset = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_OFFSET);
+		final Integer offset;
+		Integer limit = theLimit;
+
+		if (theResult.getCurrentPageOffset() != null) {
+			offset = theResult.getCurrentPageOffset();
+			limit = theResult.getCurrentPageSize();
+			Validate.notNull(limit, "IBundleProvider returned a non-null offset, but did not return a non-null page size");
+		} else {
+			offset = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_OFFSET);
+		}
 
 		int numToReturn;
 		String searchId = null;
@@ -149,9 +159,9 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
 		Integer numTotalResults = theResult.size();
 
 		int pageSize;
-		if (requestOffset != null || !theServer.canStoreSearchResults()) {
+		if (offset != null || !theServer.canStoreSearchResults()) {
-			if (theLimit != null) {
+			if (limit != null) {
-				pageSize = theLimit;
+				pageSize = limit;
 			} else {
 				if (theServer.getDefaultPageSize() != null) {
 					pageSize = theServer.getDefaultPageSize();
@@ -161,7 +171,7 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
 			}
 			numToReturn = pageSize;
 
-			if (requestOffset != null && !isOffsetModeHistory()) {
+			if ((offset != null && !isOffsetModeHistory()) || theResult.getCurrentPageOffset() != null) {
 				// When offset query is done theResult already contains correct amount (+ their includes etc.) so return everything
 				resourceList = theResult.getResources(0, Integer.MAX_VALUE);
 			} else if (numToReturn > 0) {
@@ -173,10 +183,10 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
 
 		} else {
 			IPagingProvider pagingProvider = theServer.getPagingProvider();
-			if (theLimit == null || theLimit.equals(0)) {
+			if (limit == null || ((Integer) limit).equals(0)) {
 				pageSize = pagingProvider.getDefaultPageSize();
 			} else {
-				pageSize = Math.min(pagingProvider.getMaximumPageSize(), theLimit);
+				pageSize = Math.min(pagingProvider.getMaximumPageSize(), limit);
 			}
 			numToReturn = pageSize;
 
@@ -238,19 +248,31 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
 		BundleLinks links = new BundleLinks(theRequest.getFhirServerBase(), theIncludes, RestfulServerUtils.prettyPrintResponse(theServer, theRequest), theBundleType);
 		links.setSelf(theLinkSelf);
 
-		if (requestOffset != null || (!theServer.canStoreSearchResults() && !isEverythingOperation(theRequest)) || isOffsetModeHistory()) {
+		if (theResult.getCurrentPageOffset() != null) {
-			int offset = requestOffset != null ? requestOffset : 0;
+
+			if (isNotBlank(theResult.getNextPageId())) {
+				links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), offset + limit, limit, theRequest.getParameters()));
+			}
+			if (isNotBlank(theResult.getPreviousPageId())) {
+				links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), Math.max(offset - limit, 0), limit, theRequest.getParameters()));
+			}
+
+		}
+
+		if (offset != null || (!theServer.canStoreSearchResults() && !isEverythingOperation(theRequest)) || isOffsetModeHistory()) {
 			// Paging without caching
-			// We're doing requestOffset pages
+			// We're doing offset pages
 			int requestedToReturn = numToReturn;
-			if (theServer.getPagingProvider() == null) {
+			if (theServer.getPagingProvider() == null && offset != null) {
 				// There is no paging provider at all, so assume we're querying up to all the results we need every time
 				requestedToReturn += offset;
 			}
 			if (numTotalResults == null || requestedToReturn < numTotalResults) {
-				links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), offset + numToReturn, numToReturn, theRequest.getParameters()));
+				if (!resourceList.isEmpty()) {
+					links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), defaultIfNull(offset, 0) + numToReturn, numToReturn, theRequest.getParameters()));
 				}
-			if (offset > 0) {
+			}
+			if (offset != null && offset > 0) {
 				int start = Math.max(0, theOffset - pageSize);
 				links.setPrev(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), start, pageSize, theRequest.getParameters()));
 			}

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchBundleProviderWithNoSizeDstu2Test.java

@@ -57,88 +57,6 @@ public class SearchBundleProviderWithNoSizeDstu2Test {
 		ourIdentifiers = null;
 	}
 
-	@Test
-	public void testBundleProviderReturnsNoSize() throws Exception {
-		Bundle respBundle;
-		
-		ourLastBundleProvider = mock(IBundleProvider.class); 
-		when(ourLastBundleProvider.size()).thenReturn(null);
-		when(ourLastBundleProvider.getResources(any(int.class), any(int.class))).then(new Answer<List<IBaseResource>>() {
-			@Override
-			public List<IBaseResource> answer(InvocationOnMock theInvocation) throws Throwable {
-				int from =(Integer)theInvocation.getArguments()[0]; 
-				int to =(Integer)theInvocation.getArguments()[1];
-				ArrayList<IBaseResource> retVal = Lists.newArrayList();
-				for (int i = from; i < to; i++) {
-					Patient p = new Patient();
-					p.setId(Integer.toString(i));
-					retVal.add(p);
-				}
-				return retVal;
-			}});
-		
-		HttpGet httpGet;
-		CloseableHttpResponse status = null;
-		Link linkNext;
-		
-		try {
-			httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=json");
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(10, respBundle.getEntry().size());
-			assertEquals("Patient/0", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNotNull(linkNext);
-			
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-		
-		when(ourLastBundleProvider.size()).thenReturn(25);
-
-		try {
-			httpGet = new HttpGet(linkNext.getUrl());
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(10, respBundle.getEntry().size());
-			assertEquals("Patient/10", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNotNull(linkNext);
-
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-		try {
-			httpGet = new HttpGet(linkNext.getUrl());
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(5, respBundle.getEntry().size());
-			assertEquals("Patient/20", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNull(linkNext);
-
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-	}
 
 	@AfterAll
 	public static void afterClassClearContext() throws Exception {

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchBundleProviderWithNoSizeDstu3Test.java

@@ -57,89 +57,6 @@ public class SearchBundleProviderWithNoSizeDstu3Test {
 		ourIdentifiers = null;
 	}
 
-	@Test
-	public void testBundleProviderReturnsNoSize() throws Exception {
-		Bundle respBundle;
-		
-		ourLastBundleProvider = mock(IBundleProvider.class); 
-		when(ourLastBundleProvider.size()).thenReturn(null);
-		when(ourLastBundleProvider.getResources(any(int.class), any(int.class))).then(new Answer<List<IBaseResource>>() {
-			@Override
-			public List<IBaseResource> answer(InvocationOnMock theInvocation) throws Throwable {
-				int from =(Integer)theInvocation.getArguments()[0]; 
-				int to =(Integer)theInvocation.getArguments()[1];
-				ArrayList<IBaseResource> retVal = Lists.newArrayList();
-				for (int i = from; i < to; i++) {
-					Patient p = new Patient();
-					p.setId(Integer.toString(i));
-					retVal.add(p);
-				}
-				return retVal;
-			}});
-		
-		HttpGet httpGet;
-		CloseableHttpResponse status = null;
-		BundleLinkComponent linkNext;
-		
-		try {
-			httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=json");
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(10, respBundle.getEntry().size());
-			assertEquals("Patient/0", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNotNull(linkNext);
-			
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-		
-		when(ourLastBundleProvider.size()).thenReturn(25);
-
-		try {
-			httpGet = new HttpGet(linkNext.getUrl());
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(10, respBundle.getEntry().size());
-			assertEquals("Patient/10", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNotNull(linkNext);
-
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-		try {
-			httpGet = new HttpGet(linkNext.getUrl());
-			status = ourClient.execute(httpGet);
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			assertEquals("searchAll", ourLastMethod);
-			respBundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
-			
-			assertEquals(5, respBundle.getEntry().size());
-			assertEquals("Patient/20", respBundle.getEntry().get(0).getResource().getIdElement().toUnqualifiedVersionless().getValue());
-			linkNext = respBundle.getLink("next");
-			assertNull(linkNext);
-
-		} finally {
-			IOUtils.closeQuietly(status.getEntity().getContent());
-		}
-
-	}
-
 	@AfterAll
 	public static void afterClassClearContext() throws Exception {
 		JettyUtil.closeServer(ourServer);

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchBundleProviderWithNoSizeR4Test.java

@@ -62,6 +62,7 @@ public class SearchBundleProviderWithNoSizeR4Test {
 		Bundle respBundle;
 		
 		ourLastBundleProvider = mock(IBundleProvider.class);
+		when(ourLastBundleProvider.getCurrentPageOffset()).thenReturn(null);
 		when(ourLastBundleProvider.size()).thenReturn(null);
 		when(ourLastBundleProvider.getResources(any(int.class), any(int.class))).then(new Answer<List<IBaseResource>>() {
 			@Override