From 3eb805fb0a696268015140108a7eb1d9519e3b96 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 23 Apr 2016 12:17:27 -0400 Subject: [PATCH] Respect _format=application/xml+fhir --- .../ca/uhn/fhir/rest/server/Constants.java | 29 +- .../ResponseHighlighterInterceptor.java | 22 +- .../ResponseHighlightingInterceptorTest.java | 68 +++++ .../rest/server/FormatParameterDstu3Test.java | 265 ++++++++++++++++++ src/changes/changes.xml | 5 + 5 files changed, 376 insertions(+), 13 deletions(-) create mode 100644 hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/FormatParameterDstu3Test.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java index cfa1bc0ca69..e49ad8e71b7 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java @@ -47,11 +47,16 @@ public class Constants { public static final String EXTOP_VALIDATE_MODE = "mode"; public static final String EXTOP_VALIDATE_PROFILE = "profile"; public static final String EXTOP_VALIDATE_RESOURCE = "resource"; + public static final String FORMAT_HTML = "html"; public static final String FORMAT_JSON = "json"; public static final Set FORMAT_VAL_JSON; public static final Map FORMAT_VAL_TO_ENCODING; public static final Set FORMAT_VAL_XML; public static final String FORMAT_XML = "xml"; + /** + * "text/html" and "html" + */ + public static final Set FORMATS_HTML; public static final String HEADER_ACCEPT = "Accept"; public static final String HEADER_ACCEPT_ENCODING = "Accept-Encoding"; public static final String HEADER_ACCEPT_VALUE_XML_OR_JSON = CT_FHIR_XML + ";q=1.0, " + CT_FHIR_JSON + ";q=1.0"; @@ -96,6 +101,11 @@ public class Constants { public static final String LINK_PREVIOUS = "previous"; public static final String LINK_SELF = "self"; public static final String OPENSEARCH_NS_OLDER = "http://purl.org/atompub/tombstones/1.0"; + /** + * Used in paging links + */ + public static final Object PARAM_BUNDLETYPE = "_bundletype"; + public static final String PARAM_CONTENT = "_content"; public static final String PARAM_COUNT = "_count"; public static final String PARAM_DELETE = "_delete"; public static final String PARAM_ELEMENTS = "_elements"; @@ -122,12 +132,13 @@ public class Constants { public static final String PARAM_SUMMARY = "_summary"; public static final String PARAM_TAG = "_tag"; public static final String PARAM_TAGS = "_tags"; + public static final String PARAM_TEXT = "_text"; public static final String PARAM_VALIDATE = "_validate"; public static final String PARAMQUALIFIER_MISSING = ":missing"; public static final String PARAMQUALIFIER_MISSING_FALSE = "false"; public static final String PARAMQUALIFIER_MISSING_TRUE = "true"; - public static final String PARAMQUALIFIER_STRING_EXACT = ":exact"; public static final String PARAMQUALIFIER_STRING_CONTAINS = ":contains"; + public static final String PARAMQUALIFIER_STRING_EXACT = ":exact"; public static final String PARAMQUALIFIER_TOKEN_TEXT = ":text"; public static final int STATUS_HTTP_200_OK = 200; public static final int STATUS_HTTP_201_CREATED = 201; @@ -147,20 +158,15 @@ public class Constants { public static final String TAG_SUBSETTED_CODE = "SUBSETTED"; public static final String TAG_SUBSETTED_SYSTEM = "http://hl7.org/fhir/v3/ObservationValue"; public static final String URL_TOKEN_HISTORY = "_history"; - public static final String URL_TOKEN_METADATA = "metadata"; - public static final String PARAM_CONTENT = "_content"; - public static final String PARAM_TEXT = "_text"; - /** - * Used in paging links - */ - public static final Object PARAM_BUNDLETYPE = "_bundletype"; + public static final String URL_TOKEN_METADATA = "metadata"; static { Map valToEncoding = new HashMap(); HashSet valXml = new HashSet(); valXml.add(CT_FHIR_XML); + valXml.add(CT_FHIR_XML.replace('+', ' ')); // See #346 valXml.add("text/xml"); valXml.add("application/xml"); valXml.add("xml"); @@ -171,6 +177,7 @@ public class Constants { HashSet valJson = new HashSet(); valJson.add(CT_FHIR_JSON); + valJson.add(CT_FHIR_JSON.replace('+', ' ')); // See #346 valJson.add("text/json"); valJson.add("application/json"); valJson.add("json"); @@ -248,6 +255,12 @@ public class Constants { statusNames.put(510, "Not Extended"); statusNames.put(511, "Network Authentication Required"); HTTP_STATUS_NAMES = Collections.unmodifiableMap(statusNames); + + Set formatsHtml = new HashSet(); + formatsHtml.add(CT_HTML); + formatsHtml.add(FORMAT_HTML); + FORMATS_HTML = Collections.unmodifiableSet(formatsHtml); + } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java index f7a890ca108..14fe353640b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java @@ -23,6 +23,8 @@ import static org.apache.commons.lang3.StringUtils.isBlank; */ import java.io.IOException; +import java.util.Arrays; +import java.util.List; import java.util.Set; import javax.servlet.ServletException; @@ -184,11 +186,21 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter { @Override public boolean outgoingResponse(RequestDetails theRequestDetails, IBaseResource theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse) throws AuthenticationException { + boolean force = false; + String[] formatParams = theRequestDetails.getParameters().get(Constants.PARAM_FORMAT); + if (formatParams != null) { + for (String next : formatParams) { + if (Constants.FORMATS_HTML.contains(next)) { + force = true; + } + } + } + /* * It's not a browser... */ Set highestRankedAcceptValues = RestfulServerUtils.parseAcceptHeaderAndReturnHighestRankedOptions(theServletRequest); - if (highestRankedAcceptValues.contains(Constants.CT_HTML) == false) { + if (!force && highestRankedAcceptValues.contains(Constants.CT_HTML) == false) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } @@ -196,21 +208,21 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter { * It's an AJAX request, so no HTML */ String requestedWith = theServletRequest.getHeader("X-Requested-With"); - if (requestedWith != null) { + if (!force && requestedWith != null) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } /* * Not a GET */ - if (theRequestDetails.getRequestType() != RequestTypeEnum.GET) { + if (!force && theRequestDetails.getRequestType() != RequestTypeEnum.GET) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } /* * Not binary */ - if ("Binary".equals(theRequestDetails.getResourceName())) { + if (!force && "Binary".equals(theRequestDetails.getResourceName())) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } @@ -218,7 +230,7 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter { * Request for _raw */ String[] rawParamValues = theRequestDetails.getParameters().get(PARAM_RAW); - if (rawParamValues != null && rawParamValues.length > 0 && rawParamValues[0].equals(PARAM_RAW_TRUE)) { + if (!force && rawParamValues != null && rawParamValues.length > 0 && rawParamValues[0].equals(PARAM_RAW_TRUE)) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java index 73e72f3f6ab..7e87c87c17c 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java @@ -234,6 +234,74 @@ public class ResponseHighlightingInterceptorTest { assertTrue(ic.outgoingResponse(reqDetails, resource, req, resp)); } + + /** + * See #346 + */ + @Test + public void testHighlightForceHtmlCt() throws Exception { + ResponseHighlighterInterceptor ic = new ResponseHighlighterInterceptor(); + + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeaders(Constants.HEADER_ACCEPT)).thenAnswer(new Answer>() { + @Override + public Enumeration answer(InvocationOnMock theInvocation) throws Throwable { + return new ArrayEnumeration("application/xml+fhir"); + } + }); + + HttpServletResponse resp = mock(HttpServletResponse.class); + StringWriter sw = new StringWriter(); + when(resp.getWriter()).thenReturn(new PrintWriter(sw)); + + Patient resource = new Patient(); + resource.addName().addFamily("FAMILY"); + + ServletRequestDetails reqDetails = new ServletRequestDetails(); + reqDetails.setRequestType(RequestTypeEnum.GET); + HashMap params = new HashMap(); + params.put(Constants.PARAM_FORMAT, new String[] { Constants.FORMAT_HTML }); + reqDetails.setParameters(params); + reqDetails.setServer(new RestfulServer(ourCtx)); + reqDetails.setServletRequest(req); + + // false means it decided to handle the request.. + assertFalse(ic.outgoingResponse(reqDetails, resource, req, resp)); + } + + /** + * See #346 + */ + @Test + public void testHighlightForceHtmlFormat() throws Exception { + ResponseHighlighterInterceptor ic = new ResponseHighlighterInterceptor(); + + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeaders(Constants.HEADER_ACCEPT)).thenAnswer(new Answer>() { + @Override + public Enumeration answer(InvocationOnMock theInvocation) throws Throwable { + return new ArrayEnumeration("application/xml+fhir"); + } + }); + + HttpServletResponse resp = mock(HttpServletResponse.class); + StringWriter sw = new StringWriter(); + when(resp.getWriter()).thenReturn(new PrintWriter(sw)); + + Patient resource = new Patient(); + resource.addName().addFamily("FAMILY"); + + ServletRequestDetails reqDetails = new ServletRequestDetails(); + reqDetails.setRequestType(RequestTypeEnum.GET); + HashMap params = new HashMap(); + params.put(Constants.PARAM_FORMAT, new String[] { Constants.CT_HTML }); + reqDetails.setParameters(params); + reqDetails.setServer(new RestfulServer(ourCtx)); + reqDetails.setServletRequest(req); + + // false means it decided to handle the request.. + assertFalse(ic.outgoingResponse(reqDetails, resource, req, resp)); + } @Test public void testHighlightNormalResponse() throws Exception { diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/FormatParameterDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/FormatParameterDstu3Test.java new file mode 100644 index 00000000000..062624d1b8f --- /dev/null +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/FormatParameterDstu3Test.java @@ -0,0 +1,265 @@ +package ca.uhn.fhir.rest.server; + +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.io.IOUtils; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.hl7.fhir.dstu3.model.IdType; +import org.hl7.fhir.dstu3.model.Patient; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; +import ca.uhn.fhir.model.api.annotation.ResourceDef; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.annotation.IdParam; +import ca.uhn.fhir.rest.annotation.Read; +import ca.uhn.fhir.util.PortUtil; +import ca.uhn.fhir.util.TestUtil; + +public class FormatParameterDstu3Test { + + private static final String VALUE_XML = ""; + private static final String VALUE_JSON = "{\"resourceType\":\"Patient\",\"id\":\"p1ReadId\",\"meta\":{\"profile\":[\"http://foo_profile\"]},\"identifier\":[{\"value\":\"p1ReadValue\"}]}"; + private static CloseableHttpClient ourClient; + private static FhirContext ourCtx = FhirContext.forDstu3(); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FormatParameterDstu3Test.class); + private static int ourPort; + private static Server ourServer; + private static RestfulServer ourServlet; + + /** + * See #346 + */ + @Test + public void testFormatXml() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=xml"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_XML, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationXml() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/xml"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_XML, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationXmlFhir() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/xml%2Bfhir"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_XML, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationXmlFhirUnescaped() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); + + // The plus isn't escaped here, and it should be.. but we'll be lenient + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/xml+fhir"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_XML, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatJson() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.XML); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=json"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_JSON, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationJson() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.XML); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/json"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_JSON, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationJsonFhir() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.XML); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/json%2Bfhir"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_JSON, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + /** + * See #346 + */ + @Test + public void testFormatApplicationJsonFhirUnescaped() throws Exception { + ourServlet.setDefaultResponseEncoding(EncodingEnum.XML); + + // The plus isn't escaped here, and it should be.. but we'll be lenient + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123?_format=application/json+fhir"); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(VALUE_JSON, responseContent); + } finally { + IOUtils.closeQuietly(status); + } + } + + @AfterClass + public static void afterClassClearContext() throws Exception { + ourServer.stop(); + TestUtil.clearAllStaticFieldsForUnitTest(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + + ourPort = PortUtil.findFreePort(); + ourServer = new Server(ourPort); + + DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider(); + + ServletHandler proxyHandler = new ServletHandler(); + ourServlet = new RestfulServer(ourCtx); + ourServlet.setFhirContext(ourCtx); + ourServlet.setResourceProviders(patientProvider); + ServletHolder servletHolder = new ServletHolder(ourServlet); + proxyHandler.addServletWithMapping(servletHolder, "/*"); + ourServer.setHandler(proxyHandler); + ourServer.start(); + + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); + HttpClientBuilder builder = HttpClientBuilder.create(); + builder.setConnectionManager(connectionManager); + ourClient = builder.build(); + + } + + public static class DummyPatientResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + @Read(version = true) + public Patient read(@IdParam IdType theId) { + Patient p1 = new MyPatient(); + p1.setId("p1ReadId"); + p1.addIdentifier().setValue("p1ReadValue"); + return p1; + } + + } + + @ResourceDef(name = "Patient", profile = "http://foo_profile") + public static class MyPatient extends Patient { + + private static final long serialVersionUID = 1L; + + } + +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 5d04e667c95..2e1804a8d63 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -20,6 +20,11 @@ Deprecate fluent client search operations without an explicit declaration of the bundle type being used + + Server now respects the parameter _format=application/xml+fhir"]]> + which is technically invalid since the + should be escaped, but is likely to be used. + Thanks to Jim Steel for reporting! +