include filter and better next count calculations

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

@@ -250,7 +250,7 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 	}
 
 	protected List<IBaseResource> doSearchOrEverything(
-		final int theFromIndex, final int theToIndex, ResponsePage.ResponsePageBuilder theResponsePageBuilder
+		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)

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

@@ -2652,26 +2652,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		int newSize = client.search().forResource(ImagingStudy.class).returnBundle(Bundle.class).execute().getEntry().size();
 
 		assertEquals(1, newSize - initialSize);
-
 	}
 
-
-
-	private void printResources(Bundle theBundle) {
-		IParser parser = myFhirContext.newJsonParser();
-		for (BundleEntryComponent entry : theBundle.getEntry()) {
-			ourLog.info(entry.getResource().getId());
-//			ourLog.info(
-//				parser.encodeResourceToString(entry.getResource())
-//			);
-		}
-	}
-
-	// TODO - we should not be including the
-	// _include count in the _count
-	// so current behaviour is correct
-	// but we should also not be including a next
-	// list if there are no more resources
 	@Test
 	public void testPagingWithIncludesReturnsConsistentValues() {
 		// setup
@@ -2707,10 +2689,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 			.returnBundle(Bundle.class)
 			.execute();
 		int count = bundle.getEntry().size();
-		assertTrue(bundle.getEntry().size() > 0);
+		assertFalse(bundle.getEntry().isEmpty());
-		System.out.println("Received " + bundle.getEntry().size());
+
-//		assertEquals(requestedAmount, count);
-		printResources(bundle);
 		String nextUrl = null;
 		do {
 			Bundle.BundleLinkComponent nextLink = bundle.getLink("next");
@@ -2724,12 +2704,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 				bundle = myClient.fetchResourceFromUrl(Bundle.class, nextUrl);
 				int received = bundle.getEntry().size();
 
-				// verify it's the same amount as we requested
+				// every next result should produce results
-				System.out.println("Recieved " + received);
+				assertFalse(bundle.getEntry().isEmpty());
-				printResources(bundle);
-
-				assertTrue(bundle.getEntry().size() > 0);
-//				assertEquals(requestedAmount, received);
 				count += received;
 			} else {
 				nextUrl = null;
@@ -2737,6 +2713,7 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		} while (nextUrl != null);
 
 		// verify
+		// we should receive all resources and linked resources
 		assertEquals(total + orgs, count);
 	}
 

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

@@ -32,7 +32,7 @@ public class PersistedJpaBundleProviderTest {
 		Search searchEntity = new Search();
 		searchEntity.setTotalCount(1);
 		myPersistedJpaBundleProvider.setSearchEntity(searchEntity);
-		myPersistedJpaBundleProvider.doSearchOrEverything(0, 1);
+		myPersistedJpaBundleProvider.doSearchOrEverything(0, 1, myPersistedJpaBundleProvider.getResponsePageBuilder());
 		verifyNoInteractions(myDao);
 		verifyNoInteractions(mySearchBuilderFactory);
 	}

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(),
@@ -262,108 +261,20 @@ public class ResponseBundleBuilder {
 				RestfulServerUtils.prettyPrintResponse(server, theResponseBundleRequest.requestDetails),
 				theResponseBundleRequest.bundleType);
 
+		// set self link
 		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;
+		theResponsePage.setBundleProvider(bundleProvider);
 
-			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,6 +19,11 @@
  */
 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 java.util.List;
@@ -30,30 +35,60 @@ public class ResponsePage {
 	/**
 	 * 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
 	 * has fewer results than the page size.
 	 */
-	public final int numToReturn;
+	private final int myNumToReturn;
 
 	/**
 	 * The count of resources included from the _include filter.
 	 * These _include resources are otherwise included in the resourceList.
 	 */
-	private final int includedResourceCount;
+	private final int myIncludedResourceCount;
+
+	// 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 bundle provider.
+	 */
+	private IBundleProvider myBundleProvider;
+
+	/**
+	 * The paging style being used.
+	 * This is determined by a number of conditions,
+	 * including what the bundleprovider provides.
+	 */
+	private PagingStyle myPagingStyle;
 
 	ResponsePage(
 		String theSearchId,
@@ -63,16 +98,231 @@ public class ResponsePage {
 		Integer theNumTotalResults,
 		int theIncludedResourceCount
 	) {
-		searchId = theSearchId;
+		mySearchId = theSearchId;
-		resourceList = theResourceList;
+		myResourceList = theResourceList;
-		pageSize = thePageSize;
+		myPageSize = thePageSize;
-		numToReturn = theNumToReturn;
+		myNumToReturn = theNumToReturn;
-		numTotalResults = theNumTotalResults;
+		myNumTotalResults = theNumTotalResults;
-		includedResourceCount = theIncludedResourceCount;
+		myIncludedResourceCount = theIncludedResourceCount;
 	}
 
 	public int size() {
-		return resourceList.size();
+		return myResourceList.size();
+	}
+
+	public List<IBaseResource> getResourceList() {
+		return myResourceList;
+	}
+
+	private void determinePagingStyle() {
+		if (myPagingStyle != null) {
+			// already assigned
+			return;
+		}
+
+		if (myBundleProvider != null && myBundleProvider.getCurrentPageOffset() != null) {
+			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;
+		}
+
+		assert myPagingStyle != null : "Response page requires more information to determine paging style";
+	}
+
+	public void setRequestedPage(RequestedPage theRequestedPage) {
+		myRequestedPage = theRequestedPage;
+	}
+
+	public IBundleProvider getBundleProvider() {
+		return myBundleProvider;
+	}
+
+	public void setBundleProvider(IBundleProvider theBundleProvider) {
+		myBundleProvider = theBundleProvider;
+	}
+
+	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 + myRequestedPage.offset) {
+					return true;
+				}
+				break;
+			case SAVED_SEARCH:
+				if (myNumTotalResults == null) {
+					if (myPageSize == myResourceList.size() - myIncludedResourceCount) {
+						// if the size of the resource list - included resources == pagesize
+						// we have more pages
+						return true;
+					}
+				} else if (myResponseBundleRequest.offset + myNumToReturn - myIncludedResourceCount < myNumTotalResults) {
+					/*
+					 * if we have an accurate total (myNumTotalResults),
+					 * this value is supposed to exclude _include's resource count
+					 */
+					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);
+			}
+		}
 	}
 
 	/**
@@ -128,4 +378,24 @@ public class ResponsePage {
 			);
 		}
 	}
+
+	private enum PagingStyle {
+		/**
+		 * Paging is done by offsets; pages are not cached
+		 */
+		NONCACHED_OFFSET,
+		/**
+		 * The bundle provider provides the offsets
+		 */
+		BUNDLE_PROVIDER_OFFSETS,
+		/**
+		 * Paging by page ids, but provided by the bundle provider
+		 */
+		BUNDLE_PROVIDER_PAGE_IDS,
+
+		/**
+		 * A saved search (search exists in the database).
+		 */
+		SAVED_SEARCH
+	}
 }

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

@@ -0,0 +1,311 @@
+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
+			.setResources(myList);
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+		page.setBundleProvider(myBundleProvider);
+
+		// 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
+			.setResources(myList)
+		;
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+		page.setBundleProvider(myBundleProvider);
+
+		// 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
+			.setResources(myList);
+
+		int offset = theHasPreviousBoolean ? 10 : 0;
+		if (!theNumTotalResultsIsNull) {
+			myBundleBuilder.setNumTotalResults(10 + offset);
+		}
+
+		if (!theHasNextBoolean) {
+			myBundleBuilder.setNumToReturn(10);
+		}
+
+		RequestedPage requestedPage = new RequestedPage(
+			offset, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+		page.setBundleProvider(myBundleProvider);
+		page.setUseOffsetPaging(true);
+
+		// when
+		when(myBundleProvider.getCurrentPageOffset())
+			.thenReturn(null);
+
+		// 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")
+			.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) {
+			// accurate total (myNumTotalResults has a value)
+			myBundleBuilder.setNumTotalResults(offset + pageSize);
+
+			if (!theHasNextBoolean) {
+				myBundleBuilder.setNumToReturn(pageSize + offset + includeResourceCount);
+			}
+		}
+
+		RequestedPage requestedPage = new RequestedPage(
+			0, // offset
+			10 // limit
+		);
+		ResponsePage page = myBundleBuilder.build();
+
+		page.setResponseBundleRequest(myRequest);
+		page.setRequestedPage(requestedPage);
+		page.setBundleProvider(myBundleProvider);
+
+		// when
+		when(myBundleProvider.getCurrentPageOffset())
+			.thenReturn(null);
+
+		// 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

@@ -156,12 +156,12 @@ class ResponseBundleBuilderTest {
 		// setup
 		int includeResources = 4;
 		// we want the number of resources returned to be equal to the pagesize
-		List<IBaseResource> list = buildXPatientList(DEFAULT_PAGE_SIZE + 1 - includeResources);
+		List<IBaseResource> list = buildXPatientList(DEFAULT_PAGE_SIZE - includeResources);
 
 		ResponsePage.ResponsePageBuilder builder = new ResponsePage.ResponsePageBuilder();
 		builder.setIncludedResourceCount(includeResources);
 
-		ResponseBundleBuilder svc = new ResponseBundleBuilder(true);
+		ResponseBundleBuilder svc = new ResponseBundleBuilder(false);
 
 		SimpleBundleProvider provider = new SimpleBundleProvider() {
 
@@ -173,7 +173,7 @@ class ResponseBundleBuilderTest {
 			@Nonnull
 			@Override
 			public List<IBaseResource> getResources(int theFrom, int theTo, @Nonnull ResponsePage.ResponsePageBuilder theResponsePageBuilder) {
-				List<IBaseResource> retList = list.subList(theFrom, Math.min(theTo, list.size()-1));
+				List<IBaseResource> retList = new ArrayList<>(list);
 				// our fake includes
 				for (int i = 0; i < includeResources; i++) {
 					retList.add(new Organization().setId("Organization/" + i));
@@ -182,22 +182,22 @@ class ResponseBundleBuilderTest {
 			}
 		};
 
-		// TODO - if null, it adds a next link
+		provider.setSize(null);
-		provider.setSize(DEFAULT_PAGE_SIZE);
 
 		// mocking
 		when(myServer.canStoreSearchResults()).thenReturn(true);
 		when(myServer.getPagingProvider()).thenReturn(myPagingProvider);
 		when(myPagingProvider.getDefaultPageSize()).thenReturn(DEFAULT_PAGE_SIZE);
 
-		ResponseBundleRequest req = buildResponseBundleRequest(provider);
+		ResponseBundleRequest req = buildResponseBundleRequest(provider, "search-id");
 
 		// test
 		Bundle bundle = (Bundle) svc.buildResponseBundle(req);
 
 		// verify
+		// no next link
 		assertEquals(1, bundle.getLink().size());
-		verifyBundle(bundle, RESOURCE_COUNT, DEFAULT_PAGE_SIZE -1, "A0", "A14");
+		assertEquals(DEFAULT_PAGE_SIZE, bundle.getEntry().size());
 	}
 
 	@ParameterizedTest