From 8f8f9ed308ce7f3f19bbd818e3c058f5e51cd64c Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 10 Jan 2015 07:42:46 -0500 Subject: [PATCH] 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. --- .../ca/uhn/fhir/context/ModelScanner.java | 2 +- .../BaseOutcomeReturningMethodBinding.java | 150 ++++++++++-------- ...turningMethodBindingWithResourceParam.java | 14 +- .../fhir/rest/param/ResourceParameter.java | 12 +- .../ca/uhn/fhir/rest/server/CreateTest.java | 80 ++++++++-- src/changes/changes.xml | 5 + 6 files changed, 174 insertions(+), 89 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java index ab09e07a833..54fb1143951 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java @@ -615,7 +615,7 @@ class ModelScanner { private T pullAnnotation(AnnotatedElement theTarget, Class theAnnotationType) { T retVal = theTarget.getAnnotation(theAnnotationType); 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"); if (!sourceClassName.equals(candidateAltClassName)) { 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 c4a1884493f..0002e36ffa0 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,6 +20,7 @@ package ca.uhn.fhir.rest.method; * #L% */ +import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; import java.io.Writer; @@ -32,6 +33,7 @@ import java.util.Set; import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.instance.model.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; @@ -68,6 +70,71 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding allowableRequestTypes = provideAllowableRequestTypes(); @@ -224,79 +291,20 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding wantedResourceType = requestContainsResourceType(); + if (wantedResourceType != null) { + return (IResource) parser.parseResource(wantedResourceType, requestReader); + } else { + return parser.parseResource(requestReader); + } } protected abstract Set provideAllowableRequestTypes(); @@ -307,6 +315,14 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding requestContainsResourceType() { + return null; + } protected void streamOperationOutcome(BaseServerResponseException theE, RestfulServer theServer, EncodingEnum theEncoding, HttpServletResponse theResponse, Request theRequest) throws IOException { theResponse.setStatus(theE.getStatusCode()); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBindingWithResourceParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBindingWithResourceParam.java index 17f3d125501..0fe04a9e1fe 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBindingWithResourceParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseOutcomeReturningMethodBindingWithResourceParam.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.lang.reflect.Method; import org.apache.commons.io.IOUtils; +import org.hl7.fhir.instance.model.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; @@ -42,6 +43,7 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu private int myResourceParameterIndex; private String myResourceName; private boolean myBinary; + private Class myResourceType; public BaseOutcomeReturningMethodBindingWithResourceParam(Method theMethod, FhirContext theContext, Class theMethodAnnotation, Object theProvider) { super(theMethod, theContext, theMethodAnnotation, theProvider); @@ -51,6 +53,10 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu int index = 0; for (IParameter next : getParameters()) { 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; Class resourceType = resourceParameter.getResourceType(); @@ -62,7 +68,8 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu myBinary = true; } - myResourceName = theContext.getResourceDefinition(resourceType).getName(); + myResourceType = resourceParameter.getResourceType(); + myResourceName = theContext.getResourceDefinition(myResourceType).getName(); myResourceParameterIndex = index; } @@ -75,6 +82,11 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu } + @Override + protected Class requestContainsResourceType() { + return myResourceType; + } + @Override protected IResource parseIncomingServerResource(Request theRequest) throws IOException { if (myBinary) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ResourceParameter.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ResourceParameter.java index 8926a76f3b1..57163046f2f 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ResourceParameter.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ResourceParameter.java @@ -34,16 +34,16 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; public class ResourceParameter implements IParameter { - private Class myResourceName ; + private Class myResourceType; public ResourceParameter(Class theParameterType) { - myResourceName =theParameterType; + myResourceType = theParameterType; } - + @Override public void translateClientArgumentIntoQueryArgument(FhirContext theContext, Object theSourceClientArgument, Map> theTargetQueryArguments) throws InternalErrorException { // TODO Auto-generated method stub - + } @Override @@ -53,12 +53,12 @@ public class ResourceParameter implements IParameter { } public Class getResourceType() { - return myResourceName; + return myResourceType; } @Override public void initializeTypes(Method theMethod, Class> theOuterCollectionType, Class> theInnerCollectionType, Class theParameterType) { // ignore for now } - + } diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/CreateTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/CreateTest.java index 0b54a85be29..9afbe21420e 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/CreateTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/CreateTest.java @@ -18,6 +18,7 @@ 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.instance.model.annotations.ResourceDef; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -25,11 +26,15 @@ import org.junit.Test; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.IResource; 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.DiagnosticReport; +import ca.uhn.fhir.model.dstu.resource.Observation; import ca.uhn.fhir.model.dstu.resource.OperationOutcome; import ca.uhn.fhir.model.dstu.resource.Patient; 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.ResourceParam; import ca.uhn.fhir.rest.annotation.Search; @@ -47,8 +52,6 @@ public class CreateTest { private static DiagnosticReportProvider ourReportProvider; private static Server ourServer; - - @Test public void testCreate() throws Exception { @@ -70,7 +73,6 @@ public class CreateTest { } - @Test public void testCreateById() throws Exception { @@ -92,8 +94,6 @@ public class CreateTest { } - - @Test 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 public void testCreateWithUnprocessableEntity() throws Exception { @@ -155,7 +176,7 @@ public class CreateTest { ServletHandler proxyHandler = new ServletHandler(); RestfulServer servlet = new RestfulServer(); - servlet.setResourceProviders(patientProvider, ourReportProvider, new DummyAdverseReactionResourceProvider()); + servlet.setResourceProviders(patientProvider, ourReportProvider, new DummyAdverseReactionResourceProvider(), new CustomObservationProvider()); ServletHolder servletHolder = new ServletHolder(servlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); @@ -180,7 +201,6 @@ public class CreateTest { return DiagnosticReport.class; } - @Create() public MethodOutcome createDiagnosticReport(@ResourceParam DiagnosticReport thePatient) { OperationOutcome outcome = new OperationOutcome(); @@ -189,7 +209,7 @@ public class CreateTest { } } - + /** * Created by dsotnikov on 2/25/2014. */ @@ -228,10 +248,6 @@ public class CreateTest { } - - - - public static class PatientProvider implements IResourceProvider { @Override @@ -242,12 +258,48 @@ public class CreateTest { @Create() public MethodOutcome createPatient(@ResourceParam Patient thePatient) { IdDt id = new IdDt(thePatient.getIdentifier().get(0).getValue().getValue()); - if (thePatient.getId().isEmpty()==false) { - id=thePatient.getId(); + if (thePatient.getId().isEmpty() == false) { + id = thePatient.getId(); } 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 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")); + } } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d54b1515430..2f925da7530 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -45,6 +45,11 @@ header in server was incorrectly using FHIR date format instead of RFC-1123 format. + + 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. +