diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4468-fix-previous-link-offset-no-cache-pagination.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4468-fix-previous-link-offset-no-cache-pagination.yaml new file mode 100644 index 000000000000..9b71b6281e82 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4468-fix-previous-link-offset-no-cache-pagination.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 4468 +title: "When performing a search using a plain server without a cache, pagination links + could contain invalid offsets in the _prev_ link. Thanks to Aleksej Parovysnik for the + pull request!" diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java index 5dfa3b914366..40c34c35f93b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java @@ -271,7 +271,7 @@ IBaseResource createBundleFromBundleProvider(IRestfulServer theServer, Reques } } if (offset != null && offset > 0) { - int start = Math.max(0, theOffset - pageSize); + int start = Math.max(0, offset - pageSize); links.setPrev(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), start, pageSize, theRequest.getParameters())); } } else if (isNotBlank(theResult.getCurrentPageId())) { diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingTest.java index 47b0352e6062..aed0f1187456 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingTest.java @@ -25,6 +25,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -32,6 +34,7 @@ import java.util.concurrent.TimeUnit; import static ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -146,6 +149,76 @@ public void testPreviousLinkLastPageWhenBundleSizeEqualsPageSizePlusOne() throws } } + @Test() + public void testLinksWhenUsingOffsetPaginationWithNoCaching() throws Exception { + initBundleProvider(10); + ourBundleProvider.setSize(30); + myServerExtension.getRestfulServer().registerProvider(new DummyPatientResourceProvider()); + + String nextLink; + String base = "http://localhost:" + myServerExtension.getPort(); + HttpGet get = new HttpGet(base + "/Patient?_count=10"); + String responseContent; + try (CloseableHttpResponse resp = ourClient.execute(get)) { + assertEquals(200, resp.getStatusLine().getStatusCode()); + responseContent = IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8); + + Bundle bundle = ourContext.newJsonParser().parseResource(Bundle.class, responseContent); + assertEquals(10, bundle.getEntry().size()); + + assertNull(bundle.getLink(IBaseBundle.LINK_PREV)); + + String linkSelf = bundle.getLink(IBaseBundle.LINK_SELF).getUrl(); + assertNotNull(linkSelf, "'self' link is not present"); + + nextLink = bundle.getLink(IBaseBundle.LINK_NEXT).getUrl(); + assertNotNull(nextLink, "'next' link is not present"); + checkParam(nextLink, Constants.PARAM_OFFSET, "10"); + checkParam(nextLink, Constants.PARAM_COUNT, "10"); + } + try (CloseableHttpResponse resp = ourClient.execute(new HttpGet(nextLink))) { + assertEquals(200, resp.getStatusLine().getStatusCode()); + responseContent = IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8); + + Bundle bundle = ourContext.newJsonParser().parseResource(Bundle.class, responseContent); + assertEquals(10, bundle.getEntry().size()); + + String linkPrev = bundle.getLink(IBaseBundle.LINK_PREV).getUrl(); + assertNotNull(linkPrev, "'previous' link is not present"); + checkParam(linkPrev, Constants.PARAM_OFFSET, "0"); + checkParam(linkPrev, Constants.PARAM_COUNT, "10"); + + String linkSelf = bundle.getLink(IBaseBundle.LINK_SELF).getUrl(); + assertNotNull(linkSelf, "'self' link is not present"); + checkParam(linkSelf, Constants.PARAM_OFFSET, "10"); + checkParam(linkSelf, Constants.PARAM_COUNT, "10"); + + nextLink = bundle.getLink(IBaseBundle.LINK_NEXT).getUrl(); + assertNotNull(nextLink, "'next' link is not present"); + checkParam(nextLink, Constants.PARAM_OFFSET, "20"); + checkParam(nextLink, Constants.PARAM_COUNT, "10"); + } + try (CloseableHttpResponse resp = ourClient.execute(new HttpGet(nextLink))) { + assertEquals(200, resp.getStatusLine().getStatusCode()); + responseContent = IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8); + + Bundle bundle = ourContext.newJsonParser().parseResource(Bundle.class, responseContent); + assertEquals(10, bundle.getEntry().size()); + + String linkPrev = bundle.getLink(IBaseBundle.LINK_PREV).getUrl(); + assertNotNull(linkPrev, "'previous' link is not present"); + checkParam(linkPrev, Constants.PARAM_OFFSET, "10"); + checkParam(linkPrev, Constants.PARAM_COUNT, "10"); + + String linkSelf = bundle.getLink(IBaseBundle.LINK_SELF).getUrl(); + assertNotNull(linkSelf, "'self' link is not present"); + checkParam(linkSelf, Constants.PARAM_OFFSET, "20"); + checkParam(linkSelf, Constants.PARAM_COUNT, "10"); + + assertNull(bundle.getLink(IBaseBundle.LINK_NEXT)); + } + } + @Test() public void testSendingSameRequestConsecutivelyResultsInSameResponse() throws Exception { initBundleProvider(10); @@ -177,8 +250,8 @@ public void testSendingSameRequestConsecutivelyResultsInSameResponse() throws Ex assertEquals(0, bundle.getEntry().size()); } } - private void checkParam(String theUri, String theCheckedParam, String theExpectedValue) { - Optional paramValue = URLEncodedUtils.parse(theUri, CHARSET_UTF8).stream() + private void checkParam(String theUriString, String theCheckedParam, String theExpectedValue) { + Optional paramValue = URLEncodedUtils.parse(URI.create(theUriString), CHARSET_UTF8).stream() .filter(nameValuePair -> nameValuePair.getName().equals(theCheckedParam)) .map(NameValuePair::getValue) .findAny(); diff --git a/pom.xml b/pom.xml index 72888cfc0a87..827ed5ec2453 100644 --- a/pom.xml +++ b/pom.xml @@ -868,6 +868,11 @@ zachdoctolib Doctolib + + alparodev + Aleksej Parovysnik + Doctolib +