fix hapi paging regression (#5057)

* fix regression

* change log

---------

Co-authored-by: Ken Stevens <ken@smilecdr.com>
This commit is contained in:
Ken Stevens 2023-07-04 14:43:43 -04:00 committed by GitHub
parent 216c6ed4f1
commit 9a6288c363
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 58 additions and 6 deletions

View File

@ -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."

View File

@ -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<IBaseResource> 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) {

View File

@ -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<RequestDetails> 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<IBaseResource> theResourceList) {
super(theResourceList);
}
@Nonnull
@Override
public List<IBaseResource> 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;
}
}
}