Don't add false paging link to request

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java

@@ -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('&');

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java

@@ -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);
 			}
 		}
 

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchDstu3Test.java

@@ -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.*;
-import ca.uhn.fhir.util.TestUtil;
 
 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;
 		}
 

src/changes/changes.xml

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