diff --git a/hapi-fhir-base/pom.xml b/hapi-fhir-base/pom.xml index 6d81c1f4e78..e521a9c49bd 100644 --- a/hapi-fhir-base/pom.xml +++ b/hapi-fhir-base/pom.xml @@ -180,13 +180,28 @@ SITE + + org.apache.maven.plugins + maven-checkstyle-plugin + + + + checkstyle + + + + + + + + org.apache.maven.plugins maven-jxr-plugin diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeDeclaredChildDefinition.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeDeclaredChildDefinition.java index d1431fa8148..8747bc873e0 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeDeclaredChildDefinition.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeDeclaredChildDefinition.java @@ -107,12 +107,25 @@ public abstract class BaseRuntimeDeclaredChildDefinition extends BaseRuntimeChil } final Method accessor = BeanUtils.findAccessor(declaringClass, targetReturnType, elementName); if (accessor == null) { - throw new ConfigurationException("Could not find bean accessor/getter for property " + elementName + " on class " + declaringClass.getCanonicalName()); + StringBuilder b = new StringBuilder(); + b.append("Could not find bean accessor/getter for property "); + b.append(elementName); + b.append(" on class "); + b.append(declaringClass.getCanonicalName()); + throw new ConfigurationException(b.toString()); } final Method mutator = findMutator(declaringClass, targetReturnType, elementName); if (mutator == null) { - throw new ConfigurationException("Could not find bean mutator/setter for property " + elementName + " on class " + declaringClass.getCanonicalName() + " (expected return type " + targetReturnType.getCanonicalName() + ")"); + StringBuilder b = new StringBuilder(); + b.append("Could not find bean mutator/setter for property "); + b.append(elementName); + b.append(" on class "); + b.append(declaringClass.getCanonicalName()); + b.append(" (expected return type "); + b.append(targetReturnType.getCanonicalName()); + b.append(")"); + throw new ConfigurationException(b.toString()); } if (List.class.isAssignableFrom(targetReturnType)) { @@ -134,6 +147,7 @@ public abstract class BaseRuntimeDeclaredChildDefinition extends BaseRuntimeChil return myAccessor; } + @Override public String getElementName() { return myElementName; } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/FhirContext.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/FhirContext.java index eecff3f641a..330f02d5636 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/FhirContext.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/FhirContext.java @@ -130,7 +130,11 @@ public class FhirContext { throw new IllegalStateException(getLocalizer().getMessage(FhirContext.class, "noStructures")); } - ourLog.info("Creating new FHIR context for FHIR version [{}]", myVersion.getVersion().name()); + if (theVersion == null) { + ourLog.info("Creating new FhirContext with auto-detected version [{}]. It is recommended to explicitly select a version for future compatibility by invoking FhirContext.forDstuX()", myVersion.getVersion().name()); + } else { + ourLog.info("Creating new FHIR context for FHIR version [{}]", myVersion.getVersion().name()); + } scanResourceTypes(toElementList(theResourceTypes)); } 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 72cd4ba7599..2967aae6a84 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 @@ -32,18 +32,22 @@ public class Constants { public static final Charset CHARSET_UTF8; public static final String CHARSETNAME_UTF_8 = "UTF-8"; public static final String CT_ATOM_XML = "application/atom+xml"; - public static final String CT_FHIR_JSON = "application/json+fhir"; + + public static final String CTSUFFIX_CHARSET_UTF8 = "; charset=" + CHARSETNAME_UTF_8; public static final String CT_FHIR_XML = "application/xml+fhir"; public static final String CT_HTML = "text/html"; - public static final String CT_HTML_WITH_UTF8 = "text/html; charset=UTF-8"; + public static final String CT_HTML_WITH_UTF8 = "text/html" + CTSUFFIX_CHARSET_UTF8; public static final String CT_JSON = "application/json"; public static final String CT_OCTET_STREAM = "application/octet-stream"; public static final String CT_TEXT = "text/plain"; - public static final String CT_TEXT_WITH_UTF8 = CT_TEXT + "; charset=UTF-8"; + public static final String CT_TEXT_WITH_UTF8 = CT_TEXT + CTSUFFIX_CHARSET_UTF8; public static final String CT_XML = "application/xml"; public static final String ENCODING_GZIP = "gzip"; public static final String EXTOP_VALIDATE = "$validate"; + 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_JSON = "json"; public static final Set FORMAT_VAL_JSON; public static final Map FORMAT_VAL_TO_ENCODING; @@ -128,9 +132,6 @@ public class Constants { public static final int STATUS_HTTP_501_NOT_IMPLEMENTED = 501; public static final String URL_TOKEN_HISTORY = "_history"; public static final String URL_TOKEN_METADATA = "metadata"; - public static final String EXTOP_VALIDATE_MODE = "mode"; - public static final String EXTOP_VALIDATE_PROFILE = "profile"; - public static final String EXTOP_VALIDATE_RESOURCE = "resource"; static { Map valToEncoding = new HashMap(); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index b938514ae90..ad34aec2797 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -600,7 +600,6 @@ public class RestfulServer extends HttpServlet { operation = Constants.PARAM_HISTORY; } } else if (partIsOperation(nextString)) { - // FIXME: this would be untrue for _meta/_delete if (operation != null) { throw new InvalidRequestException("URL Path contains two operations: " + requestPath); } @@ -635,8 +634,6 @@ public class RestfulServer extends HttpServlet { requestDetails.setSecondaryOperation(secondaryOperation); requestDetails.setCompartmentName(compartment); - // TODO: look for more tokens for version, compartments, etc... - String acceptEncoding = theRequest.getHeader(Constants.HEADER_ACCEPT_ENCODING); boolean respondGzip = false; if (acceptEncoding != null) { @@ -819,7 +816,6 @@ public class RestfulServer extends HttpServlet { * (which extends {@link ServletException}), as this is a flag to the servlet container that the servlet * is not usable. */ - @SuppressWarnings("unused") protected void initialize() throws ServletException { // nothing by default } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java index 7dcde66f03a..2eeaa007567 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java @@ -32,6 +32,8 @@ import java.util.Enumeration; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.zip.GZIPOutputStream; import javax.servlet.ServletOutputStream; @@ -59,6 +61,8 @@ import ca.uhn.fhir.rest.method.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; 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*)?(,?)"); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RestfulServerUtils.class); public static void addProfileToBundleEntry(FhirContext theContext, IBaseResource theResource, String theServerBase) { @@ -188,27 +192,51 @@ public class RestfulServerUtils { } } + /* + * The Accept header is kind of ridiculous, e.g. + */ + // text/xml, application/xml, application/xhtml+xml, text/html;q=0.9, text/plain;q=0.8, image/png, */*;q=0.5 + Enumeration acceptValues = theReq.getHeaders(Constants.HEADER_ACCEPT); if (acceptValues != null) { + float bestQ = -1f; + EncodingEnum retVal = null; while (acceptValues.hasMoreElements()) { String nextAcceptHeaderValue = acceptValues.nextElement(); - if (nextAcceptHeaderValue != null && isNotBlank(nextAcceptHeaderValue)) { - for (String nextPart : nextAcceptHeaderValue.split(",")) { - int scIdx = nextPart.indexOf(';'); - if (scIdx == 0) { - continue; - } - if (scIdx != -1) { - nextPart = nextPart.substring(0, scIdx); - } - nextPart = nextPart.trim(); - EncodingEnum retVal = Constants.FORMAT_VAL_TO_ENCODING.get(nextPart); - if (retVal != null) { - return retVal; + Matcher m = ACCEPT_HEADER_PATTERN.matcher(nextAcceptHeaderValue); + float q = 1.0f; + while (m.find()) { + String contentTypeGroup = m.group(1); + EncodingEnum encoding = Constants.FORMAT_VAL_TO_ENCODING.get(contentTypeGroup); + if (encoding != null) { + + String name = m.group(3); + String value = m.group(4); + if (name != null && value != null) { + if ("q".equals(name)) { + try { + q = Float.parseFloat(value); + q = Math.max(q, 0.0f); + } catch (NumberFormatException e) { + ourLog.debug("Invalid Accept header q value: {}", value); + } + } } } + + if (q > bestQ) { + retVal = encoding; + bestQ = q; + } + + if (!",".equals(m.group(5))) { + break; + } } + } + + return retVal; } return null; } diff --git a/hapi-fhir-jpaserver-base/.gitignore b/hapi-fhir-jpaserver-base/.gitignore index b53df3969af..eaa09ffd910 100644 --- a/hapi-fhir-jpaserver-base/.gitignore +++ b/hapi-fhir-jpaserver-base/.gitignore @@ -129,3 +129,4 @@ local.properties /target/ /target/ +/target/ diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/Dstu1EnvTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/Dstu1EnvTest.java new file mode 100644 index 00000000000..594c887c060 --- /dev/null +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/Dstu1EnvTest.java @@ -0,0 +1,17 @@ +package ca.uhn.fhir; + +import static org.junit.Assert.*; + +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; + +public class Dstu1EnvTest { + + @Test + public void testCorrectDefault() { + FhirContext ctx = new FhirContext(); + assertEquals("new FhirContext() is creating a context with the wrong FHIR versions. Something is probably wrong with the classpath.", FhirVersionEnum.DSTU1, ctx.getVersion().getVersion()); + } +} diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/AcceptHeaderTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/AcceptHeaderTest.java new file mode 100644 index 00000000000..c6882fd4d26 --- /dev/null +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/AcceptHeaderTest.java @@ -0,0 +1,135 @@ +package ca.uhn.fhir.rest.server; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.net.URLEncoder; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.io.IOUtils; +import org.apache.http.Header; +import org.apache.http.HttpResponse; +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.junit.AfterClass; +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.dstu.composite.IdentifierDt; +import ca.uhn.fhir.model.dstu.resource.Organization; +import ca.uhn.fhir.model.dstu.resource.Patient; +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; + +/** + * Created by dsotnikov on 2/25/2014. + */ +public class AcceptHeaderTest { + + private static CloseableHttpClient ourClient; + private static int ourPort; + private static Server ourServer; + + @Test + public void testReadNoHeader() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); + HttpResponse status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(Constants.CT_FHIR_XML + Constants.CTSUFFIX_CHARSET_UTF8, status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue()); + } + + @Test + public void testReadXmlHeaderHigherPriority() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); + httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_XML + "; q=1.0, " + Constants.CT_FHIR_JSON + "; q=0.9"); + HttpResponse status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(Constants.CT_FHIR_XML + Constants.CTSUFFIX_CHARSET_UTF8, status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue()); + } + + @Test + public void testReadXmlHeaderHigherPriorityWithStar() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); + httpGet.addHeader(Constants.HEADER_ACCEPT, "*/*; q=1.0, " + Constants.CT_FHIR_XML + "; q=1.0, " + Constants.CT_FHIR_JSON + "; q=0.9"); + HttpResponse status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(Constants.CT_FHIR_XML + Constants.CTSUFFIX_CHARSET_UTF8, status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue()); + } + + @Test + public void testReadXmlHeaderHigherPriorityBackwards() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); + httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_JSON + "; q=0.9, " + Constants.CT_FHIR_XML + "; q=1.0"); + HttpResponse status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(Constants.CT_FHIR_XML + Constants.CTSUFFIX_CHARSET_UTF8, status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue()); + + // Now with spaces + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); + httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_JSON + "; q = 000000.9 , " + Constants.CT_FHIR_XML + " ; q = 1.0 ,"); + status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(Constants.CT_FHIR_XML + Constants.CTSUFFIX_CHARSET_UTF8, status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue()); + } + + @AfterClass + public static void afterClass() throws Exception { + ourServer.stop(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + ourPort = PortUtil.findFreePort(); + ourServer = new Server(ourPort); + + ServletHandler proxyHandler = new ServletHandler(); + RestfulServer servlet = new RestfulServer(); + servlet.setResourceProviders(new PatientProvider()); + servlet.setDefaultResponseEncoding(EncodingEnum.XML); + ServletHolder servletHolder = new ServletHolder(servlet); + 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(); + + } + + /** + * Created by dsotnikov on 2/25/2014. + */ + public static class PatientProvider implements IResourceProvider { + + @Read(version = true) + public Patient read(@IdParam IdDt theId) { + Patient patient = new Patient(); + patient.addIdentifier(theId.getIdPart(), theId.getVersionIdPart()); + patient.setId("Patient/1/_history/1"); + return patient; + } + + @Override + public Class getResourceType() { + return Patient.class; + } + + } + +} diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/RestfulServerUtilsTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/RestfulServerUtilsTest.java new file mode 100644 index 00000000000..ea449f6fa1b --- /dev/null +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/RestfulServerUtilsTest.java @@ -0,0 +1,19 @@ +package ca.uhn.fhir.rest.server; + +import static org.junit.Assert.*; + +import java.util.regex.Matcher; + +import org.junit.Test; + +public class RestfulServerUtilsTest { + + @Test + public void testAcceptPattern() { + Matcher m = RestfulServerUtils.ACCEPT_HEADER_PATTERN.matcher("application/json+fhir"); + assertTrue(m.find()); + assertEquals("application/json+fhir", m.group(0)); + assertEquals("application/json+fhir", m.group(1)); + } + +} diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ServerFeaturesTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ServerFeaturesTest.java index 4a7ff63c8d9..90754d15272 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ServerFeaturesTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ServerFeaturesTest.java @@ -88,7 +88,7 @@ public class ServerFeaturesTest { } @Test - public void testAcceptHeader() throws Exception { + public void testAcceptHeaderXml() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1"); httpGet.addHeader("Accept", Constants.CT_FHIR_XML); HttpResponse status = ourClient.execute(httpGet); @@ -97,25 +97,34 @@ public class ServerFeaturesTest { assertThat(responseContent, StringContains.containsString(" 1.6 1.6 - + javac-with-errorprone + true @@ -258,7 +258,6 @@ true random - -Dfile.encoding=UTF-8 @@ -426,6 +425,21 @@ + + org.apache.maven.plugins + maven-checkstyle-plugin + 2.15 + + + com.puppycrawl.tools + checkstyle + 6.7 + + + + src/checkstyle/checkstyle.xml + + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7040037a3bb..e7afb56b70e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -95,6 +95,10 @@ in DSTU2 mode. Thanks to Eric from the FHIR Skype Implementers chat for reporting. + + Server now supports complete Accept header content negotiation, including + q values specifying order of preference. Previously the q value was ignored. + diff --git a/src/checkstyle/checkstyle.xml b/src/checkstyle/checkstyle.xml new file mode 100644 index 00000000000..4deedf36ebc --- /dev/null +++ b/src/checkstyle/checkstyle.xml @@ -0,0 +1,204 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +