honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix #4- Create operation should return HTTP 200

This commit is contained in:
James Agnew 2014-08-05 10:46:46 -04:00
parent 33d667d448
commit 1793f6d9ab
8 changed files with 128 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
hapi-fhir-base/src/changes/changes.xml
View File

@ -8,7 +8,16 @@
<body>
<release version="0.6" date="TBD">
<action type="add">
Allow generic client
Allow generic client ... OAUTH
</action>
<action type="fix">
Tester UI created double _format and _pretty param entries in searches. Thanks to Gered King of University
Health Network for reporting!
</action>
<action type="fix" issue="4">
Create method was incorrectly returning an HTTP 204 on sucessful completion, but
should be returning an HTTP 200 per the FHIR specification. Thanks to wanghaisheng
for reporting!
</action>
</release>
<release version="0.5" date="2014-Jul-30">

8
hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java
View File

@ -821,10 +821,10 @@ public class GenericClient extends BaseClient implements IGenericClient {
public Bundle execute() {
Map<String, List<String>> params = new LinkedHashMap<String, List<String>>();
Map<String, List<String>> initial = createExtraParams();
if (initial != null) {
params.putAll(initial);
}
// Map<String, List<String>> initial = createExtraParams();
// if (initial != null) {
// params.putAll(initial);
// }
for (ICriterionInternal next : myCriterion) {
String parameterName = next.getParameterName();

113
hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBinding.java
View File

@ -65,7 +65,8 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
if (!theMethod.getReturnType().equals(MethodOutcome.class)) {
if (!allowVoidReturnType()) {
throw new ConfigurationException("Method " + theMethod.getName() + " in type " + theMethod.getDeclaringClass().getCanonicalName() + " is a @" + theMethodAnnotation.getSimpleName() + " method but it does not return " + MethodOutcome.class);
throw new ConfigurationException("Method " + theMethod.getName() + " in type " + theMethod.getDeclaringClass().getCanonicalName() + " is a @" + theMethodAnnotation.getSimpleName()
+ " method but it does not return " + MethodOutcome.class);
} else if (theMethod.getReturnType() == void.class) {
myReturnVoid = true;
}
@ -92,7 +93,8 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
}
@Override
public MethodOutcome invokeClient(String theResponseMimeType, Reader theResponseReader, int theResponseStatusCode, Map<String, List<String>> theHeaders) throws IOException, BaseServerResponseException {
public MethodOutcome invokeClient(String theResponseMimeType, Reader theResponseReader, int theResponseStatusCode, Map<String, List<String>> theHeaders) throws IOException,
BaseServerResponseException {
switch (theResponseStatusCode) {
case Constants.STATUS_HTTP_200_OK:
case Constants.STATUS_HTTP_201_CREATED:
@ -148,26 +150,40 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
throw new InternalErrorException("Server method returned invalid resource ID: " + response.getId().getValue());
}
}
if (getResourceOperationType() == RestfulOperationTypeEnum.CREATE) {
switch (getResourceOperationType()) {
case CREATE:
if (response == null) {
throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null, which is not allowed for create operation");
throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName()
+ " returned null, which is not allowed for create operation");
}
theResponse.setStatus(Constants.STATUS_HTTP_201_CREATED);
addLocationHeader(theRequest, theResponse, response);
} else if (response == null) {
if (isReturnVoid() == false) {
throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null");
}
theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT);
} else {
if (response.getOperationOutcome() == null) {
break;
case UPDATE:
theResponse.setStatus(Constants.STATUS_HTTP_200_OK);
addLocationHeader(theRequest, theResponse, response);
break;
case VALIDATE:
case DELETE:
default:
if (response == null) {
if (isReturnVoid() == false) {
throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null");
}
theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT);
} else {
theResponse.setStatus(Constants.STATUS_HTTP_200_OK);
}
if (getResourceOperationType() == RestfulOperationTypeEnum.UPDATE) {
addLocationHeader(theRequest, theResponse, response);
if (response.getOperationOutcome() == null) {
theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT);
} else {
theResponse.setStatus(Constants.STATUS_HTTP_200_OK);
}
if (getResourceOperationType() == RestfulOperationTypeEnum.UPDATE) {
addLocationHeader(theRequest, theResponse, response);
}
}
}
@ -200,7 +216,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
}
/**
* @throws IOException
* @throws IOException
*/
protected IResource parseIncomingServerResource(Request theRequest) throws IOException {
EncodingEnum encoding = RestfulServer.determineRequestEncoding(theRequest);
@ -210,48 +226,27 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
}
/*
* @Override public void invokeServer(RestfulServer theServer, Request
* theRequest, HttpServletResponse theResponse) throws
* BaseServerResponseException, IOException { Object[] params = new
* Object[getParameters().size()]; for (int i = 0; i <
* getParameters().size(); i++) { IParameter param = getParameters().get(i);
* if (param != null) { params[i] =
* @Override public void invokeServer(RestfulServer theServer, Request theRequest, HttpServletResponse theResponse) throws BaseServerResponseException, IOException { Object[] params = new
* Object[getParameters().size()]; for (int i = 0; i < getParameters().size(); i++) { IParameter param = getParameters().get(i); if (param != null) { params[i] =
* param.translateQueryParametersIntoServerArgument(theRequest, null); } }
*
* addParametersForServerRequest(theRequest, params);
*
* MethodOutcome response = (MethodOutcome)
* invokeServerMethod(getProvider(), params);
* MethodOutcome response = (MethodOutcome) invokeServerMethod(getProvider(), params);
*
* if (response == null) { if (myReturnVoid == false) { throw new
* ConfigurationException("Method " + getMethod().getName() + " in type " +
* getMethod().getDeclaringClass().getCanonicalName() + " returned null"); }
* else { theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT); } }
* else if (!myReturnVoid) { if (response.isCreated()) {
* theResponse.setStatus(Constants.STATUS_HTTP_201_CREATED); StringBuilder b
* = new StringBuilder(); b.append(theRequest.getFhirServerBase());
* b.append('/'); b.append(getResourceName()); b.append('/');
* b.append(response.getId().getValue()); if (response.getVersionId() !=
* null && response.getVersionId().isEmpty() == false) {
* b.append("/_history/"); b.append(response.getVersionId().getValue()); }
* theResponse.addHeader("Location", b.toString()); } else {
* theResponse.setStatus(Constants.STATUS_HTTP_200_OK); } } else {
* if (response == null) { if (myReturnVoid == false) { throw new ConfigurationException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() +
* " returned null"); } else { theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT); } } else if (!myReturnVoid) { if (response.isCreated()) {
* theResponse.setStatus(Constants.STATUS_HTTP_201_CREATED); StringBuilder b = new StringBuilder(); b.append(theRequest.getFhirServerBase()); b.append('/'); b.append(getResourceName());
* b.append('/'); b.append(response.getId().getValue()); if (response.getVersionId() != null && response.getVersionId().isEmpty() == false) { b.append("/_history/");
* b.append(response.getVersionId().getValue()); } theResponse.addHeader("Location", b.toString()); } else { theResponse.setStatus(Constants.STATUS_HTTP_200_OK); } } else {
* theResponse.setStatus(Constants.STATUS_HTTP_204_NO_CONTENT); }
*
* theServer.addHeadersToResponse(theResponse);
*
* Writer writer = theResponse.getWriter(); try { if (response != null) {
* OperationOutcome outcome = new OperationOutcome(); if
* (response.getOperationOutcome() != null &&
* response.getOperationOutcome().getIssue() != null) {
* outcome.getIssue().addAll(response.getOperationOutcome().getIssue()); }
* EncodingUtil encoding =
* BaseMethodBinding.determineResponseEncoding(theRequest
* .getServletRequest(), theRequest.getParameters());
* theResponse.setContentType(encoding.getResourceContentType()); IParser
* parser = encoding.newParser(getContext());
* parser.encodeResourceToWriter(outcome, writer); } } finally {
* writer.close(); } // getMethod().in }
* Writer writer = theResponse.getWriter(); try { if (response != null) { OperationOutcome outcome = new OperationOutcome(); if (response.getOperationOutcome() != null &&
* response.getOperationOutcome().getIssue() != null) { outcome.getIssue().addAll(response.getOperationOutcome().getIssue()); } EncodingUtil encoding =
* BaseMethodBinding.determineResponseEncoding(theRequest .getServletRequest(), theRequest.getParameters()); theResponse.setContentType(encoding.getResourceContentType()); IParser parser =
* encoding.newParser(getContext()); parser.encodeResourceToWriter(outcome, writer); } } finally { writer.close(); } // getMethod().in }
*/
public boolean isReturnVoid() {
@ -266,10 +261,10 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
b.append('/');
b.append(response.getId().getIdPart());
if (response.getId().hasVersionIdPart()) {
b.append("/"+Constants.PARAM_HISTORY+"/");
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getId().getVersionIdPart());
}else if (response.getVersionId() != null && response.getVersionId().isEmpty() == false) {
b.append("/"+Constants.PARAM_HISTORY+"/");
} else if (response.getVersionId() != null && response.getVersionId().isEmpty() == false) {
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getVersionId().getValue());
}
theResponse.addHeader(Constants.HEADER_LOCATION, b.toString());
@ -350,8 +345,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
protected abstract void addParametersForServerRequest(Request theRequest, Object[] theParams);
/**
* Subclasses may override to allow a void method return type, which is
* allowable for some methods (e.g. delete)
* Subclasses may override to allow a void method return type, which is allowable for some methods (e.g. delete)
*/
protected boolean allowVoidReturnType() {
return false;
@ -360,17 +354,14 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
protected abstract BaseHttpClientInvocation createClientInvocation(Object[] theArgs, IResource resource);
/**
* For servers, this method will match only incoming requests that match the
* given operation, or which have no operation in the URL if this method
* returns null.
* For servers, this method will match only incoming requests that match the given operation, or which have no operation in the URL if this method returns null.
*/
protected abstract String getMatchingOperation();
protected abstract Set<RequestType> provideAllowableRequestTypes();
/**
* Subclasses may override if the incoming request should not contain a
* resource
* Subclasses may override if the incoming request should not contain a resource
*/
protected boolean requestContainsResource() {
return true;
@ -406,7 +397,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
if (StringUtils.isBlank(theLocationHeader)) {
return;
}
theOutcomeToPopulate.setId(new IdDt(theLocationHeader));
String resourceNamePart = "/" + theResourceName + "/";

28
hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/client/GenericClientTest.java
View File

@ -49,6 +49,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.client.exceptions.NonFhirResponseException;
import ca.uhn.fhir.rest.method.SearchStyleEnum;
import ca.uhn.fhir.rest.server.Constants;
import ca.uhn.fhir.rest.server.EncodingEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
@ -343,6 +344,33 @@ public class GenericClientTest {
}
@SuppressWarnings("unused")
@Test
public void testSearchWithClientEncodingAndPrettyPrintConfig() throws Exception {
String msg = getPatientFeedWithOneResult();
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK"));
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(msg), Charset.forName("UTF-8")));
GenericClient client = (GenericClient) myCtx.newRestfulGenericClient("http://example.com/fhir");
client.setPrettyPrint(true);
client.setEncoding(EncodingEnum.JSON);
//@formatter:off
Bundle response = client.search()
.forResource(Patient.class)
.execute();
//@formatter:on
assertEquals("http://example.com/fhir/Patient?_format=json&_pretty=true", capt.getValue().getURI().toString());
}
@SuppressWarnings("unused")
@Test
public void testSearchByDate() throws Exception {

33
hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/UpdateTest.java
View File

@ -104,9 +104,10 @@ public class UpdateTest {
HttpResponse status = ourClient.execute(httpPost);
assertEquals(204, status.getStatusLine().getStatusCode());
assertEquals(200, status.getStatusLine().getStatusCode());
assertEquals("http://localhost:" + ourPort + "/DiagnosticReport/001/_history/002", status.getFirstHeader("location").getValue());
IOUtils.closeQuietly(status.getEntity().getContent());
}
@Test
@ -118,18 +119,20 @@ public class UpdateTest {
HttpPut httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog, Cat");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
CloseableHttpResponse status = ourClient.execute(httpPost);
assertEquals(2, ourReportProvider.getLastTags().size());
assertEquals(new Tag("Dog"), ourReportProvider.getLastTags().get(0));
assertEquals(new Tag("Cat"), ourReportProvider.getLastTags().get(1));
IOUtils.closeQuietly(status.getEntity().getContent());
httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog; label=\"aa\", Cat; label=\"bb\"");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
status = ourClient.execute(httpPost);
assertEquals(2, ourReportProvider.getLastTags().size());
assertEquals(new Tag((String) null, "Dog", "aa"), ourReportProvider.getLastTags().get(0));
assertEquals(new Tag((String) null, "Cat", "bb"), ourReportProvider.getLastTags().get(1));
IOUtils.closeQuietly(status.getEntity().getContent());
}
@ -142,9 +145,10 @@ public class UpdateTest {
HttpPut httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
CloseableHttpResponse status = ourClient.execute(httpPost);
assertEquals(1, ourReportProvider.getLastTags().size());
assertEquals(new Tag("Dog"), ourReportProvider.getLastTags().get(0));
IOUtils.closeQuietly(status.getEntity().getContent());
}
@ -157,9 +161,10 @@ public class UpdateTest {
HttpPut httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog; scheme=\"http://foo\"");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
CloseableHttpResponse status = ourClient.execute(httpPost);
assertEquals(1, ourReportProvider.getLastTags().size());
assertEquals(new Tag("http://foo", "Dog", null), ourReportProvider.getLastTags().get(0));
IOUtils.closeQuietly(status.getEntity().getContent());
httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog; scheme=\"http://foo\";");
@ -167,6 +172,7 @@ public class UpdateTest {
ourClient.execute(httpPost);
assertEquals(1, ourReportProvider.getLastTags().size());
assertEquals(new Tag("http://foo", "Dog", null), ourReportProvider.getLastTags().get(0));
IOUtils.closeQuietly(status.getEntity().getContent());
}
@ -179,16 +185,18 @@ public class UpdateTest {
HttpPut httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog; scheme=\"http://foo\"; label=\"aaaa\"");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
CloseableHttpResponse status = ourClient.execute(httpPost);
assertEquals(1, ourReportProvider.getLastTags().size());
assertEquals(new Tag("http://foo", "Dog", "aaaa"), ourReportProvider.getLastTags().get(0));
IOUtils.closeQuietly(status.getEntity().getContent());
httpPost = new HttpPut("http://localhost:" + ourPort + "/DiagnosticReport/001");
httpPost.addHeader("Category", "Dog; scheme=\"http://foo\"; label=\"aaaa\"; ");
httpPost.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(dr), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
ourClient.execute(httpPost);
status=ourClient.execute(httpPost);
assertEquals(1, ourReportProvider.getLastTags().size());
assertEquals(new Tag("http://foo", "Dog", "aaaa"), ourReportProvider.getLastTags().get(0));
IOUtils.closeQuietly(status.getEntity().getContent());
}
@ -208,9 +216,9 @@ public class UpdateTest {
// IOUtils.toString(status.getEntity().getContent());
// ourLog.info("Response was:\n{}", responseContent);
assertEquals(204, status.getStatusLine().getStatusCode());
assertNull(status.getEntity());
assertEquals(200, status.getStatusLine().getStatusCode());
assertEquals("http://localhost:" + ourPort + "/DiagnosticReport/001/_history/002", status.getFirstHeader("Location").getValue());
IOUtils.closeQuietly(status.getEntity().getContent());
}
@ -224,10 +232,11 @@ public class UpdateTest {
httpPut.addHeader("Content-Location", "/Patient/001/_history/002");
httpPut.setEntity(new StringEntity(new FhirContext().newXmlParser().encodeResourceToString(patient), ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
CloseableHttpResponse results = ourClient.execute(httpPut);
assertEquals(400, results.getStatusLine().getStatusCode());
String responseContent = IOUtils.toString(results.getEntity().getContent());
CloseableHttpResponse status = ourClient.execute(httpPut);
assertEquals(400, status.getStatusLine().getStatusCode());
String responseContent = IOUtils.toString(status.getEntity().getContent());
ourLog.info("Response was:\n{}", responseContent);
IOUtils.closeQuietly(status.getEntity().getContent());
}

10
hapi-fhir-jpaserver-base/pom.xml
View File

@ -253,6 +253,16 @@
<artifactId>validation-api</artifactId>
<version>1.1.0.Final</version>
</dependency>
<dependency>
<groupId>javax.el</groupId>
<artifactId>javax.el-api</artifactId>
<version>3.0.0</version>
</dependency>
<dependency>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<version>3.0.0</version>
</dependency>
<!-- Misc -->
<dependency>

2
pom.xml
View File

@ -142,6 +142,7 @@
</configuration>
</plugin>
<!--This plugin's configuration is used to store Eclipse m2e settings only. It has no influence on the Maven build itself.-->
<!--
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
@ -172,6 +173,7 @@
</lifecycleMappingMetadata>
</configuration>
</plugin>
-->
</plugins>
</pluginManagement>

2
restful-server-example/.settings/org.eclipse.wst.common.component
View File

@ -6,7 +6,7 @@
<dependent-module archiveName="hapi-fhir-base-0.6-SNAPSHOT.jar" deploy-path="/WEB-INF/lib" handle="module:/resource/hapi-fhir-base/hapi-fhir-base">
<dependency-type>uses</dependency-type>
</dependent-module>
<dependent-module deploy-path="/" handle="module:/overlay/prj/hapi-fhir-tester-overlay?includes=**/**&amp;excludes=META-INF/MANIFEST.MF">
<dependent-module deploy-path="/" handle="module:/overlay/prj/hapi-fhir-testpage-overlay?includes=**/**&amp;excludes=META-INF/MANIFEST.MF">
<dependency-type>consumes</dependency-type>
</dependent-module>
<dependent-module deploy-path="/" handle="module:/overlay/slf/?includes=**/**&amp;excludes=META-INF/MANIFEST.MF">