Don't add false paging link to request

This commit is contained in:
James Agnew 2017-07-13 11:48:10 -04:00
parent bd4e1d3388
commit 3196db96d1
4 changed files with 219 additions and 10 deletions

View File

@ -21,6 +21,7 @@ package ca.uhn.fhir.rest.server;
*/
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.StringUtils.replace;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
@ -70,6 +71,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.method.ElementsParameter;
import ca.uhn.fhir.rest.server.method.SummaryEnumParameter;
import ca.uhn.fhir.util.DateUtils;
import ca.uhn.fhir.util.UrlUtil;
public class RestfulServerUtils {
static final Pattern ACCEPT_HEADER_PATTERN = Pattern.compile("\\s*([a-zA-Z0-9+.*/-]+)\\s*(;\\s*([a-zA-Z]+)\\s*=\\s*([a-zA-Z0-9.]+)\\s*)?(,?)");
@ -118,7 +120,7 @@ public class RestfulServerUtils {
}
}
public static String createPagingLink(Set<Include> theIncludes, String theServerBase, String theSearchId, int theOffset, int theCount, EncodingEnum theResponseEncoding, boolean thePrettyPrint,
public static String createPagingLink(Set<Include> theIncludes, String theServerBase, String theSearchId, int theOffset, int theCount, Map<String, String[]> theRequestParameters, boolean thePrettyPrint,
BundleTypeEnum theBundleType) {
try {
StringBuilder b = new StringBuilder();
@ -136,11 +138,14 @@ public class RestfulServerUtils {
b.append(Constants.PARAM_COUNT);
b.append('=');
b.append(theCount);
if (theResponseEncoding != null) {
String[] strings = theRequestParameters.get(Constants.PARAM_FORMAT);
if (strings != null && strings.length > 0) {
b.append('&');
b.append(Constants.PARAM_FORMAT);
b.append('=');
b.append(theResponseEncoding.getRequestContentType());
String format = strings[0];
format = replace(format, " ", "+");
b.append(UrlUtil.escape(format));
}
if (thePrettyPrint) {
b.append('&');

View File

@ -349,11 +349,11 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
String linkNext = null;
if (searchId != null) {
if (numTotalResults == null || theOffset + numToReturn < numTotalResults) {
linkNext = (RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, theOffset + numToReturn, numToReturn, theLinkEncoding, prettyPrint, theBundleType));
linkNext = (RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, theOffset + numToReturn, numToReturn, theRequest.getParameters(), prettyPrint, theBundleType));
}
if (theOffset > 0) {
int start = Math.max(0, theOffset - theLimit);
linkPrev = RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, start, theLimit, theLinkEncoding, prettyPrint, theBundleType);
linkPrev = RestfulServerUtils.createPagingLink(theIncludes, serverBase, searchId, start, theLimit, theRequest.getParameters(), prettyPrint, theBundleType);
}
}

View File

@ -1,16 +1,20 @@
package ca.uhn.fhir.rest.server;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import ca.uhn.fhir.rest.api.Constants;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
@ -26,14 +30,14 @@ import org.junit.*;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.rest.annotation.RequiredParam;
import ca.uhn.fhir.rest.annotation.Search;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.api.SearchStyleEnum;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
import ca.uhn.fhir.rest.gclient.StringClientParam;
import ca.uhn.fhir.rest.param.TokenAndListParam;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.util.PortUtil;
import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.util.*;
public class SearchDstu3Test {
@ -80,7 +84,7 @@ public class SearchDstu3Test {
ourLog.info(responseContent);
assertEquals(400, status.getStatusLine().getStatusCode());
OperationOutcome oo = (OperationOutcome) ourCtx.newXmlParser().parseResource(responseContent);
OperationOutcome oo = (OperationOutcome) ourCtx.newJsonParser().parseResource(responseContent);
assertEquals(
"Invalid search parameter \"identifier.chain\". Parameter contains a chain (.chain) and chains are not supported for this parameter (chaining is only allowed on reference parameters)",
oo.getIssueFirstRep().getDiagnostics());
@ -90,6 +94,191 @@ public class SearchDstu3Test {
}
@Test
public void testPagingPreservesEncodingJson() throws Exception {
HttpGet httpGet;
String linkNext;
Bundle bundle;
// Initial search
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar&_format=json");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=json"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=json"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=json"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=json"));
}
@Test
public void testPagingPreservesEncodingApplicationJsonFhir() throws Exception {
HttpGet httpGet;
String linkNext;
Bundle bundle;
// Initial search
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar&_format=" + Constants.CT_FHIR_JSON_NEW);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=" + UrlUtil.escape(Constants.CT_FHIR_JSON_NEW)));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=" + UrlUtil.escape(Constants.CT_FHIR_JSON_NEW)));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=" + UrlUtil.escape(Constants.CT_FHIR_JSON_NEW)));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=" + UrlUtil.escape(Constants.CT_FHIR_JSON_NEW)));
}
@Test
public void testPagingPreservesEncodingXml() throws Exception {
HttpGet httpGet;
String linkNext;
Bundle bundle;
// Initial search
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar&_format=xml");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=xml"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=xml"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=xml"));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, containsString("_format=xml"));
}
@Test
public void testPagingPreservesEncodingNone() throws Exception {
HttpGet httpGet;
String linkNext;
Bundle bundle;
// Initial search
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.JSON);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
}
@Test
public void testPagingPreservesEncodingNoneWithBrowserAcceptHeader() throws Exception {
HttpGet httpGet;
String linkNext;
Bundle bundle;
// Initial search
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar");
httpGet.addHeader(Constants.HEADER_ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
httpGet.addHeader(Constants.HEADER_ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
httpGet.addHeader(Constants.HEADER_ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
// Fetch the next page
httpGet = new HttpGet(linkNext);
httpGet.addHeader(Constants.HEADER_ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8");
bundle = executeAndReturnLinkNext(httpGet, EncodingEnum.XML);
linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertThat(linkNext, not(containsString("_format")));
}
private Bundle executeAndReturnLinkNext(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException, ClientProtocolException {
CloseableHttpResponse status = ourClient.execute(httpGet);
Bundle bundle;
try {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
ourLog.info(responseContent);
assertEquals(200, status.getStatusLine().getStatusCode());
EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim());
assertEquals(theExpectEncoding, ct);
bundle = ct.newParser(ourCtx).parseResource(Bundle.class, responseContent);
assertEquals(10, bundle.getEntry().size());
String linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertNotNull(linkNext);
} finally {
IOUtils.closeQuietly(status.getEntity().getContent());
}
return bundle;
}
@Test
public void testSearchWithPostAndInvalidParameters() throws Exception {
IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort);
@ -133,6 +322,7 @@ public class SearchDstu3Test {
ServletHandler proxyHandler = new ServletHandler();
RestfulServer servlet = new RestfulServer(ourCtx);
servlet.setDefaultResponseEncoding(EncodingEnum.JSON);
servlet.setPagingProvider(new FifoMemoryPagingProvider(10));
servlet.setResourceProviders(patientProvider);
@ -162,7 +352,13 @@ public class SearchDstu3Test {
ourLastMethod = "search";
ourIdentifiers = theIdentifiers;
ArrayList<Patient> retVal = new ArrayList<Patient>();
retVal.add((Patient) new Patient().addName(new HumanName().setFamily("FAMILY")).setId("1"));
for (int i = 0; i < 200; i++) {
Patient patient = new Patient();
patient.addName(new HumanName().setFamily("FAMILY"));
patient.getIdElement().setValue("Patient/" + i);
retVal.add((Patient) patient);
}
return retVal;
}

View File

@ -153,6 +153,14 @@
is often the case in more recent versions of HAPI). Thanks to Sujay R
for reporting!
</action>
<action type="fix">
When the server was returning a multi-page search result where the
client did not explicitly request an encoding via the _format
parameter, a _format parameter was incorrectly added to the paging
links in the response Bundle. This would often explicitly request
XML encoding because of the browser Accept header even though
this was not what the client wanted.
</action>
</release>
<release version="2.5" date="2017-06-08">
<action type="fix">