5192 include in search empty page results (#5211)

Fixing paging with _include query parameter

hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/PagingPatientProvider.java

@@ -24,6 +24,7 @@ import ca.uhn.fhir.rest.annotation.Search;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.param.StringParam;
 import ca.uhn.fhir.rest.server.IResourceProvider;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.InstantType;
 import org.hl7.fhir.r4.model.Patient;
@@ -63,7 +64,10 @@ public class PagingPatientProvider implements IResourceProvider {
 
 			@Nonnull
 			@Override
-			public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+			public List<IBaseResource> getResources(
+					int theFromIndex,
+					int theToIndex,
+					@Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 				int end = Math.max(theToIndex, matchingResourceIds.size() - 1);
 				List<Long> idsToReturn = matchingResourceIds.subList(theFromIndex, end);
 				return loadResourcesByIds(idsToReturn);

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5192-include-in-search-paging-fix.yaml

@@ -0,0 +1,9 @@
+---
+type: fix
+issue: 5192
+title: "Fixed a bug where search Bundles with `include` entries from an _include query parameter might
+      trigger a 'next' link to blank pages.
+      Specifically, if _include'd resources + requested resources were greater than (or equal to)
+      requested page size, a 'next' link would be generated, even though no additional
+      resources are available.
+"

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

@@ -55,6 +55,7 @@ import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.api.server.SimplePreResourceAccessDetails;
 import ca.uhn.fhir.rest.api.server.SimplePreResourceShowDetails;
 import ca.uhn.fhir.rest.server.interceptor.ServerInterceptorUtil;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster;
 import com.google.common.annotations.VisibleForTesting;
@@ -128,6 +129,7 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 	private String myUuid;
 	private SearchCacheStatusEnum myCacheStatus;
 	private RequestPartitionId myRequestPartitionId;
+
 	/**
 	 * Constructor
 	 */
@@ -180,7 +182,6 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 			BaseHasResource resource;
 			resource = next;
 
-			IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(next.getResourceType());
 			retVal.add(myJpaStorageResourceParser.toResource(resource, true));
 		}
 
@@ -238,7 +239,10 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 		myRequestPartitionId = theRequestPartitionId;
 	}
 
-	protected List<IBaseResource> doSearchOrEverything(final int theFromIndex, final int theToIndex) {
+	protected List<IBaseResource> doSearchOrEverything(
+			final int theFromIndex,
+			final int theToIndex,
+			@Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		if (mySearchEntity.getTotalCount() != null && mySearchEntity.getNumFound() <= 0) {
 			// No resources to fetch (e.g. we did a _summary=count search)
 			return Collections.emptyList();
@@ -253,12 +257,14 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 		RequestPartitionId requestPartitionId = getRequestPartitionId();
 		final List<JpaPid> pidsSubList =
 				mySearchCoordinatorSvc.getResources(myUuid, theFromIndex, theToIndex, myRequest, requestPartitionId);
-		return myTxService
+		List<IBaseResource> resources = myTxService
 				.withRequest(myRequest)
 				.withRequestPartitionId(requestPartitionId)
 				.execute(() -> {
-					return toResourceList(sb, pidsSubList);
+					return toResourceList(sb, pidsSubList, theResponsePageBuilder);
 				});
+
+		return resources;
 	}
 
 	/**
@@ -351,7 +357,13 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 
 	@Nonnull
 	@Override
-	public List<IBaseResource> getResources(final int theFromIndex, final int theToIndex) {
+	public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+		return getResources(theFromIndex, theToIndex, new ResponsePage.ResponsePageBuilder());
+	}
+
+	@Override
+	public List<IBaseResource> getResources(
+			int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		boolean entityLoaded = ensureSearchEntityLoaded();
 		assert entityLoaded;
 		assert mySearchEntity != null;
@@ -366,7 +378,7 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 			case SEARCH:
 			case EVERYTHING:
 			default:
-				List<IBaseResource> retVal = doSearchOrEverything(theFromIndex, theToIndex);
+				List<IBaseResource> retVal = doSearchOrEverything(theFromIndex, theToIndex, theResponsePageBuilder);
 				/*
 				 * If we got fewer resources back than we asked for, it's possible that the search
 				 * completed. If that's the case, the cached version of the search entity is probably
@@ -443,8 +455,10 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 
 	// Note: Leave as protected, HSPC depends on this
 	@SuppressWarnings("WeakerAccess")
-	protected List<IBaseResource> toResourceList(ISearchBuilder theSearchBuilder, List<JpaPid> thePids) {
+	protected List<IBaseResource> toResourceList(
-
+			ISearchBuilder theSearchBuilder,
+			List<JpaPid> thePids,
+			ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		List<JpaPid> includedPidList = new ArrayList<>();
 		if (mySearchEntity.getSearchType() == SearchTypeEnum.SEARCH) {
 			Integer maxIncludes = myStorageSettings.getMaximumIncludesToLoadPerPage();
@@ -521,7 +535,14 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 		List<IBaseResource> resources = new ArrayList<>();
 		theSearchBuilder.loadResourcesByPid(thePids, includedPidList, resources, false, myRequest);
 
+		// we will send the resource list to our interceptors
+		// this can (potentially) change the results being returned.
+		int precount = resources.size();
 		resources = ServerInterceptorUtil.fireStoragePreshowResource(resources, myRequest, myInterceptorBroadcaster);
+		// we only care about omitted results from *this* page
+		theResponsePageBuilder.setToOmittedResourceCount(precount - resources.size());
+		theResponsePageBuilder.setResources(resources);
+		theResponsePageBuilder.setIncludedResourceCount(includedPidList.size());
 
 		return resources;
 	}

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

@@ -30,6 +30,7 @@ import ca.uhn.fhir.jpa.util.QueryParameterUtils;
 import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
 import ca.uhn.fhir.model.valueset.BundleEntrySearchModeEnum;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -42,11 +43,14 @@ import javax.annotation.Nonnull;
 public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundleProvider {
 	private static final Logger ourLog = LoggerFactory.getLogger(PersistedJpaSearchFirstPageBundleProvider.class);
 	private final SearchTask mySearchTask;
+
+	@SuppressWarnings("rawtypes")
 	private final ISearchBuilder mySearchBuilder;
 
 	/**
 	 * Constructor
 	 */
+	@SuppressWarnings("rawtypes")
 	public PersistedJpaSearchFirstPageBundleProvider(
 			Search theSearch,
 			SearchTask theSearchTask,
@@ -65,7 +69,8 @@ public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundl
 
 	@Nonnull
 	@Override
-	public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+	public List<IBaseResource> getResources(
+			int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder thePageBuilder) {
 		ensureSearchEntityLoaded();
 		QueryParameterUtils.verifySearchHasntFailedOrThrowInternalErrorException(getSearchEntity());
 
@@ -80,7 +85,7 @@ public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundl
 		List<IBaseResource> retVal = myTxService
 				.withRequest(myRequest)
 				.withRequestPartitionId(requestPartitionId)
-				.execute(() -> toResourceList(mySearchBuilder, pids));
+				.execute(() -> toResourceList(mySearchBuilder, pids, thePageBuilder));
 
 		long totalCountWanted = theToIndex - theFromIndex;
 		long totalCountMatch = (int) retVal.stream().filter(t -> !isInclude(t)).count();
@@ -101,7 +106,7 @@ public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundl
 
 				long remainingWanted = totalCountWanted - totalCountMatch;
 				long fromIndex = theToIndex - remainingWanted;
-				List<IBaseResource> remaining = super.getResources((int) fromIndex, theToIndex);
+				List<IBaseResource> remaining = super.getResources((int) fromIndex, theToIndex, thePageBuilder);
 				remaining.forEach(t -> {
 					if (!existingIds.contains(t.getIdElement().getValue())) {
 						retVal.add(t);

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

@@ -7,8 +7,6 @@ import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.EncodingEnum;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.server.BasePagingProvider;
-import ca.uhn.fhir.rest.server.IPagingProvider;
-import ca.uhn.fhir.rest.server.RestfulServer;
 import ca.uhn.fhir.util.BundleUtil;
 import com.google.common.base.Charsets;
 import org.apache.commons.io.IOUtils;

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

@@ -146,6 +146,7 @@ import org.hl7.fhir.r4.model.StructureDefinition;
 import org.hl7.fhir.r4.model.Subscription;
 import org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType;
 import org.hl7.fhir.r4.model.Subscription.SubscriptionStatus;
+import org.hl7.fhir.r4.model.Task;
 import org.hl7.fhir.r4.model.UriType;
 import org.hl7.fhir.r4.model.ValueSet;
 import org.hl7.fhir.utilities.xhtml.NodeType;
@@ -594,7 +595,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 	@Test
 	public void testSearchLinksWorkWithIncludes() {
 		for (int i = 0; i < 5; i++) {
-
 			Organization o = new Organization();
 			o.setId("O" + i);
 			o.setName("O" + i);
@@ -604,7 +604,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 			p.setId("P" + i);
 			p.getManagingOrganization().setReference(oid.getValue());
 			myClient.update().resource(p).execute();
-
 		}
 
 		Bundle output = myClient
@@ -2651,9 +2650,138 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		int newSize = client.search().forResource(ImagingStudy.class).returnBundle(Bundle.class).execute().getEntry().size();
 
 		assertEquals(1, newSize - initialSize);
-
 	}
 
+	@Test
+	public void testPagingWithIncludesOnEachResource() {
+		// setup
+		int total = 20;
+		Organization org = new Organization();
+		org.setName("ORG");
+		IIdType orgId = myOrganizationDao.create(org).getId().toUnqualifiedVersionless();
+
+		Coding tagCode = new Coding();
+		tagCode.setCode("test");
+		tagCode.setSystem("http://example.com");
+		for (int i = 0; i < total; i++) {
+			Task t = new Task();
+			t.getMeta()
+				.addTag(tagCode);
+			t.setStatus(Task.TaskStatus.REQUESTED);
+			t.getOwner().setReference(orgId.getValue());
+			myTaskDao.create(t);
+		}
+		HashSet<String> ids = new HashSet<>();
+
+		// test
+		int requestedAmount = 10;
+		Bundle bundle = myClient
+			.search()
+			.byUrl("Task?_count=10&_tag=test&status=requested&_include=Task%3Aowner&_sort=status")
+			.returnBundle(Bundle.class)
+			.execute();
+		assertFalse(bundle.getEntry().isEmpty());
+		assertEquals(11, bundle.getEntry().size());
+		for (BundleEntryComponent resource : bundle.getEntry()) {
+			ids.add(resource.getResource().getId());
+		}
+
+		String nextUrl = null;
+		do {
+			Bundle.BundleLinkComponent nextLink = bundle.getLink("next");
+			if (nextLink != null) {
+				nextUrl = nextLink.getUrl();
+
+				// make sure we're always requesting 10
+				assertTrue(nextUrl.contains(String.format("_count=%d", requestedAmount)));
+
+				// get next batch
+				bundle = myClient.fetchResourceFromUrl(Bundle.class, nextUrl);
+				int received = bundle.getEntry().size();
+
+				// currently, last page could be empty... so we'll
+				// short circuit out here
+				if (received != 0) {
+					// every batch should include the 10 tasks + 1 orgranization
+					assertEquals(11, received);
+					for (BundleEntryComponent resource : bundle.getEntry()) {
+						ids.add(resource.getResource().getId());
+					}
+				}
+			} else {
+				nextUrl = null;
+			}
+		} while (nextUrl != null);
+
+		// verify
+		// we should receive all resources and the single organization (repeatedly)
+		assertEquals(total + 1, ids.size());
+	}
+
+	@Test
+	public void testPagingWithIncludesReturnsConsistentValues() {
+		// setup
+		int total = 19;
+		int orgs = 10;
+		// create resources
+		{
+			Coding tagCode = new Coding();
+			tagCode.setCode("test");
+			tagCode.setSystem("http://example.com");
+			int orgCount = orgs;
+			for (int i = 0; i < total; i++) {
+				Task t = new Task();
+				t.getMeta()
+					.addTag(tagCode);
+				t.setStatus(Task.TaskStatus.REQUESTED);
+				if (orgCount > 0) {
+					Organization org = new Organization();
+					org.setName("ORG");
+					IIdType orgId = myOrganizationDao.create(org).getId().toUnqualifiedVersionless();
+
+					orgCount--;
+					t.getOwner().setReference(orgId.getValue());
+				}
+				myTaskDao.create(t);
+			}
+		}
+
+		int requestedAmount = 10;
+		Bundle bundle = myClient
+			.search()
+			.byUrl("Task?_count=10&_tag=test&status=requested&_include=Task%3Aowner&_sort=status")
+			.returnBundle(Bundle.class)
+			.execute();
+		int count = bundle.getEntry().size();
+		assertFalse(bundle.getEntry().isEmpty());
+
+		String nextUrl = null;
+		do {
+			Bundle.BundleLinkComponent nextLink = bundle.getLink("next");
+			if (nextLink != null) {
+				nextUrl = nextLink.getUrl();
+
+				// make sure we're always requesting 10
+				assertTrue(nextUrl.contains(String.format("_count=%d", requestedAmount)));
+
+				// get next batch
+				bundle = myClient.fetchResourceFromUrl(Bundle.class, nextUrl);
+				int received = bundle.getEntry().size();
+
+				// every next result should produce results
+				assertFalse(bundle.getEntry().isEmpty());
+				count += received;
+			} else {
+				nextUrl = null;
+			}
+		} while (nextUrl != null);
+
+		// verify
+		// we should receive all resources and linked resources
+		assertEquals(total + orgs, count);
+	}
+
+
 	/**
 	 * See #793
 	 */

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProviderTest.java

@@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.api.dao.IDao;
 import ca.uhn.fhir.jpa.dao.SearchBuilderFactory;
 import ca.uhn.fhir.jpa.entity.Search;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -32,7 +33,7 @@ public class PersistedJpaBundleProviderTest {
 		Search searchEntity = new Search();
 		searchEntity.setTotalCount(1);
 		myPersistedJpaBundleProvider.setSearchEntity(searchEntity);
-		myPersistedJpaBundleProvider.doSearchOrEverything(0, 1);
+		myPersistedJpaBundleProvider.doSearchOrEverything(0, 1, new ResponsePage.ResponsePageBuilder());
 		verifyNoInteractions(myDao);
 		verifyNoInteractions(mySearchBuilderFactory);
 	}

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

@@ -21,6 +21,7 @@ package ca.uhn.fhir.rest.api.server;
 
 import ca.uhn.fhir.context.ConfigurationException;
 import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.apache.commons.lang3.Validate;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
@@ -125,13 +126,41 @@ public interface IBundleProvider {
 	 * previous page, then the indexes should be ignored and the
 	 * whole page returned.
 	 * </p>
+	 * Note that this implementation should not be used if accurate paging is required,
+	 * as page calculation depends on _include'd resource counts.
+	 * For accurate paging, use {@link IBundleProvider#getResources(int, int, ResponsePage.ResponsePageBuilder)}
 	 *
 	 * @param theFromIndex The low index (inclusive) to return
 	 * @param theToIndex   The high index (exclusive) to return
 	 * @return A list of resources. The size of this list must be at least <code>theToIndex - theFromIndex</code>.
 	 */
 	@Nonnull
-	List<IBaseResource> getResources(int theFromIndex, int theToIndex);
+	default List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+		return getResources(theFromIndex, theToIndex, new ResponsePage.ResponsePageBuilder());
+	}
+
+	/**
+	 * Load the given collection of resources by index, plus any additional resources per the
+	 * server's processing rules (e.g. _include'd resources, OperationOutcome, etc.). For example,
+	 * if the method is invoked with index 0,10 the method might return 10 search results, plus an
+	 * additional 20 resources which matched a client's _include specification.
+	 * <p>
+	 * Note that if this bundle provider was loaded using a
+	 * page ID (i.e. via {@link ca.uhn.fhir.rest.server.IPagingProvider#retrieveResultList(RequestDetails, String, String)}
+	 * because {@link #getNextPageId()} provided a value on the
+	 * previous page, then the indexes should be ignored and the
+	 * whole page returned.
+	 * </p>
+	 *
+	 * @param theFromIndex           The low index (inclusive) to return
+	 * @param theToIndex             The high index (exclusive) to return
+	 * @param theResponsePageBuilder The ResponsePageBuilder. The builder will add values needed for the response page.
+	 * @return A list of resources. The size of this list must be at least <code>theToIndex - theFromIndex</code>.
+	 */
+	default List<IBaseResource> getResources(
+			int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
+		return getResources(theFromIndex, theToIndex);
+	}
 
 	/**
 	 * Get all resources

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

@@ -19,6 +19,7 @@
  */
 package ca.uhn.fhir.rest.server;
 
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.apache.commons.lang3.Validate;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 
@@ -85,9 +86,11 @@ public class BundleProviderWithNamedPages extends SimpleBundleProvider {
 		return this;
 	}
 
+	@SuppressWarnings("unchecked")
 	@Nonnull
 	@Override
-	public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+	public List<IBaseResource> getResources(
+			int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		return (List<IBaseResource>) getList(); // indexes are ignored for this provider type
 	}
 

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

@@ -21,6 +21,7 @@ package ca.uhn.fhir.rest.server;
 
 import ca.uhn.fhir.model.primitive.InstantDt;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import ca.uhn.fhir.util.CoverageIgnore;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 
@@ -47,7 +48,8 @@ public class BundleProviders {
 		return new IBundleProvider() {
 			@Nonnull
 			@Override
-			public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+			public List<IBaseResource> getResources(
+					int theFromIndex, int theToIndex, ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 				return Collections.emptyList();
 			}
 

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

@@ -21,6 +21,7 @@ package ca.uhn.fhir.rest.server;
 
 import ca.uhn.fhir.model.primitive.InstantDt;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
@@ -40,6 +41,7 @@ public class SimpleBundleProvider implements IBundleProvider {
 	private IPrimitiveType<Date> myPublished = InstantDt.withCurrentTime();
 	private Integer myCurrentPageOffset;
 	private Integer myCurrentPageSize;
+	private ResponsePage.ResponsePageBuilder myPageBuilder;
 
 	/**
 	 * Constructor
@@ -137,9 +139,11 @@ public class SimpleBundleProvider implements IBundleProvider {
 		myPublished = thePublished;
 	}
 
+	@SuppressWarnings("unchecked")
 	@Nonnull
 	@Override
-	public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+	public List<IBaseResource> getResources(
+			int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		return (List<IBaseResource>)
 				myList.subList(Math.min(theFromIndex, myList.size()), Math.min(theToIndex, myList.size()));
 	}

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

@@ -181,8 +181,9 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding {
 
 			@Nonnull
 			@Override
-			public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+			public List<IBaseResource> getResources(
-				List<IBaseResource> retVal = resources.getResources(theFromIndex, theToIndex);
+					int theFromIndex, int theToIndex, ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
+				List<IBaseResource> retVal = resources.getResources(theFromIndex, theToIndex, theResponsePageBuilder);
 				int index = theFromIndex;
 				for (IBaseResource nextResource : retVal) {
 					if (nextResource.getIdElement() == null

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

@@ -29,7 +29,6 @@ import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.server.IPagingProvider;
 import ca.uhn.fhir.rest.server.RestfulServerUtils;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
-import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IBaseBundle;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
@@ -57,8 +56,8 @@ public class ResponseBundleBuilder {
 	IBaseBundle buildResponseBundle(ResponseBundleRequest theResponseBundleRequest) {
 		final ResponsePage responsePage = buildResponsePage(theResponseBundleRequest);
 
-		removeNulls(responsePage.resourceList);
+		removeNulls(responsePage.getResourceList());
-		validateIds(responsePage.resourceList);
+		validateIds(responsePage.getResourceList());
 
 		BundleLinks links = buildLinks(theResponseBundleRequest, responsePage);
 
@@ -75,7 +74,7 @@ public class ResponseBundleBuilder {
 		bundleFactory.addRootPropertiesToBundle(
 				bundleProvider.getUuid(), links, bundleProvider.size(), bundleProvider.getPublished());
 		bundleFactory.addResourcesToBundle(
-				new ArrayList<>(pageResponse.resourceList),
+				new ArrayList<>(pageResponse.getResourceList()),
 				theResponseBundleRequest.bundleType,
 				links.serverBase,
 				server.getBundleInclusionRule(),
@@ -91,6 +90,8 @@ public class ResponseBundleBuilder {
 		final List<IBaseResource> resourceList;
 		final int pageSize;
 
+		ResponsePage.ResponsePageBuilder responsePageBuilder = new ResponsePage.ResponsePageBuilder();
+
 		int numToReturn;
 		String searchId = null;
 
@@ -98,24 +99,33 @@ public class ResponseBundleBuilder {
 			pageSize = offsetCalculatePageSize(server, requestedPage, bundleProvider.size());
 			numToReturn = pageSize;
 
-			resourceList = offsetBuildResourceList(bundleProvider, requestedPage, numToReturn);
+			resourceList = offsetBuildResourceList(bundleProvider, requestedPage, numToReturn, responsePageBuilder);
 			RestfulServerUtils.validateResourceListNotNull(resourceList);
 		} else {
 			pageSize = pagingCalculatePageSize(requestedPage, server.getPagingProvider());
 
-			if (bundleProvider.size() == null) {
+			Integer size = bundleProvider.size();
-				numToReturn = pageSize;
+			numToReturn =
-			} else {
+					(size == null) ? pageSize : Math.min(pageSize, size.intValue() - theResponseBundleRequest.offset);
-				numToReturn = Math.min(pageSize, bundleProvider.size() - theResponseBundleRequest.offset);
-			}
 
-			resourceList = pagingBuildResourceList(theResponseBundleRequest, bundleProvider, numToReturn);
+			resourceList =
+					pagingBuildResourceList(theResponseBundleRequest, bundleProvider, numToReturn, responsePageBuilder);
 			RestfulServerUtils.validateResourceListNotNull(resourceList);
 
 			searchId = pagingBuildSearchId(theResponseBundleRequest, numToReturn, bundleProvider.size());
 		}
 
-		return new ResponsePage(searchId, resourceList, pageSize, numToReturn, bundleProvider.size());
+		// We should leave the IBundleProvider to populate these values (specifically resourceList).
+		// But since we haven't updated all such providers, we will
+		// build it here (this is at best 'duplicating' work).
+		responsePageBuilder
+				.setSearchId(searchId)
+				.setPageSize(pageSize)
+				.setNumToReturn(numToReturn)
+				.setBundleProvider(bundleProvider)
+				.setResources(resourceList);
+
+		return responsePageBuilder.build();
 	}
 
 	private static String pagingBuildSearchId(
@@ -141,11 +151,16 @@ public class ResponseBundleBuilder {
 	}
 
 	private static List<IBaseResource> pagingBuildResourceList(
-			ResponseBundleRequest theResponseBundleRequest, IBundleProvider theBundleProvider, int theNumToReturn) {
+			ResponseBundleRequest theResponseBundleRequest,
+			IBundleProvider theBundleProvider,
+			int theNumToReturn,
+			ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		final List<IBaseResource> retval;
 		if (theNumToReturn > 0 || theBundleProvider.getCurrentPageId() != null) {
 			retval = theBundleProvider.getResources(
-					theResponseBundleRequest.offset, theNumToReturn + theResponseBundleRequest.offset);
+					theResponseBundleRequest.offset,
+					theNumToReturn + theResponseBundleRequest.offset,
+					theResponsePageBuilder);
 		} else {
 			retval = Collections.emptyList();
 		}
@@ -161,15 +176,18 @@ public class ResponseBundleBuilder {
 	}
 
 	private List<IBaseResource> offsetBuildResourceList(
-			IBundleProvider theBundleProvider, RequestedPage theRequestedPage, int theNumToReturn) {
+			IBundleProvider theBundleProvider,
+			RequestedPage theRequestedPage,
+			int theNumToReturn,
+			ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 		final List<IBaseResource> retval;
 		if ((theRequestedPage.offset != null && !myIsOffsetModeHistory)
 				|| theBundleProvider.getCurrentPageOffset() != null) {
 			// When offset query is done theResult already contains correct amount (+ their includes etc.) so return
 			// everything
-			retval = theBundleProvider.getResources(0, Integer.MAX_VALUE);
+			retval = theBundleProvider.getResources(0, Integer.MAX_VALUE, theResponsePageBuilder);
 		} else if (theNumToReturn > 0) {
-			retval = theBundleProvider.getResources(0, theNumToReturn);
+			retval = theBundleProvider.getResources(0, theNumToReturn, theResponsePageBuilder);
 		} else {
 			retval = Collections.emptyList();
 		}
@@ -226,7 +244,6 @@ public class ResponseBundleBuilder {
 
 	private BundleLinks buildLinks(ResponseBundleRequest theResponseBundleRequest, ResponsePage theResponsePage) {
 		final IRestfulServer<?> server = theResponseBundleRequest.server;
-		final IBundleProvider bundleProvider = theResponseBundleRequest.bundleProvider;
 		final RequestedPage pageRequest = theResponseBundleRequest.requestedPage;
 
 		BundleLinks retval = new BundleLinks(
@@ -237,107 +254,16 @@ public class ResponseBundleBuilder {
 
 		retval.setSelf(theResponseBundleRequest.linkSelf);
 
-		if (bundleProvider.getCurrentPageOffset() != null) {
+		// determine if we are using offset / uncached pages
-
+		theResponsePage.setUseOffsetPaging(pageRequest.offset != null
-			if (StringUtils.isNotBlank(bundleProvider.getNextPageId())) {
-				retval.setNext(RestfulServerUtils.createOffsetPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails.getRequestPath(),
-						theResponseBundleRequest.requestDetails.getTenantId(),
-						pageRequest.offset + pageRequest.limit,
-						pageRequest.limit,
-						theResponseBundleRequest.getRequestParameters()));
-			}
-			if (StringUtils.isNotBlank(bundleProvider.getPreviousPageId())) {
-				retval.setNext(RestfulServerUtils.createOffsetPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails.getRequestPath(),
-						theResponseBundleRequest.requestDetails.getTenantId(),
-						Math.max(pageRequest.offset - pageRequest.limit, 0),
-						pageRequest.limit,
-						theResponseBundleRequest.getRequestParameters()));
-			}
-		}
-
-		if (pageRequest.offset != null
 				|| (!server.canStoreSearchResults() && !isEverythingOperation(theResponseBundleRequest.requestDetails))
-				|| myIsOffsetModeHistory) {
+				|| myIsOffsetModeHistory);
-			// Paging without caching
+		theResponsePage.setResponseBundleRequest(theResponseBundleRequest);
-			// We're doing offset pages
+		theResponsePage.setRequestedPage(pageRequest);
-			int requestedToReturn = theResponsePage.numToReturn;
 
-			if (pageRequest.offset != null) {
+		// generate our links
-				requestedToReturn += pageRequest.offset;
+		theResponsePage.setNextPageIfNecessary(retval);
-			}
+		theResponsePage.setPreviousPageIfNecessary(retval);
-
-			if (theResponsePage.numTotalResults == null || requestedToReturn < theResponsePage.numTotalResults) {
-
-				retval.setNext(RestfulServerUtils.createOffsetPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails.getRequestPath(),
-						theResponseBundleRequest.requestDetails.getTenantId(),
-						ObjectUtils.defaultIfNull(pageRequest.offset, 0) + theResponsePage.numToReturn,
-						theResponsePage.numToReturn,
-						theResponseBundleRequest.getRequestParameters()));
-			}
-
-			if (pageRequest.offset != null && pageRequest.offset > 0) {
-				int start = Math.max(0, pageRequest.offset - theResponsePage.pageSize);
-				retval.setPrev(RestfulServerUtils.createOffsetPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails.getRequestPath(),
-						theResponseBundleRequest.requestDetails.getTenantId(),
-						start,
-						theResponsePage.pageSize,
-						theResponseBundleRequest.getRequestParameters()));
-			}
-
-		} else if (StringUtils.isNotBlank(bundleProvider.getCurrentPageId())) {
-			// We're doing named pages
-			final String uuid = bundleProvider.getUuid();
-			if (StringUtils.isNotBlank(bundleProvider.getNextPageId())) {
-				retval.setNext(RestfulServerUtils.createPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails,
-						uuid,
-						bundleProvider.getNextPageId(),
-						theResponseBundleRequest.getRequestParameters()));
-			}
-
-			if (StringUtils.isNotBlank(bundleProvider.getPreviousPageId())) {
-				retval.setPrev(RestfulServerUtils.createPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails,
-						uuid,
-						bundleProvider.getPreviousPageId(),
-						theResponseBundleRequest.getRequestParameters()));
-			}
-
-		} else if (theResponsePage.searchId != null) {
-
-			if (theResponsePage.numTotalResults == null
-					|| theResponseBundleRequest.offset + theResponsePage.numToReturn
-							< theResponsePage.numTotalResults) {
-				retval.setNext((RestfulServerUtils.createPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails,
-						theResponsePage.searchId,
-						theResponseBundleRequest.offset + theResponsePage.numToReturn,
-						theResponsePage.numToReturn,
-						theResponseBundleRequest.getRequestParameters())));
-			}
-
-			if (theResponseBundleRequest.offset > 0) {
-				int start = Math.max(0, theResponseBundleRequest.offset - theResponsePage.pageSize);
-				retval.setPrev(RestfulServerUtils.createPagingLink(
-						retval,
-						theResponseBundleRequest.requestDetails,
-						theResponsePage.searchId,
-						start,
-						theResponsePage.pageSize,
-						theResponseBundleRequest.getRequestParameters()));
-			}
-		}
 
 		return retval;
 	}

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

@@ -19,7 +19,14 @@
  */
 package ca.uhn.fhir.rest.server.method;
 
+import ca.uhn.fhir.rest.api.BundleLinks;
+import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.server.RestfulServerUtils;
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
@@ -27,42 +34,419 @@ import java.util.List;
  * This is an intermediate record object that holds all the fields required to make the final bundle that will be returned to the client.
  */
 public class ResponsePage {
+	private static final Logger ourLog = LoggerFactory.getLogger(ResponsePage.class);
+
 	/**
 	 * The id of the search used to page through search results
 	 */
-	public final String searchId;
+	private final String mySearchId;
 	/**
 	 * The list of resources that will be used to create the bundle
 	 */
-	public final List<IBaseResource> resourceList;
+	private final List<IBaseResource> myResourceList;
 	/**
 	 * The total number of results that matched the search
 	 */
-	public final Integer numTotalResults;
+	private final Integer myNumTotalResults;
 	/**
 	 * The number of resources that should be returned in each page
 	 */
-	public final int pageSize;
+	private final int myPageSize;
 	/**
-	 * The number of resources that should be returned in the bundle.  Can be smaller than pageSize when the bundleProvider
+	 * The number of resources that should be returned in the bundle.
+	 * Can be smaller than pageSize when the bundleProvider
 	 * has fewer results than the page size.
 	 */
-	public final int numToReturn;
+	private final int myNumToReturn;
 
-	public ResponsePage(
+	/**
+	 * The count of resources included from the _include filter.
+	 * These _include resources are otherwise included in the resourceList.
+	 */
+	private final int myIncludedResourceCount;
+	/**
+	 * This is the count of resources that have been omitted from results
+	 * (typically because of consent interceptors).
+	 * We track these because they shouldn't change paging results,
+	 * even though it will change number of resources returned.
+	 */
+	private final int myOmittedResourceCount;
+
+	/**
+	 * The bundle provider.
+	 */
+	private final IBundleProvider myBundleProvider;
+
+	// Properties below here are set for calculation of pages;
+	// not part of the response pages in and of themselves
+
+	/**
+	 * The response bundle request object
+	 */
+	private ResponseBundleRequest myResponseBundleRequest;
+
+	/**
+	 * Whether or not this page uses (non-cached) offset paging
+	 */
+	private boolean myIsUsingOffsetPages = false;
+
+	/**
+	 * The requested page object (should not be null for proper calculations)
+	 */
+	private RequestedPage myRequestedPage;
+
+	/**
+	 * The paging style being used.
+	 * This is determined by a number of conditions,
+	 * including what the bundleprovider provides.
+	 */
+	private PagingStyle myPagingStyle;
+
+	ResponsePage(
 			String theSearchId,
 			List<IBaseResource> theResourceList,
 			int thePageSize,
 			int theNumToReturn,
-			Integer theNumTotalResults) {
+			int theIncludedResourceCount,
-		searchId = theSearchId;
+			int theOmittedResourceCount,
-		resourceList = theResourceList;
+			IBundleProvider theBundleProvider) {
-		pageSize = thePageSize;
+		mySearchId = theSearchId;
-		numToReturn = theNumToReturn;
+		myResourceList = theResourceList;
-		numTotalResults = theNumTotalResults;
+		myPageSize = thePageSize;
+		myNumToReturn = theNumToReturn;
+		myIncludedResourceCount = theIncludedResourceCount;
+		myOmittedResourceCount = theOmittedResourceCount;
+		myBundleProvider = theBundleProvider;
+
+		myNumTotalResults = myBundleProvider.size();
 	}
 
 	public int size() {
-		return resourceList.size();
+		return myResourceList.size();
+	}
+
+	public List<IBaseResource> getResourceList() {
+		return myResourceList;
+	}
+
+	private boolean isBundleProviderOffsetPaging() {
+		if (myBundleProvider != null) {
+			if (myBundleProvider.getCurrentPageOffset() != null) {
+				// it's not enough that currentpageoffset is not null
+				// (sometimes it's 0, even if it's not a currentpageoffset search)
+				// so we have to make sure either next or prev links are not null
+				return (StringUtils.isNotBlank(myBundleProvider.getNextPageId())
+						|| StringUtils.isNotBlank(myBundleProvider.getPreviousPageId()));
+			}
+		}
+
+		return false;
+	}
+
+	private void determinePagingStyle() {
+		if (myPagingStyle != null) {
+			// already assigned
+			return;
+		}
+
+		if (isBundleProviderOffsetPaging()) {
+			myPagingStyle = PagingStyle.BUNDLE_PROVIDER_OFFSETS;
+		} else if (myIsUsingOffsetPages) {
+			myPagingStyle = PagingStyle.NONCACHED_OFFSET;
+		} else if (myBundleProvider != null && StringUtils.isNotBlank(myBundleProvider.getCurrentPageId())) {
+			myPagingStyle = PagingStyle.BUNDLE_PROVIDER_PAGE_IDS;
+		} else if (StringUtils.isNotBlank(mySearchId)) {
+			myPagingStyle = PagingStyle.SAVED_SEARCH;
+		} else {
+			myPagingStyle = PagingStyle.NONE;
+			// only end up here if no paging is desired
+			ourLog.debug(
+					"No accurate paging will be generated."
+							+ " If accurate paging is desired, ResponsePageBuilder must be provided with additioanl information.");
+		}
+	}
+
+	public void setRequestedPage(RequestedPage theRequestedPage) {
+		myRequestedPage = theRequestedPage;
+	}
+
+	public IBundleProvider getBundleProvider() {
+		return myBundleProvider;
+	}
+
+	public void setUseOffsetPaging(boolean theIsUsingOffsetPaging) {
+		myIsUsingOffsetPages = theIsUsingOffsetPaging;
+	}
+
+	public void setResponseBundleRequest(ResponseBundleRequest theRequest) {
+		myResponseBundleRequest = theRequest;
+	}
+
+	private boolean hasNextPage() {
+		determinePagingStyle();
+		switch (myPagingStyle) {
+			case BUNDLE_PROVIDER_OFFSETS:
+			case BUNDLE_PROVIDER_PAGE_IDS:
+				return StringUtils.isNotBlank(myBundleProvider.getNextPageId());
+			case NONCACHED_OFFSET:
+				if (myNumTotalResults == null) {
+					/*
+					 * Having a null total results is synonymous with
+					 * having a next link. Once our results are exhausted,
+					 * we will always have a myNumTotalResults value.
+					 *
+					 * Alternatively, if _total=accurate is provided,
+					 * we'll also have a myNumTotalResults value.
+					 */
+					return true;
+				} else if (myNumTotalResults > myNumToReturn + ObjectUtils.defaultIfNull(myRequestedPage.offset, 0)) {
+					return true;
+				}
+				break;
+			case SAVED_SEARCH:
+				if (myNumTotalResults == null) {
+					if (myPageSize == myResourceList.size() + myOmittedResourceCount - myIncludedResourceCount) {
+						// if the size of the resource list - included resources + omitted resources == pagesize
+						// we have more pages
+						return true;
+					}
+				} else if (myResponseBundleRequest.offset + myNumToReturn < myNumTotalResults) {
+					return true;
+				}
+				break;
+		}
+
+		// fallthrough
+		return false;
+	}
+
+	public void setNextPageIfNecessary(BundleLinks theLinks) {
+		if (hasNextPage()) {
+			String next;
+			switch (myPagingStyle) {
+				case BUNDLE_PROVIDER_OFFSETS:
+					next = RestfulServerUtils.createOffsetPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails.getRequestPath(),
+							myResponseBundleRequest.requestDetails.getTenantId(),
+							myRequestedPage.offset + myRequestedPage.limit,
+							myRequestedPage.limit,
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				case NONCACHED_OFFSET:
+					next = RestfulServerUtils.createOffsetPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails.getRequestPath(),
+							myResponseBundleRequest.requestDetails.getTenantId(),
+							ObjectUtils.defaultIfNull(myRequestedPage.offset, 0) + myNumToReturn,
+							myNumToReturn,
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				case BUNDLE_PROVIDER_PAGE_IDS:
+					next = RestfulServerUtils.createPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails,
+							myBundleProvider.getUuid(),
+							myBundleProvider.getNextPageId(),
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				case SAVED_SEARCH:
+					next = RestfulServerUtils.createPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails,
+							mySearchId,
+							myResponseBundleRequest.offset + myNumToReturn,
+							myNumToReturn,
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				default:
+					next = null;
+					break;
+			}
+
+			if (StringUtils.isNotBlank(next)) {
+				theLinks.setNext(next);
+			}
+		}
+	}
+
+	private boolean hasPreviousPage() {
+		determinePagingStyle();
+		switch (myPagingStyle) {
+			case BUNDLE_PROVIDER_OFFSETS:
+			case BUNDLE_PROVIDER_PAGE_IDS:
+				return StringUtils.isNotBlank(myBundleProvider.getPreviousPageId());
+			case NONCACHED_OFFSET:
+				if (myRequestedPage != null && myRequestedPage.offset != null && myRequestedPage.offset > 0) {
+					return true;
+				}
+				break;
+			case SAVED_SEARCH:
+				return myResponseBundleRequest.offset > 0;
+		}
+
+		// fallthrough
+		return false;
+	}
+
+	public void setPreviousPageIfNecessary(BundleLinks theLinks) {
+		if (hasPreviousPage()) {
+			String prev;
+			switch (myPagingStyle) {
+				case BUNDLE_PROVIDER_OFFSETS:
+					prev = RestfulServerUtils.createOffsetPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails.getRequestPath(),
+							myResponseBundleRequest.requestDetails.getTenantId(),
+							Math.max(ObjectUtils.defaultIfNull(myRequestedPage.offset, 0) - myRequestedPage.limit, 0),
+							myRequestedPage.limit,
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				case NONCACHED_OFFSET:
+					{
+						int start = Math.max(0, ObjectUtils.defaultIfNull(myRequestedPage.offset, 0) - myPageSize);
+						prev = RestfulServerUtils.createOffsetPagingLink(
+								theLinks,
+								myResponseBundleRequest.requestDetails.getRequestPath(),
+								myResponseBundleRequest.requestDetails.getTenantId(),
+								start,
+								myPageSize,
+								myResponseBundleRequest.getRequestParameters());
+					}
+					break;
+				case BUNDLE_PROVIDER_PAGE_IDS:
+					prev = RestfulServerUtils.createPagingLink(
+							theLinks,
+							myResponseBundleRequest.requestDetails,
+							myBundleProvider.getUuid(),
+							myBundleProvider.getPreviousPageId(),
+							myResponseBundleRequest.getRequestParameters());
+					break;
+				case SAVED_SEARCH:
+					{
+						int start = Math.max(0, myResponseBundleRequest.offset - myPageSize);
+						prev = RestfulServerUtils.createPagingLink(
+								theLinks,
+								myResponseBundleRequest.requestDetails,
+								mySearchId,
+								start,
+								myPageSize,
+								myResponseBundleRequest.getRequestParameters());
+					}
+					break;
+				default:
+					prev = null;
+			}
+
+			if (StringUtils.isNotBlank(prev)) {
+				theLinks.setPrev(prev);
+			}
+		}
+	}
+
+	/**
+	 * A builder for constructing ResponsePage objects.
+	 */
+	public static class ResponsePageBuilder {
+
+		private String mySearchId;
+		private List<IBaseResource> myResources;
+		private int myPageSize;
+		private int myNumToReturn;
+		private int myIncludedResourceCount;
+		private int myOmittedResourceCount;
+		private IBundleProvider myBundleProvider;
+
+		public ResponsePageBuilder setToOmittedResourceCount(int theOmittedResourcesCountToAdd) {
+			myOmittedResourceCount = theOmittedResourcesCountToAdd;
+			return this;
+		}
+
+		public ResponsePageBuilder setIncludedResourceCount(int theIncludedResourceCount) {
+			myIncludedResourceCount = theIncludedResourceCount;
+			return this;
+		}
+
+		public ResponsePageBuilder setNumToReturn(int theNumToReturn) {
+			myNumToReturn = theNumToReturn;
+			return this;
+		}
+
+		public ResponsePageBuilder setPageSize(int thePageSize) {
+			myPageSize = thePageSize;
+			return this;
+		}
+
+		public ResponsePageBuilder setBundleProvider(IBundleProvider theBundleProvider) {
+			myBundleProvider = theBundleProvider;
+			return this;
+		}
+
+		public ResponsePageBuilder setResources(List<IBaseResource> theResources) {
+			myResources = theResources;
+			return this;
+		}
+
+		public ResponsePageBuilder setSearchId(String theSearchId) {
+			mySearchId = theSearchId;
+			return this;
+		}
+
+		public ResponsePage build() {
+			return new ResponsePage(
+					mySearchId, // search id
+					myResources, // resource list
+					myPageSize, // page size
+					myNumToReturn, // num to return
+					myIncludedResourceCount, // included count
+					myOmittedResourceCount, // omitted resources
+					myBundleProvider // the bundle provider
+					);
+		}
+	}
+
+	/**
+	 * First we determine what kind of paging we use:
+	 * * Bundle Provider Offsets - the bundle provider has offset counts that it uses
+	 * 							to determine the page. For legacy reasons, it's not enough
+	 * 							that the bundle provider has a currentOffsetPage. Sometimes
+	 * 							this value is provided (often as a 0), but no nextPageId nor previousPageId
+	 * 							is available. Typically this is the case in UnitTests.
+	 * * non-cached offsets - if the server is not storing the search results (and it's not
+	 * 							an everything operator) OR the Requested Page has an initial offset
+	 * 							OR it is explicitly set to use non-cached offset
+	 * 							(ResponseBundleBuilder.myIsOffsetModeHistory)
+	 * * Bundle Provider Page Ids - the bundle provider knows the page ids and will
+	 * 							provide them. bundle provider will have a currentPageId
+	 * * Saved Search			- the server has a saved search object with an id that it
+	 * 							uses to page through results.
+	 */
+	private enum PagingStyle {
+		/**
+		 * Paging is done by offsets; pages are not cached
+		 */
+		NONCACHED_OFFSET,
+		/**
+		 * Paging is done by offsets, but
+		 * the bundle provider provides the offsets
+		 */
+		BUNDLE_PROVIDER_OFFSETS,
+		/**
+		 * Paging is done by page ids,
+		 * but bundle provider provides the page ids
+		 */
+		BUNDLE_PROVIDER_PAGE_IDS,
+		/**
+		 * The server has a saved search object with an id
+		 * that is used to page through results.
+		 */
+		SAVED_SEARCH,
+		/**
+		 * No paging is done at all.
+		 * No previous nor next links will be available, even if previous or next
+		 * links exist.
+		 * If paging is required, a different paging method must be specified.
+		 */
+		NONE;
 	}
 }

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

@@ -53,6 +53,7 @@ import ca.uhn.fhir.rest.server.IResourceProvider;
 import ca.uhn.fhir.rest.server.SimpleBundleProvider;
 import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import ca.uhn.fhir.util.ValidateUtil;
 import com.google.common.collect.Lists;
@@ -319,7 +320,10 @@ public class HashMapResourceProvider<T extends IBaseResource> implements IResour
 			@SuppressWarnings("unchecked")
 			@Nonnull
 			@Override
-			public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
+			public List<IBaseResource> getResources(
+					int theFromIndex,
+					int theToIndex,
+					@Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
 
 				// Make sure that "from" isn't less than 0, "to" isn't more than the number available,
 				// and "from" <= "to"

hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/api/server/method/ResponsePageTest.java

@@ -0,0 +1,320 @@
+package ca.uhn.fhir.rest.api.server.method;
+
+import ca.uhn.fhir.model.valueset.BundleTypeEnum;
+import ca.uhn.fhir.rest.api.BundleLinks;
+import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
+import ca.uhn.fhir.rest.server.method.RequestedPage;
+import ca.uhn.fhir.rest.server.method.ResponseBundleRequest;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+public class ResponsePageTest {
+
+	private ResponsePage.ResponsePageBuilder myBundleBuilder;
+
+	private BundleLinks myLinks;
+
+	private List<IBaseResource> myList;
+
+	@Mock
+	private IBundleProvider myBundleProvider;
+
+	private ResponseBundleRequest myRequest;
+
+	@BeforeEach
+	public void before() {
+		myBundleBuilder = new ResponsePage.ResponsePageBuilder();
+
+		myLinks = new BundleLinks(
+			"http://localhost", // server base
+			new HashSet<>(), // includes set
+			false, // pretty print
+			BundleTypeEnum.SEARCHSET // links type
+		);
+
+		myList = new ArrayList<>();
+		for (int i = 0; i < 10; i++) {
+			// does not matter what these are
+			myList.add(mock(IBaseResource.class));
+		}
+
+		myRequest = createBundleRequest(0);
+	}
+
+	@ParameterizedTest
+	@CsvSource({
+		"false,false",
+		"true,false",
+		"false,true",
+		"true,true"
+	})
+	public void bundleProviderOffsets_setsNextPreviousLinks_test(
+		boolean theHasPreviousBoolean,
+		boolean theHasNextBoolean
+	) {
+		// setup
+		myBundleBuilder
+			.setBundleProvider(myBundleProvider)
+			.setResources(myList);
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+
+		// when
+		if (theHasNextBoolean) {
+			when(myBundleProvider.getNextPageId())
+				.thenReturn("next");
+		}
+		if (theHasPreviousBoolean) {
+			when(myBundleProvider.getPreviousPageId())
+				.thenReturn("previous");
+		}
+		when(myBundleProvider.getCurrentPageOffset())
+			.thenReturn(1);
+
+		// test
+		page.setNextPageIfNecessary(myLinks);
+		page.setPreviousPageIfNecessary(myLinks);
+
+		// verify
+		verifyNextAndPreviousLinks(theHasPreviousBoolean, theHasNextBoolean);
+	}
+
+	@ParameterizedTest
+	@CsvSource({
+		"false,false",
+		"true,false",
+		"false,true",
+		"true,true"
+	})
+	public void bundleProviderPageIds_setsNextPreviousLinks_test(
+		boolean theHasPreviousBoolean,
+		boolean theHasNextBoolean
+	) {
+		// setup
+		// setup
+		myBundleBuilder
+			.setBundleProvider(myBundleProvider)
+			.setResources(myList)
+		;
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+
+		// when
+		if (theHasNextBoolean) {
+			when(myBundleProvider.getNextPageId())
+				.thenReturn("next");
+		}
+		if (theHasPreviousBoolean) {
+			when(myBundleProvider.getPreviousPageId())
+				.thenReturn("previous");
+		}
+
+		// test
+		page.setNextPageIfNecessary(myLinks);
+		page.setPreviousPageIfNecessary(myLinks);
+
+		// verify
+		verifyNextAndPreviousLinks(theHasPreviousBoolean, theHasNextBoolean);
+	}
+
+	/**
+	 * Tests for next and previous links
+	 * when doing non-cached offsets.
+	 *
+	 * NB: In a non-cached search, having a null
+	 *     myNumTotalResult is synonymous with having
+	 *     a next link.
+	 *     As such, we do not test for
+	 *     null myNumTotalResults and expect no
+	 *     next.
+	 *     These test cases are omitted as a result.
+	 */
+	@ParameterizedTest
+	@CsvSource({
+		"true,false,true",
+		"true,true,true",
+		"false,false,false",
+		"false,true,false",
+		"false,false,true",
+		"false,true,true"
+	})
+	public void nonCachedOffsetPaging_setsNextPreviousLinks_test(
+		boolean theNumTotalResultsIsNull,
+		boolean theHasPreviousBoolean,
+		boolean theHasNextBoolean
+	) {
+		// setup
+		myBundleBuilder
+			.setBundleProvider(myBundleProvider)
+			.setResources(myList);
+
+		int offset = theHasPreviousBoolean ? 10 : 0;
+
+		if (!theHasNextBoolean) {
+			myBundleBuilder.setNumToReturn(10);
+		}
+
+		// when
+		when(myBundleProvider.getCurrentPageOffset())
+			.thenReturn(null);
+		if (!theNumTotalResultsIsNull) {
+			when(myBundleProvider.size())
+				.thenReturn(10 + offset);
+		} else {
+			when(myBundleProvider.size())
+				.thenReturn(null);
+		}
+
+		RequestedPage requestedPage = new RequestedPage(
+			offset, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+		page.setUseOffsetPaging(true);
+
+		// test
+		page.setNextPageIfNecessary(myLinks);
+		page.setPreviousPageIfNecessary(myLinks);
+
+		// verify
+		verifyNextAndPreviousLinks(theHasPreviousBoolean, theHasNextBoolean);
+	}
+
+	@ParameterizedTest
+	@CsvSource({
+		"true,false,false",
+		"true,true,false",
+		"true,false,true",
+		"true,true,true",
+		"false,false,false",
+		"false,true,false",
+		"false,false,true",
+		"false,true,true"
+	})
+	public void savedSearch_setsNextPreviousLinks_test(
+		boolean theNumTotalResultsIsNull,
+		boolean theHasPreviousBoolean,
+		boolean theHasNextBoolean
+	) {
+		// setup
+		int pageSize = myList.size();
+		myBundleBuilder
+			.setResources(myList)
+			.setSearchId("search-id")
+			.setBundleProvider(myBundleProvider)
+			.setPageSize(pageSize);
+
+		int offset = 0;
+		int includeResourceCount = 0;
+		if (theHasPreviousBoolean) {
+			offset = 10;
+			myRequest = createBundleRequest(offset);
+		}
+
+		if (!theHasNextBoolean) {
+			// add some includes to reach up to pagesize
+			includeResourceCount = 1;
+		}
+
+		myBundleBuilder.setIncludedResourceCount(includeResourceCount);
+
+		if (!theNumTotalResultsIsNull) {
+			if (!theHasNextBoolean) {
+				myBundleBuilder.setNumToReturn(pageSize + offset + includeResourceCount);
+			}
+		}
+
+		// when
+		when(myBundleProvider.getCurrentPageOffset())
+			.thenReturn(null);
+		if (!theNumTotalResultsIsNull) {
+			// accurate total (myNumTotalResults has a value)
+			when(myBundleProvider.size())
+				.thenReturn(offset + pageSize);
+		} else {
+			when(myBundleProvider.size())
+				.thenReturn(null);
+		}
+
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+
+		// test
+		page.setNextPageIfNecessary(myLinks);
+		page.setPreviousPageIfNecessary(myLinks);
+
+		// verify
+		verifyNextAndPreviousLinks(theHasPreviousBoolean, theHasNextBoolean);
+	}
+
+	private ResponseBundleRequest createBundleRequest(int theOffset) {
+		RequestDetails details = new SystemRequestDetails();
+		details.setFhirServerBase("http://serverbase.com");
+		return new ResponseBundleRequest(
+			null, // server
+			myBundleProvider,
+			details,
+			theOffset, // offset
+			null, // limit
+			"self", // self link
+			new HashSet<>(), // includes
+			BundleTypeEnum.SEARCHSET,
+			"search-id"
+		);
+	}
+
+	private void verifyNextAndPreviousLinks(
+		boolean theHasPreviousBoolean,
+		boolean theHasNextBoolean
+	) {
+		if (theHasNextBoolean) {
+			assertNotNull(myLinks.getNext(), "Next link expected but not found");
+		} else {
+			assertNull(myLinks.getNext(), "Found unexpected next link");
+		}
+		if (theHasPreviousBoolean) {
+			assertNotNull(myLinks.getPrev(), "Previous link expected but not found");
+		} else {
+			assertNull(myLinks.getPrev(), "Found unexpected previous link");
+		}
+	}
+}

hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java

@@ -13,6 +13,7 @@ import ca.uhn.fhir.rest.server.SimpleBundleProvider;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Patient;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -150,6 +151,47 @@ class ResponseBundleBuilderTest {
 		assertNextLink(bundle, DEFAULT_PAGE_SIZE);
 	}
 
+	@Test
+	public void buildResponseBundle_withIncludeParamAndFewerResultsThanPageSize_doesNotReturnNextLink() {
+		// setup
+		int includeResources = 4;
+		// we want the number of resources returned to be equal to the pagesize
+		List<IBaseResource> list = buildXPatientList(DEFAULT_PAGE_SIZE - includeResources);
+
+		ResponseBundleBuilder svc = new ResponseBundleBuilder(false);
+
+		SimpleBundleProvider provider = new SimpleBundleProvider() {
+			@Nonnull
+			@Override
+			public List<IBaseResource> getResources(int theFrom, int theTo, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
+				List<IBaseResource> retList = new ArrayList<>(list);
+				// our fake includes
+				for (int i = 0; i < includeResources; i++) {
+					retList.add(new Organization().setId("Organization/" + i));
+				}
+				theResponsePageBuilder.setIncludedResourceCount(includeResources);
+				return retList;
+			}
+		};
+
+		provider.setSize(null);
+
+		// mocking
+		when(myServer.canStoreSearchResults()).thenReturn(true);
+		when(myServer.getPagingProvider()).thenReturn(myPagingProvider);
+		when(myPagingProvider.getDefaultPageSize()).thenReturn(DEFAULT_PAGE_SIZE);
+
+		ResponseBundleRequest req = buildResponseBundleRequest(provider, "search-id");
+
+		// test
+		Bundle bundle = (Bundle) svc.buildResponseBundle(req);
+
+		// verify
+		// no next link
+		assertEquals(1, bundle.getLink().size());
+		assertEquals(DEFAULT_PAGE_SIZE, bundle.getEntry().size());
+	}
+
 	@ParameterizedTest
 	@ValueSource(booleans = {true, false})
 	void testFilterNulls(boolean theCanStoreSearchResults) {
@@ -423,8 +465,12 @@ class ResponseBundleBuilderTest {
 	}
 
 	private List<IBaseResource> buildPatientList() {
+		return buildXPatientList(ResponseBundleBuilderTest.RESOURCE_COUNT);
+	}
+
+	private List<IBaseResource> buildXPatientList(int theCount) {
 		List<IBaseResource> retval = new ArrayList<>();
-		for (int i = 0; i < ResponseBundleBuilderTest.RESOURCE_COUNT; ++i) {
+		for (int i = 0; i < theCount; ++i) {
 			Patient p = new Patient();
 			p.setId("A" + i);
 			p.setActive(true);
@@ -499,10 +545,9 @@ class ResponseBundleBuilderTest {
 		}
 
 		@Nonnull
-		@Override
+		public List<IBaseResource> getResources(int theFromIndex, int theToIndex, @Nonnull ResponsePage.ResponsePageBuilder theResponseBundleBuilder) {
-		public List<IBaseResource> getResources(int theFromIndex, int theToIndex) {
 			getResourcesCalled = true;
-			return super.getResources(theFromIndex, theToIndex);
+			return super.getResources(theFromIndex, theToIndex, theResponseBundleBuilder);
 		}
 
 		// Emulate the behaviour of PersistedJpaBundleProvider where size() is only set after getResources() has been called

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

@@ -4,6 +4,7 @@ import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.rest.annotation.Search;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.param.TokenAndListParam;
+import ca.uhn.fhir.rest.server.method.ResponsePage;
 import ca.uhn.fhir.test.utilities.JettyUtil;
 import ca.uhn.fhir.util.TestUtil;
 import com.google.common.collect.Lists;
@@ -64,7 +65,8 @@ public class SearchBundleProviderWithNoSizeR4Test {
 		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>>() {
+		when(ourLastBundleProvider.getResources(any(int.class), any(int.class), any(ResponsePage.ResponsePageBuilder.class)))
+			.then(new Answer<List<IBaseResource>>() {
 				@Override
 				public List<IBaseResource> answer(InvocationOnMock theInvocation) {
 					int from = (Integer) theInvocation.getArguments()[0];
@@ -76,7 +78,8 @@ public class SearchBundleProviderWithNoSizeR4Test {
 						retVal.add(p);
 					}
 					return retVal;
-			}});
+				}
+			});
 
 		HttpGet httpGet;
 		CloseableHttpResponse status = null;