Server create and update methods failed with an IllegalArgumentException if the method type was a custom resource definition type (instead of a built-in HAPI type). Thanks to Neal Acharya for the analysis.

This commit is contained in:
jamesagnew 2015-01-10 07:42:46 -05:00
parent 1063b6cf1e
commit 8f8f9ed308
6 changed files with 174 additions and 89 deletions

View File

@ -615,7 +615,7 @@ class ModelScanner {
private <T extends Annotation> T pullAnnotation(AnnotatedElement theTarget, Class<T> theAnnotationType) { private <T extends Annotation> T pullAnnotation(AnnotatedElement theTarget, Class<T> theAnnotationType) {
T retVal = theTarget.getAnnotation(theAnnotationType); T retVal = theTarget.getAnnotation(theAnnotationType);
if (retVal == null) { if (retVal == null) {
String sourceClassName = theAnnotationType.getClass().getName(); String sourceClassName = theAnnotationType.getName();
String candidateAltClassName = sourceClassName.replace("ca.uhn.fhir.model.api.annotation", "org.hl7.fhir.instance.model.annotations"); String candidateAltClassName = sourceClassName.replace("ca.uhn.fhir.model.api.annotation", "org.hl7.fhir.instance.model.annotations");
if (!sourceClassName.equals(candidateAltClassName)) { if (!sourceClassName.equals(candidateAltClassName)) {

View File

@ -20,6 +20,7 @@ package ca.uhn.fhir.rest.method;
* #L% * #L%
*/ */
import java.io.BufferedReader;
import java.io.IOException; import java.io.IOException;
import java.io.Reader; import java.io.Reader;
import java.io.Writer; import java.io.Writer;
@ -32,6 +33,7 @@ import java.util.Set;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.instance.model.IBaseResource;
import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
@ -68,6 +70,71 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
} }
} }
private void addLocationHeader(Request theRequest, HttpServletResponse theResponse, MethodOutcome response, String headerLocation) {
StringBuilder b = new StringBuilder();
b.append(theRequest.getFhirServerBase());
b.append('/');
b.append(getResourceName());
b.append('/');
b.append(response.getId().getIdPart());
if (response.getId().hasVersionIdPart()) {
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getId().getVersionIdPart());
} else if (response.getVersionId() != null && response.getVersionId().isEmpty() == false) {
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getVersionId().getValue());
}
theResponse.addHeader(headerLocation, b.toString());
}
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)
*/
protected boolean allowVoidReturnType() {
return false;
}
protected abstract BaseHttpClientInvocation createClientInvocation(Object[] theArgs, IResource resource);
/*
* @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);
*
* 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 }
*/
/**
* 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();
@Override @Override
public boolean incomingServerRequestMatchesMethod(Request theRequest) { public boolean incomingServerRequestMatchesMethod(Request theRequest) {
Set<RequestType> allowableRequestTypes = provideAllowableRequestTypes(); Set<RequestType> allowableRequestTypes = provideAllowableRequestTypes();
@ -224,79 +291,20 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
return myReturnVoid; return myReturnVoid;
} }
private void addLocationHeader(Request theRequest, HttpServletResponse theResponse, MethodOutcome response, String headerLocation) {
StringBuilder b = new StringBuilder();
b.append(theRequest.getFhirServerBase());
b.append('/');
b.append(getResourceName());
b.append('/');
b.append(response.getId().getIdPart());
if (response.getId().hasVersionIdPart()) {
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getId().getVersionIdPart());
} else if (response.getVersionId() != null && response.getVersionId().isEmpty() == false) {
b.append("/" + Constants.PARAM_HISTORY + "/");
b.append(response.getVersionId().getValue());
}
theResponse.addHeader(headerLocation, b.toString());
}
/*
* @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);
*
* 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 }
*/
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)
*/
protected boolean allowVoidReturnType() {
return false;
}
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.
*/
protected abstract String getMatchingOperation();
/** /**
* @throws IOException * @throws IOException
*/ */
protected IResource parseIncomingServerResource(Request theRequest) throws IOException { protected IResource parseIncomingServerResource(Request theRequest) throws IOException {
EncodingEnum encoding = RestfulServer.determineRequestEncoding(theRequest); EncodingEnum encoding = RestfulServer.determineRequestEncoding(theRequest);
IParser parser = encoding.newParser(getContext()); IParser parser = encoding.newParser(getContext());
IResource resource = parser.parseResource(theRequest.getServletRequest().getReader()); BufferedReader requestReader = theRequest.getServletRequest().getReader();
return resource;
Class<? extends IBaseResource> wantedResourceType = requestContainsResourceType();
if (wantedResourceType != null) {
return (IResource) parser.parseResource(wantedResourceType, requestReader);
} else {
return parser.parseResource(requestReader);
}
} }
protected abstract Set<RequestType> provideAllowableRequestTypes(); protected abstract Set<RequestType> provideAllowableRequestTypes();
@ -307,6 +315,14 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
protected boolean requestContainsResource() { protected boolean requestContainsResource() {
return true; return true;
} }
/**
* Subclasses may override to provide a specific resource type that this method wants
* as a parameter
*/
protected Class<? extends IBaseResource> requestContainsResourceType() {
return null;
}
protected void streamOperationOutcome(BaseServerResponseException theE, RestfulServer theServer, EncodingEnum theEncoding, HttpServletResponse theResponse, Request theRequest) throws IOException { protected void streamOperationOutcome(BaseServerResponseException theE, RestfulServer theServer, EncodingEnum theEncoding, HttpServletResponse theResponse, Request theRequest) throws IOException {
theResponse.setStatus(theE.getStatusCode()); theResponse.setStatus(theE.getStatusCode());

View File

@ -24,6 +24,7 @@ import java.io.IOException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.hl7.fhir.instance.model.IBaseResource;
import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
@ -42,6 +43,7 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu
private int myResourceParameterIndex; private int myResourceParameterIndex;
private String myResourceName; private String myResourceName;
private boolean myBinary; private boolean myBinary;
private Class<? extends IResource> myResourceType;
public BaseOutcomeReturningMethodBindingWithResourceParam(Method theMethod, FhirContext theContext, Class<?> theMethodAnnotation, Object theProvider) { public BaseOutcomeReturningMethodBindingWithResourceParam(Method theMethod, FhirContext theContext, Class<?> theMethodAnnotation, Object theProvider) {
super(theMethod, theContext, theMethodAnnotation, theProvider); super(theMethod, theContext, theMethodAnnotation, theProvider);
@ -51,6 +53,10 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu
int index = 0; int index = 0;
for (IParameter next : getParameters()) { for (IParameter next : getParameters()) {
if (next instanceof ResourceParameter) { if (next instanceof ResourceParameter) {
if (myResourceType != null) {
throw new ConfigurationException("Method " + theMethod.getName() + " on type " + theMethod.getDeclaringClass() + " has more than one @ResourceParam. Only one is allowed.");
}
resourceParameter = (ResourceParameter) next; resourceParameter = (ResourceParameter) next;
Class<? extends IResource> resourceType = resourceParameter.getResourceType(); Class<? extends IResource> resourceType = resourceParameter.getResourceType();
@ -62,7 +68,8 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu
myBinary = true; myBinary = true;
} }
myResourceName = theContext.getResourceDefinition(resourceType).getName(); myResourceType = resourceParameter.getResourceType();
myResourceName = theContext.getResourceDefinition(myResourceType).getName();
myResourceParameterIndex = index; myResourceParameterIndex = index;
} }
@ -75,6 +82,11 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu
} }
@Override
protected Class<? extends IBaseResource> requestContainsResourceType() {
return myResourceType;
}
@Override @Override
protected IResource parseIncomingServerResource(Request theRequest) throws IOException { protected IResource parseIncomingServerResource(Request theRequest) throws IOException {
if (myBinary) { if (myBinary) {

View File

@ -34,16 +34,16 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
public class ResourceParameter implements IParameter { public class ResourceParameter implements IParameter {
private Class<? extends IResource> myResourceName ; private Class<? extends IResource> myResourceType;
public ResourceParameter(Class<? extends IResource> theParameterType) { public ResourceParameter(Class<? extends IResource> theParameterType) {
myResourceName =theParameterType; myResourceType = theParameterType;
} }
@Override @Override
public void translateClientArgumentIntoQueryArgument(FhirContext theContext, Object theSourceClientArgument, Map<String, List<String>> theTargetQueryArguments) throws InternalErrorException { public void translateClientArgumentIntoQueryArgument(FhirContext theContext, Object theSourceClientArgument, Map<String, List<String>> theTargetQueryArguments) throws InternalErrorException {
// TODO Auto-generated method stub // TODO Auto-generated method stub
} }
@Override @Override
@ -53,12 +53,12 @@ public class ResourceParameter implements IParameter {
} }
public Class<? extends IResource> getResourceType() { public Class<? extends IResource> getResourceType() {
return myResourceName; return myResourceType;
} }
@Override @Override
public void initializeTypes(Method theMethod, Class<? extends Collection<?>> theOuterCollectionType, Class<? extends Collection<?>> theInnerCollectionType, Class<?> theParameterType) { public void initializeTypes(Method theMethod, Class<? extends Collection<?>> theOuterCollectionType, Class<? extends Collection<?>> theInnerCollectionType, Class<?> theParameterType) {
// ignore for now // ignore for now
} }
} }

View File

@ -18,6 +18,7 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.instance.model.annotations.ResourceDef;
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
@ -25,11 +26,15 @@ import org.junit.Test;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.TagList; import ca.uhn.fhir.model.api.TagList;
import ca.uhn.fhir.model.api.annotation.Child;
import ca.uhn.fhir.model.api.annotation.Extension;
import ca.uhn.fhir.model.dstu.resource.AdverseReaction; import ca.uhn.fhir.model.dstu.resource.AdverseReaction;
import ca.uhn.fhir.model.dstu.resource.DiagnosticReport; import ca.uhn.fhir.model.dstu.resource.DiagnosticReport;
import ca.uhn.fhir.model.dstu.resource.Observation;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome; import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.dstu.resource.Patient;
import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.model.primitive.StringDt;
import ca.uhn.fhir.rest.annotation.Create; import ca.uhn.fhir.rest.annotation.Create;
import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.ResourceParam;
import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.annotation.Search;
@ -47,8 +52,6 @@ public class CreateTest {
private static DiagnosticReportProvider ourReportProvider; private static DiagnosticReportProvider ourReportProvider;
private static Server ourServer; private static Server ourServer;
@Test @Test
public void testCreate() throws Exception { public void testCreate() throws Exception {
@ -70,7 +73,6 @@ public class CreateTest {
} }
@Test @Test
public void testCreateById() throws Exception { public void testCreateById() throws Exception {
@ -92,8 +94,6 @@ public class CreateTest {
} }
@Test @Test
public void testCreateJson() throws Exception { public void testCreateJson() throws Exception {
@ -116,6 +116,27 @@ public class CreateTest {
} }
@Test
public void testCreateCustomType() throws Exception {
Observation obs = new Observation();
obs.getIdentifier().setValue("001");
HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Observation");
httpPost.setEntity(new StringEntity(new FhirContext().newJsonParser().encodeResourceToString(obs), ContentType.create(Constants.CT_FHIR_JSON, "UTF-8")));
HttpResponse status = ourClient.execute(httpPost);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
ourLog.info("Response was:\n{}", responseContent);
assertEquals(201, status.getStatusLine().getStatusCode());
assertEquals("http://localhost:" + ourPort + "/Observation/001/_history/002", status.getFirstHeader("location").getValue());
}
@Test @Test
public void testCreateWithUnprocessableEntity() throws Exception { public void testCreateWithUnprocessableEntity() throws Exception {
@ -155,7 +176,7 @@ public class CreateTest {
ServletHandler proxyHandler = new ServletHandler(); ServletHandler proxyHandler = new ServletHandler();
RestfulServer servlet = new RestfulServer(); RestfulServer servlet = new RestfulServer();
servlet.setResourceProviders(patientProvider, ourReportProvider, new DummyAdverseReactionResourceProvider()); servlet.setResourceProviders(patientProvider, ourReportProvider, new DummyAdverseReactionResourceProvider(), new CustomObservationProvider());
ServletHolder servletHolder = new ServletHolder(servlet); ServletHolder servletHolder = new ServletHolder(servlet);
proxyHandler.addServletWithMapping(servletHolder, "/*"); proxyHandler.addServletWithMapping(servletHolder, "/*");
ourServer.setHandler(proxyHandler); ourServer.setHandler(proxyHandler);
@ -180,7 +201,6 @@ public class CreateTest {
return DiagnosticReport.class; return DiagnosticReport.class;
} }
@Create() @Create()
public MethodOutcome createDiagnosticReport(@ResourceParam DiagnosticReport thePatient) { public MethodOutcome createDiagnosticReport(@ResourceParam DiagnosticReport thePatient) {
OperationOutcome outcome = new OperationOutcome(); OperationOutcome outcome = new OperationOutcome();
@ -189,7 +209,7 @@ public class CreateTest {
} }
} }
/** /**
* Created by dsotnikov on 2/25/2014. * Created by dsotnikov on 2/25/2014.
*/ */
@ -228,10 +248,6 @@ public class CreateTest {
} }
public static class PatientProvider implements IResourceProvider { public static class PatientProvider implements IResourceProvider {
@Override @Override
@ -242,12 +258,48 @@ public class CreateTest {
@Create() @Create()
public MethodOutcome createPatient(@ResourceParam Patient thePatient) { public MethodOutcome createPatient(@ResourceParam Patient thePatient) {
IdDt id = new IdDt(thePatient.getIdentifier().get(0).getValue().getValue()); IdDt id = new IdDt(thePatient.getIdentifier().get(0).getValue().getValue());
if (thePatient.getId().isEmpty()==false) { if (thePatient.getId().isEmpty() == false) {
id=thePatient.getId(); id = thePatient.getId();
} }
return new MethodOutcome(id.withVersion("002")); return new MethodOutcome(id.withVersion("002"));
} }
}
@ResourceDef(name = "Observation")
public static class CustomObservation extends Observation {
@Extension(url = "http://foo#string", definedLocally = false, isModifier = false)
@Child(name = "string")
private StringDt myString;
public StringDt getString() {
if (myString == null) {
myString = new StringDt();
}
return myString;
}
public void setString(StringDt theString) {
myString = theString;
}
}
public static class CustomObservationProvider implements IResourceProvider {
@Override
public Class<? extends IResource> getResourceType() {
return Observation.class;
}
@Create()
public MethodOutcome createPatient(@ResourceParam CustomObservation thePatient) {
IdDt id = new IdDt(thePatient.getIdentifier().getValue().getValue());
if (thePatient.getId().isEmpty() == false) {
id = thePatient.getId();
}
return new MethodOutcome(id.withVersion("002"));
}
} }

View File

@ -45,6 +45,11 @@
header in server was incorrectly using FHIR date format instead header in server was incorrectly using FHIR date format instead
of RFC-1123 format. of RFC-1123 format.
</action> </action>
<action type="fix">
Server create and update methods failed with an IllegalArgumentException if
the method type was a custom resource definition type (instead of a built-in
HAPI type). Thanks to Neal Acharya for the analysis.
</action>
</release> </release>
<release version="0.8" date="2014-Dec-17"> <release version="0.8" date="2014-Dec-17">
<action type="add"> <action type="add">