diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java index 8e898b99659..73f1a78f812 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java @@ -581,7 +581,7 @@ public class GenericClient extends BaseClient implements IGenericClient { } protected IResource parseResourceBody(String theResourceBody) { - EncodingEnum encoding = determineRawEncoding(theResourceBody); + EncodingEnum encoding = MethodUtil.detectEncodingNoDefault(theResourceBody); if (encoding == null) { throw new InvalidRequestException("FHIR client can't determine resource encoding"); } @@ -597,24 +597,6 @@ public class GenericClient extends BaseClient implements IGenericClient { } } - - /** - * Returns null if encoding can't be determined - */ - private static EncodingEnum determineRawEncoding(String theResourceBody) { - EncodingEnum encoding = null; - for (int i = 0; i < theResourceBody.length() && encoding == null; i++) { - switch (theResourceBody.charAt(i)) { - case '<': - encoding = EncodingEnum.XML; - break; - case '{': - encoding = EncodingEnum.JSON; - break; - } - } - return encoding; - } private final class BundleResponseHandler implements IClientResponseHandler { @@ -1579,7 +1561,7 @@ public class GenericClient extends BaseClient implements IGenericClient { public TransactionExecutable(String theBundle) { myRawBundle = theBundle; - myRawBundleEncoding = determineRawEncoding(myRawBundle); + myRawBundleEncoding = MethodUtil.detectEncodingNoDefault(myRawBundle); if (myRawBundleEncoding == null) { throw new IllegalArgumentException("Can not determine encoding of raw resource body"); } @@ -1603,7 +1585,7 @@ public class GenericClient extends BaseClient implements IGenericClient { * If the user has explicitly requested a given encoding, we may need to reencode the raw string */ if (getParamEncoding() != null) { - if (determineRawEncoding(myRawBundle) != getParamEncoding()) { + if (MethodUtil.detectEncodingNoDefault(myRawBundle) != getParamEncoding()) { IResource parsed = parseResourceBody(myRawBundle); myRawBundle = getParamEncoding().newParser(getFhirContext()).encodeResourceToString(parsed); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseMethodBinding.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseMethodBinding.java index 59ff899af92..15c2a40bf3b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseMethodBinding.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseMethodBinding.java @@ -167,6 +167,25 @@ public abstract class BaseMethodBinding implements IClientResponseHandler */ public abstract String getResourceName(); + /** + * Returns the value of {@link #getResourceOperationType()} or {@link #getSystemOperationType()} or {@link #getOtherOperationType()} + */ + public String getResourceOrSystemOperationType() { + Enum retVal = getResourceOperationType(); + if (retVal != null) { + return retVal.name(); + } + retVal = getSystemOperationType(); + if (retVal != null) { + return retVal.name(); + } + retVal = getOtherOperationType(); + if (retVal != null) { + return retVal.name(); + } + return null; + } + public abstract RestfulOperationTypeEnum getResourceOperationType(); public abstract RestfulOperationSystemEnum getSystemOperationType(); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBinding.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBinding.java index 9ca10643fce..1f187476b3e 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBinding.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBinding.java @@ -20,9 +20,12 @@ package ca.uhn.fhir.rest.method; * #L% */ +import static org.apache.commons.lang3.StringUtils.isBlank; + import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; +import java.io.StringReader; import java.io.Writer; import java.lang.reflect.Method; import java.util.Enumeration; @@ -32,6 +35,7 @@ import java.util.Set; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.IBaseResource; @@ -52,6 +56,7 @@ import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.RestfulServerUtils; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding { @@ -64,7 +69,8 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding> theHeaders) throws IOException, BaseServerResponseException { + public MethodOutcome invokeClient(String theResponseMimeType, Reader theResponseReader, int theResponseStatusCode, Map> theHeaders) throws IOException, + BaseServerResponseException { switch (theResponseStatusCode) { case Constants.STATUS_HTTP_200_OK: case Constants.STATUS_HTTP_201_CREATED: @@ -195,7 +201,8 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding wantedResourceType = requestContainsResourceType(); IResource retVal; if (wantedResourceType != null) { @@ -294,20 +331,20 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding requestContainsResourceType() { return null; } - protected void streamOperationOutcome(BaseServerResponseException theE, RestfulServer theServer, EncodingEnum theEncodingNotNull, HttpServletResponse theResponse, Request theRequest) throws IOException { + protected void streamOperationOutcome(BaseServerResponseException theE, RestfulServer theServer, EncodingEnum theEncodingNotNull, HttpServletResponse theResponse, Request theRequest) + throws IOException { theResponse.setStatus(theE.getStatusCode()); theServer.addHeadersToResponse(theResponse); - + if (theE.getOperationOutcome() != null) { theResponse.setContentType(theEncodingNotNull.getResourceContentType()); IParser parser = theEncodingNotNull.newParser(theServer.getFhirContext()); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/MethodUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/MethodUtil.java index 94e1e6bd305..91539d8d110 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/MethodUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/MethodUtil.java @@ -248,15 +248,26 @@ public class MethodUtil { } public static EncodingEnum detectEncoding(String theBody) { - for (int i = 0; i < theBody.length(); i++) { + EncodingEnum retVal = detectEncodingNoDefault(theBody); + if (retVal == null) { + retVal = EncodingEnum.XML; + } + return retVal; + } + + public static EncodingEnum detectEncodingNoDefault(String theBody) { + EncodingEnum retVal = null; + for (int i = 0; i < theBody.length() && retVal == null; i++) { switch (theBody.charAt(i)) { case '<': - return EncodingEnum.XML; + retVal = EncodingEnum.XML; + break; case '{': - return EncodingEnum.JSON; + retVal = EncodingEnum.JSON; + break; } } - return EncodingEnum.XML; + return retVal; } public static void extractDescription(SearchParameter theParameter, Annotation[] theAnnotations) { 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 27be55ace35..ee01ff51f00 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 @@ -195,9 +195,18 @@ public class RestfulServerUtils { } public static EncodingEnum determineRequestEncoding(Request theReq) { + EncodingEnum retVal = determineRequestEncodingNoDefault(theReq); + if (retVal != null) { + return retVal; + } + return EncodingEnum.XML; + } + + public static EncodingEnum determineRequestEncodingNoDefault(Request theReq) { + EncodingEnum retVal = null; Enumeration acceptValues = theReq.getServletRequest().getHeaders(Constants.HEADER_CONTENT_TYPE); if (acceptValues != null) { - while (acceptValues.hasMoreElements()) { + while (acceptValues.hasMoreElements() && retVal == null) { String nextAcceptHeaderValue = acceptValues.nextElement(); if (nextAcceptHeaderValue != null && isNotBlank(nextAcceptHeaderValue)) { for (String nextPart : nextAcceptHeaderValue.split(",")) { @@ -209,15 +218,15 @@ public class RestfulServerUtils { nextPart = nextPart.substring(0, scIdx); } nextPart = nextPart.trim(); - EncodingEnum retVal = Constants.FORMAT_VAL_TO_ENCODING.get(nextPart); + retVal = Constants.FORMAT_VAL_TO_ENCODING.get(nextPart); if (retVal != null) { - return retVal; + break; } } } } } - return EncodingEnum.XML; + return retVal; } public static String createPagingLink(Set theIncludes, String theServerBase, String theSearchId, int theOffset, int theCount, EncodingEnum theResponseEncoding, boolean thePrettyPrint) { diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index ed11a144c98..3be62aef4ce 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -12,6 +12,9 @@ ca.uhn.fhir.rest.client.GenericClient.cannotDetermineResourceTypeFromUri=Unable ca.uhn.fhir.rest.client.RestfulClientFactory.failedToRetrieveConformance=Failed to retrieve the server's metadata statement during client initialization. URL used was: {0} ca.uhn.fhir.rest.client.RestfulClientFactory.wrongVersionInConformance=The server at base URL "{0}" returned a conformance statement indicating that it supports FHIR version "{1}" which corresponds to {2}, but this client is configured to use {3} (via the FhirContext). +ca.uhn.fhir.rest.method.BaseOutcomeReturningMethodBinding.noContentTypeInRequest=No Content-Type header was provided in the request. This is required for "{0}" operation +ca.uhn.fhir.rest.method.BaseOutcomeReturningMethodBinding.invalidContentTypeInRequest=Incorrect Content-Type header value of "{0}" was provided in the request. A FHIR Content-Type is required for "{1}" operation + ca.uhn.fhir.rest.method.OperationMethodBinding.methodNotSupported=HTTP Method {0} is not allowed for this operation. Allowed method(s): {1} ca.uhn.fhir.rest.method.OperationParamBinder.urlParamNotPrimitive=Can not invoke operation {0} using HTTP GET because parameter {1} is not a primitive datatype ca.uhn.fhir.rest.method.IncludeParameter.invalidIncludeNameInRequest=Invalid {2} parameter value: "{0}". Valid values are: {1} diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java index 66a0ee07316..631139e28ed 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java @@ -9,12 +9,14 @@ import java.util.List; import java.util.concurrent.TimeUnit; import org.apache.commons.io.IOUtils; +import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ByteArrayEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; @@ -38,10 +40,13 @@ import ca.uhn.fhir.model.dstu.resource.Observation; import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; +import ca.uhn.fhir.rest.annotation.Create; import ca.uhn.fhir.rest.annotation.IdParam; import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.annotation.RequiredParam; +import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.client.IGenericClient; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringOrListParam; @@ -233,6 +238,75 @@ public class SearchTest { assertEquals("IDAAA (identifier123)", bundle.getEntries().get(0).getTitle().getValue()); } + /** + * See #164 + */ + @Test + public void testSearchByPostWithParamsInBodyAndUrl() throws Exception { + HttpPost filePost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?name=Central"); + + // add parameters to the post method + List parameters = new ArrayList(); + parameters.add(new BasicNameValuePair("_id", "aaa")); + + UrlEncodedFormEntity sendentity = new UrlEncodedFormEntity(parameters, "UTF-8"); + filePost.setEntity(sendentity); + + HttpResponse status = ourClient.execute(filePost); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + + Bundle bundle = ourCtx.newXmlParser().parseBundle(responseContent); + assertEquals(1, bundle.getEntries().size()); + + Patient p = bundle.getResources(Patient.class).get(0); + assertEquals("idaaa", p.getName().get(0).getFamilyAsSingleString()); + assertEquals("nameCentral", p.getName().get(1).getFamilyAsSingleString()); + + } + + /** + * See #164 + */ + @Test + public void testSearchByPostWithInvalidPostUrl() throws Exception { + HttpPost filePost = new HttpPost("http://localhost:" + ourPort + "/Patient?name=Central"); // should end with _search + + // add parameters to the post method + List parameters = new ArrayList(); + parameters.add(new BasicNameValuePair("_id", "aaa")); + + UrlEncodedFormEntity sendentity = new UrlEncodedFormEntity(parameters, "UTF-8"); + filePost.setEntity(sendentity); + + HttpResponse status = ourClient.execute(filePost); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + assertEquals(400, status.getStatusLine().getStatusCode()); + assertThat(responseContent, containsString("
")); + } + + /** + * See #164 + */ + @Test + public void testSearchByPostWithMissingContentType() throws Exception { + HttpPost filePost = new HttpPost("http://localhost:" + ourPort + "/Patient?name=Central"); // should end with _search + + HttpEntity sendentity = new ByteArrayEntity(new byte[] {1,2,3,4} ); + filePost.setEntity(sendentity); + + HttpResponse status = ourClient.execute(filePost); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + assertEquals(400, status.getStatusLine().getStatusCode()); + assertThat(responseContent, containsString("
")); + } + @Test public void testSearchCompartment() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123/fooCompartment"); @@ -354,6 +428,14 @@ public class SearchTest { */ public static class DummyPatientResourceProvider implements IResourceProvider { + /** + * Only needed for #164 + */ + @Create + public MethodOutcome create(@ResourceParam Patient thePatient) { + throw new IllegalArgumentException(); + } + @Search(compartmentName = "fooCompartment") public List compartment(@IdParam IdDt theId) { ArrayList retVal = new ArrayList(); @@ -383,7 +465,7 @@ public class SearchTest { @Search - public List findPatient(@RequiredParam(name = "_id") StringParam theParam) { + public List findPatient(@RequiredParam(name = "_id") StringParam theParam, @OptionalParam(name="name") StringParam theName) { ArrayList retVal = new ArrayList(); Patient patient = new Patient(); @@ -391,6 +473,7 @@ public class SearchTest { patient.addIdentifier("system", "identifier123"); if (theParam != null) { patient.addName().addFamily("id" + theParam.getValue()); + patient.addName().addFamily("name" + theName.getValue()); } retVal.add(patient); return retVal; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 3a0bf3342df..fb7ba36c1a9 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -183,6 +183,10 @@ default bean name expected in other parts of the Spring framework. Thanks to Mohammad Jafari for the suggestion! + + Improve error message when a user tries to perform a create/update with an invalid + or missing Content-Type header. +