From ed5bffba9ead06ab23b169a546b8cdc146535b65 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 28 Feb 2016 19:38:54 -0500 Subject: [PATCH] Fix #299 - Don't crash if the client receives extensions in Bundle.entry.search --- .../java/ca/uhn/fhir/parser/ParserState.java | 3 + .../BaseResourceReturningMethodBinding.java | 39 +++-- .../fhir/rest/method/SearchMethodBinding.java | 7 +- .../java/ca/uhn/fhir/util/BundleUtil.java | 41 +++++ .../client/ClientWithProfileDstu2Test.java | 4 - .../rest/client/SearchClientDstu3Test.java | 156 ++++++++++++++++++ 6 files changed, 231 insertions(+), 19 deletions(-) create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java create mode 100644 hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java index ad495489338..e0f96cb61b4 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java @@ -2117,16 +2117,19 @@ class ParserState { public PreResourceStateHapi(BundleEntry theEntry, Class theResourceType) { super(theResourceType); myEntry = theEntry; + assert theResourceType == null || IResource.class.isAssignableFrom(theResourceType); } public PreResourceStateHapi(Class theResourceType) { super(theResourceType); + assert theResourceType == null || IResource.class.isAssignableFrom(theResourceType); } public PreResourceStateHapi(Object theTarget, IMutator theMutator, Class theResourceType) { super(theResourceType); myTarget = theTarget; myMutator = theMutator; + assert theResourceType == null || IResource.class.isAssignableFrom(theResourceType); } @Override diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseResourceReturningMethodBinding.java index 6c404daa432..3bc3deec7f9 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseResourceReturningMethodBinding.java @@ -25,21 +25,23 @@ import java.io.IOException; import java.io.Reader; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.model.api.Bundle; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.Include; @@ -62,6 +64,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor; +import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.ReflectionUtil; import ca.uhn.fhir.util.UrlUtil; @@ -157,32 +160,40 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi switch (getReturnType()) { case BUNDLE: { - Bundle bundle; - if (myResourceType != null) { - bundle = parser.parseBundle(myResourceType, theResponseReader); + + Bundle dstu1bundle = null; + IBaseBundle dstu2bundle = null; + List listOfResources = null; + if (getMethodReturnType() == MethodReturnTypeEnum.BUNDLE || getContext().getVersion().getVersion() == FhirVersionEnum.DSTU1) { + if (myResourceType != null) { + dstu1bundle = parser.parseBundle(myResourceType, theResponseReader); + } else { + dstu1bundle = parser.parseBundle(theResponseReader); + } } else { - bundle = parser.parseBundle(theResponseReader); + Class type = getContext().getResourceDefinition("Bundle").getImplementingClass(); + dstu2bundle = (IBaseBundle) parser.parseResource(type, theResponseReader); + listOfResources = BundleUtil.toListOfResources(getContext(), dstu2bundle); } + switch (getMethodReturnType()) { case BUNDLE: - return bundle; + return dstu1bundle; + case BUNDLE_RESOURCE: + return dstu2bundle; case LIST_OF_RESOURCES: - List listOfResources; if (myResourceListCollectionType != null) { - listOfResources = new ArrayList(); - for (IResource next : bundle.toListOfResources()) { + for (Iterator iter = listOfResources.iterator(); iter.hasNext(); ) { + IBaseResource next = iter.next(); if (!myResourceListCollectionType.isAssignableFrom(next.getClass())) { ourLog.debug("Not returning resource of type {} because it is not a subclass or instance of {}", next.getClass(), myResourceListCollectionType); - continue; + iter.remove(); } - listOfResources.add(next); } - } else { - listOfResources = bundle.toListOfResources(); } return listOfResources; case RESOURCE: - List list = bundle.toListOfResources(); + List list = dstu1bundle.toListOfResources(); if (list.size() == 0) { return null; } else if (list.size() == 1) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java index e710e80a0b4..984bdd978ab 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java @@ -37,6 +37,7 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.model.api.annotation.Description; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.valueset.BundleTypeEnum; @@ -130,7 +131,11 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { @Override public ReturnTypeEnum getReturnType() { - return ReturnTypeEnum.BUNDLE; +// if (getContext().getVersion().getVersion() == FhirVersionEnum.DSTU1) { + return ReturnTypeEnum.BUNDLE; +// } else { +// return ReturnTypeEnum.RESOURCE; +// } } @Override diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java new file mode 100644 index 00000000000..576363ee856 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -0,0 +1,41 @@ +package ca.uhn.fhir.util; + +import java.util.ArrayList; +import java.util.List; + +import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.instance.model.api.IBaseBundle; +import org.hl7.fhir.instance.model.api.IBaseResource; + +import ca.uhn.fhir.context.BaseRuntimeChildDefinition; +import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; + +/** + * Fetch resources from a bundle + */ +public class BundleUtil { + + /** + * Extract all of the resources from a given bundle + */ + public static List toListOfResources(FhirContext theContext, IBaseBundle theBundle) { + List retVal = new ArrayList(); + + RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle); + BaseRuntimeChildDefinition entryChild = def.getChildByName("entry"); + List entries = entryChild.getAccessor().getValues(theBundle); + + BaseRuntimeElementCompositeDefinition entryChildElem = (BaseRuntimeElementCompositeDefinition) entryChild.getChildByName("entry"); + BaseRuntimeChildDefinition resourceChild = entryChildElem.getChildByName("resource"); + for (IBase nextEntry : entries) { + for (IBase next : resourceChild.getAccessor().getValues(nextEntry)) { + retVal.add((IBaseResource) next); + } + } + + return retVal; + } + +} diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientWithProfileDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientWithProfileDstu2Test.java index e94e122f71a..d807a768ee6 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientWithProfileDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientWithProfileDstu2Test.java @@ -1,6 +1,5 @@ package ca.uhn.fhir.rest.client; -import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -27,10 +26,8 @@ import org.mockito.stubbing.Answer; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.annotation.ResourceDef; import ca.uhn.fhir.model.dstu2.resource.OperationOutcome; -import ca.uhn.fhir.model.dstu2.resource.Parameters; import ca.uhn.fhir.model.dstu2.resource.Patient; import ca.uhn.fhir.model.dstu2.valueset.IssueTypeEnum; -import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.Constants; @@ -39,7 +36,6 @@ public class ClientWithProfileDstu2Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ClientWithProfileDstu2Test.class); private FhirContext ourCtx; private HttpClient ourHttpClient; - private HttpResponse ourHttpResponse; @Before diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java new file mode 100644 index 00000000000..a62275a9d2b --- /dev/null +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java @@ -0,0 +1,156 @@ +package ca.uhn.fhir.rest.client; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.InputStream; +import java.io.StringReader; +import java.nio.charset.Charset; +import java.util.List; + +import org.apache.commons.io.input.ReaderInputStream; +import org.apache.http.HttpResponse; +import org.apache.http.ProtocolVersion; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.message.BasicHeader; +import org.apache.http.message.BasicStatusLine; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; +import org.hl7.fhir.dstu3.model.Extension; +import org.hl7.fhir.dstu3.model.Location; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.internal.stubbing.defaultanswers.ReturnsDeepStubs; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.annotation.Count; +import ca.uhn.fhir.rest.annotation.RequiredParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.client.api.IRestfulClient; +import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.server.Constants; + + +public class SearchClientDstu3Test { + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchClientDstu3Test.class); + private FhirContext ourCtx; + private HttpClient ourHttpClient; + private HttpResponse ourHttpResponse; + + @Before + public void before() { + ourCtx = FhirContext.forDstu3(); + + ourHttpClient = mock(HttpClient.class, new ReturnsDeepStubs()); + ourCtx.getRestfulClientFactory().setHttpClient(ourHttpClient); + ourCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); + + ourHttpResponse = mock(HttpResponse.class, new ReturnsDeepStubs()); + } + + /** + * See #299 + */ + @Test + public void testListResponseWithSearchExtension() throws Exception { + + final String response = createBundleWithSearchExtension(); + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); + when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8")); + when(ourHttpResponse.getEntity().getContent()).thenAnswer(new Answer() { + @Override + public InputStream answer(InvocationOnMock theInvocation) throws Throwable { + return new ReaderInputStream(new StringReader(response), Charset.forName("UTF-8")); + } + }); + + ILocationClient client = ourCtx.newRestfulClient(ILocationClient.class, "http://localhost:8081/hapi-fhir/fhir"); + + List matches = client.getMatches(new StringParam("smith"), 100); + assertEquals(1, matches.size()); + assertEquals("Sample Clinic", matches.get(0).getName()); + + HttpGet value = (HttpGet) capt.getValue(); + assertEquals("http://localhost:8081/hapi-fhir/fhir/Location?_query=match&name=smith&_count=100", value.getURI().toString()); + } + + /** + * See #299 + */ + @Test + public void testBundleResponseWithSearchExtension() throws Exception { + + final String response = createBundleWithSearchExtension(); + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); + when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8")); + when(ourHttpResponse.getEntity().getContent()).thenAnswer(new Answer() { + @Override + public InputStream answer(InvocationOnMock theInvocation) throws Throwable { + return new ReaderInputStream(new StringReader(response), Charset.forName("UTF-8")); + } + }); + + ILocationClient client = ourCtx.newRestfulClient(ILocationClient.class, "http://localhost:8081/hapi-fhir/fhir"); + + Bundle matches = client.getMatchesReturnBundle(new StringParam("smith"), 100); + + assertEquals(1, matches.getEntry().size()); + BundleEntryComponent entry = matches.getEntry().get(0); + assertEquals("Sample Clinic", ((Location)entry.getResource()).getName()); + + List ext = entry.getSearch().getExtensionsByUrl("http://hl7.org/fhir/StructureDefinition/algorithmic-match"); + assertEquals(1, ext.size()); + + HttpGet value = (HttpGet) capt.getValue(); + assertEquals("http://localhost:8081/hapi-fhir/fhir/Location?_query=match&name=smith&_count=100", value.getURI().toString()); + } + + private String createBundleWithSearchExtension() { + //@formatter:off + final String response = "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + //@formatter:on + return response; + } + + + + public interface ILocationClient extends IRestfulClient { + @Search(queryName = "match") + public List getMatches(final @RequiredParam(name = Location.SP_NAME) StringParam name, final @Count Integer count); + + @Search(queryName = "match", type=Location.class) + public Bundle getMatchesReturnBundle(final @RequiredParam(name = Location.SP_NAME) StringParam name, final @Count Integer count); + } + +}