diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5057-paging-regression.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5057-paging-regression.yaml new file mode 100644 index 00000000000..004183060e6 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5057-paging-regression.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 5057 +title: "Fixed recently introduced regression that when following bundle next links through to the end, in certain circumstances +a next link would appear in the final bundle when in fact it was the last page." diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java index 3c7b769f33a..818d47bdf35 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java @@ -60,7 +60,6 @@ public class ResponseBundleBuilder { private ResponsePage buildResponsePage(ResponseBundleRequest theResponseBundleRequest) { final IRestfulServer server = theResponseBundleRequest.server; final IBundleProvider bundleProvider = theResponseBundleRequest.bundleProvider; - final Integer bundleProviderSize = bundleProvider.size(); final RequestedPage requestedPage = theResponseBundleRequest.requestedPage; final List resourceList; final int pageSize; @@ -69,7 +68,7 @@ public class ResponseBundleBuilder { String searchId = null; if (requestedPage.offset != null || !server.canStoreSearchResults()) { - pageSize = offsetCalculatePageSize(server, requestedPage, bundleProviderSize); + pageSize = offsetCalculatePageSize(server, requestedPage, bundleProvider.size()); numToReturn = pageSize; resourceList = offsetBuildResourceList(bundleProvider, requestedPage, numToReturn); @@ -77,19 +76,19 @@ public class ResponseBundleBuilder { } else { pageSize = pagingCalculatePageSize(requestedPage, server.getPagingProvider()); - if (bundleProviderSize == null) { + if (bundleProvider.size() == null) { numToReturn = pageSize; } else { - numToReturn = Math.min(pageSize, bundleProviderSize - theResponseBundleRequest.offset); + numToReturn = Math.min(pageSize, bundleProvider.size() - theResponseBundleRequest.offset); } resourceList = pagingBuildResourceList(theResponseBundleRequest, bundleProvider, numToReturn); RestfulServerUtils.validateResourceListNotNull(resourceList); - searchId = pagingBuildSearchId(theResponseBundleRequest, numToReturn, bundleProviderSize); + searchId = pagingBuildSearchId(theResponseBundleRequest, numToReturn, bundleProvider.size()); } - return new ResponsePage(searchId, resourceList, pageSize, numToReturn, bundleProviderSize); + return new ResponsePage(searchId, resourceList, pageSize, numToReturn, bundleProvider.size()); } private static String pagingBuildSearchId(ResponseBundleRequest theResponseBundleRequest, int theNumToReturn, Integer theNumTotalResults) { diff --git a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java index 9f991ff42a4..f0e3b5ec714 100644 --- a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java +++ b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java @@ -59,6 +59,7 @@ class ResponseBundleBuilderTest { private static final FhirContext ourFhirContext = FhirContext.forR4Cached(); private static final String TEST_REQUEST_PATH = "test/request/path"; private static final Integer REQUEST_OFFSET = 3; + private static final Integer NEAR_END_NO_NEXT_REQUEST_OFFSET = 49; @Mock IRestfulServer myServer; @Mock @@ -363,6 +364,30 @@ class ResponseBundleBuilderTest { assertPrevLinkOffset(bundle); } + @Test + void testHighOffsetWithSearchIdAndInitiallyUnknownBundleSize() { + // setup + setCanStoreSearchResults(true); + SimpleBundleProvider bundleProvider = new TestBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider, SEARCH_ID, NEAR_END_NO_NEXT_REQUEST_OFFSET); + + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(false); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, RESOURCE_COUNT - NEAR_END_NO_NEXT_REQUEST_OFFSET, "A49", "A49"); + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + + Bundle.BundleLinkComponent nextLink = bundle.getLink().get(1); + assertEquals(LINK_PREVIOUS, nextLink.getRelation()); + int prevOffset = NEAR_END_NO_NEXT_REQUEST_OFFSET - DEFAULT_PAGE_SIZE; + assertEquals(TEST_SERVER_BASE + "?_getpages=" + SEARCH_ID + "&_getpagesoffset=" + prevOffset + "&_count=" + ResponseBundleBuilderTest.DEFAULT_PAGE_SIZE + "&_bundletype=" + SEARCHSET.toCode(), nextLink.getUrl()); + } + private static void assertNextLinkOffset(Bundle theBundle, Integer theOffset, Integer theCount) { Bundle.BundleLinkComponent nextLink = theBundle.getLink().get(1); @@ -466,4 +491,27 @@ class ResponseBundleBuilderTest { assertEquals(theLastId, entries.get(theExpectedEntryCount - 1).getResource().getId()); } } + + private static class TestBundleProvider extends SimpleBundleProvider { + boolean getResourcesCalled = false; + public TestBundleProvider(List theResourceList) { + super(theResourceList); + } + + @Nonnull + @Override + public List getResources(int theFromIndex, int theToIndex) { + getResourcesCalled = true; + return super.getResources(theFromIndex, theToIndex); + } + + // Emulate the behaviour of PersistedJpaBundleProvider where size() is only set after getResources() has been called + @Override + public Integer size() { + if (getResourcesCalled) { + return super.size(); + } + return null; + } + } }