diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4399-avoid-noisy-brokenpipe-exception.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4399-avoid-noisy-brokenpipe-exception.yaml new file mode 100644 index 00000000000..8ddab94d425 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4399-avoid-noisy-brokenpipe-exception.yaml @@ -0,0 +1,5 @@ +--- +type: fix +title: "When running RestfulServer under heavy load or a slow network, the server sometimes + logged an EOFException or an IOException in the system logs despite the response completing + successfully. This has been corrected." diff --git a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProvider.java b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProvider.java index 9f88a8dd902..23db1597ee9 100644 --- a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProvider.java +++ b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProvider.java @@ -36,6 +36,7 @@ import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.jaxrs.server.util.JaxRsRequest; import ca.uhn.fhir.jaxrs.server.util.JaxRsRequest.Builder; import ca.uhn.fhir.rest.annotation.IdParam; import ca.uhn.fhir.rest.api.Constants; @@ -47,6 +48,7 @@ import ca.uhn.fhir.rest.server.HardcodedServerAddressStrategy; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.ResourceBinding; import ca.uhn.fhir.rest.server.RestfulServerConfiguration; +import ca.uhn.fhir.rest.server.RestfulServerUtils; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.provider.ServerCapabilityStatementProvider; import ca.uhn.fhir.util.ReflectionUtil; @@ -214,7 +216,8 @@ public abstract class AbstractJaxRsConformanceProvider extends AbstractJaxRsProv setUpPostConstruct(); Builder request = getRequest(RequestTypeEnum.OPTIONS, RestOperationTypeEnum.METADATA); - IRestfulResponse response = request.build().getResponse(); + JaxRsRequest requestDetails = request.build(); + IRestfulResponse response = requestDetails.getResponse(); response.addHeader(Constants.HEADER_CORS_ALLOW_ORIGIN, "*"); IBaseResource conformance; @@ -239,11 +242,9 @@ public abstract class AbstractJaxRsConformanceProvider extends AbstractJaxRsProv throw new ConfigurationException(Msg.code(592) + "Unsupported Fhir version: " + fhirContextVersion); } - if (conformance != null) { - Set summaryMode = Collections.emptySet(); - return (Response) response.streamResponseAsResource(conformance, false, summaryMode, Constants.STATUS_HTTP_200_OK, null, true, false); - } - return (Response) response.returnResponse(null, Constants.STATUS_HTTP_500_INTERNAL_ERROR, true, null, getResourceType().getSimpleName()); + Set summaryMode = Collections.emptySet(); + + return (Response) RestfulServerUtils.streamResponseAsResource(this, conformance, summaryMode, Constants.STATUS_HTTP_200_OK, false, true, requestDetails, null, null); } /** diff --git a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponse.java b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponse.java index 85c527adf2f..b8c25373a4e 100644 --- a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponse.java +++ b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponse.java @@ -1,20 +1,17 @@ package ca.uhn.fhir.jaxrs.server.util; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.EncodingEnum; -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.server.BaseParseAction; import ca.uhn.fhir.rest.server.BaseRestfulResponse; -import ca.uhn.fhir.rest.server.RestfulServerUtils; +import ca.uhn.fhir.util.IoUtil; import org.apache.commons.lang3.StringUtils; -import org.hl7.fhir.instance.model.api.IBaseBinary; +import org.apache.commons.lang3.Validate; -import javax.ws.rs.core.MediaType; +import javax.annotation.Nonnull; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.ResponseBuilder; -import java.io.IOException; +import java.io.ByteArrayOutputStream; +import java.io.Closeable; +import java.io.OutputStream; import java.io.StringWriter; import java.io.Writer; import java.util.List; @@ -49,6 +46,12 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; */ public class JaxRsResponse extends BaseRestfulResponse { + private StringWriter myWriter; + private int myStatusCode; + private String myContentType; + private String myCharset; + private ByteArrayOutputStream myOutputStream; + /** * The constructor * @@ -62,49 +65,54 @@ public class JaxRsResponse extends BaseRestfulResponse { * The response writer is a simple String Writer. All output is configured * by the server. */ + @Nonnull @Override - public Writer getResponseWriter(int theStatusCode, String theStatusMessage, String theContentType, String theCharset, boolean theRespondGzip) { - return new StringWriter(); + public Writer getResponseWriter(int theStatusCode, String theContentType, String theCharset, boolean theRespondGzip) { + Validate.isTrue(myWriter == null, "getResponseWriter() called multiple times"); + Validate.isTrue(myOutputStream == null, "getResponseWriter() called after getResponseOutputStream()"); + myWriter = new StringWriter(); + myStatusCode = theStatusCode; + myContentType = theContentType; + myCharset = theCharset; + return myWriter; + } + + @Nonnull + @Override + public OutputStream getResponseOutputStream(int theStatusCode, String theContentType, Integer theContentLength) { + Validate.isTrue(myWriter == null, "getResponseOutputStream() called multiple times"); + Validate.isTrue(myOutputStream == null, "getResponseOutputStream() called after getResponseWriter()"); + + myOutputStream = new ByteArrayOutputStream(); + myStatusCode = theStatusCode; + myContentType = theContentType; + + return myOutputStream; } @Override - public Response sendWriterResponse(int theStatus, String theContentType, String theCharset, Writer theWriter) { - ResponseBuilder builder = buildResponse(theStatus); - if (isNotBlank(theContentType)) { - String charContentType = theContentType + "; charset=" + StringUtils.defaultIfBlank(theCharset, Constants.CHARSET_NAME_UTF8); - builder.header(Constants.HEADER_CONTENT_TYPE, charContentType); + public Response commitResponse(@Nonnull Closeable theWriterOrOutputStream) { + IoUtil.closeQuietly(theWriterOrOutputStream); + + ResponseBuilder builder = buildResponse(myStatusCode); + if (isNotBlank(myContentType)) { + if (myWriter != null) { + String charContentType = myContentType + "; charset=" + StringUtils.defaultIfBlank(myCharset, Constants.CHARSET_NAME_UTF8); + builder.header(Constants.HEADER_CONTENT_TYPE, charContentType); + builder.entity(myWriter.toString()); + } else { + byte[] byteArray = myOutputStream.toByteArray(); + if (byteArray.length > 0) { + builder.header(Constants.HEADER_CONTENT_TYPE, myContentType); + builder.entity(byteArray); + } + } } - builder.entity(theWriter.toString()); + Response retVal = builder.build(); return retVal; } - @Override - public Object sendAttachmentResponse(IBaseBinary bin, int statusCode, String contentType) { - ResponseBuilder response = buildResponse(statusCode); - if (bin.getContent() != null && bin.getContent().length > 0) { - response.header(Constants.HEADER_CONTENT_TYPE, contentType).entity(bin.getContent()); - } - return response.build(); - } - - @Override - public Response returnResponse(BaseParseAction outcome, int operationStatus, boolean allowPrefer, - MethodOutcome response, String resourceName) throws IOException { - StringWriter writer = new StringWriter(); - if (outcome != null) { - FhirContext fhirContext = getRequestDetails().getServer().getFhirContext(); - IParser parser = RestfulServerUtils.getNewParser(fhirContext, fhirContext.getVersion().getVersion(), getRequestDetails()); - outcome.execute(parser, writer); - } - return sendWriterResponse(operationStatus, getParserType(), null, writer); - } - - protected String getParserType() { - EncodingEnum encodingEnum = RestfulServerUtils.determineResponseEncodingWithDefault(getRequestDetails()).getEncoding(); - return encodingEnum == EncodingEnum.JSON ? MediaType.APPLICATION_JSON : MediaType.APPLICATION_XML; - } - private ResponseBuilder buildResponse(int statusCode) { ResponseBuilder response = Response.status(statusCode); for (Entry> header : getHeaders().entrySet()) { diff --git a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java index d08f334ca6d..9c357ddc523 100644 --- a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java +++ b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jaxrs.server.util; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -61,7 +62,7 @@ public class JaxRsResponseDstu3Test { Response result = (Response) RestfulServerUtils.streamResponseAsResource(request.getServer(), binary, theSummaryMode, 200, theAddContentLocationHeader, respondGzip, this.request); assertEquals(200, result.getStatus()); assertEquals(contentType, result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); - assertEquals(content, result.getEntity()); + assertArrayEquals(content, (byte[])result.getEntity()); } @Test diff --git a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseTest.java b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseTest.java index 7aa1b36e5d6..b2426246986 100644 --- a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseTest.java +++ b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseTest.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jaxrs.server.util; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -48,7 +49,7 @@ public class JaxRsResponseTest { Response result = (Response) RestfulServerUtils.streamResponseAsResource(request.getServer(), binary, theSummaryMode, 200, theAddContentLocationHeader, respondGzip, this.request); assertEquals(200, result.getStatus()); assertEquals(contentType, result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); - assertEquals(content, result.getEntity()); + assertArrayEquals(content, (byte[])result.getEntity()); } @Test diff --git a/hapi-fhir-jpaserver-cql/src/test/java/ca/uhn/fhir/cql/r4/CqlMeasureEvaluationR4ImmunizationTest.java b/hapi-fhir-jpaserver-cql/src/test/java/ca/uhn/fhir/cql/r4/CqlMeasureEvaluationR4ImmunizationTest.java index f48137e06a0..96e22f8286c 100644 --- a/hapi-fhir-jpaserver-cql/src/test/java/ca/uhn/fhir/cql/r4/CqlMeasureEvaluationR4ImmunizationTest.java +++ b/hapi-fhir-jpaserver-cql/src/test/java/ca/uhn/fhir/cql/r4/CqlMeasureEvaluationR4ImmunizationTest.java @@ -4,8 +4,6 @@ import ca.uhn.fhir.cql.BaseCqlR4Test; import ca.uhn.fhir.cql.r4.provider.MeasureOperationsProvider; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.MeasureReport; -import org.junit.Ignore; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -41,12 +39,7 @@ public class CqlMeasureEvaluationR4ImmunizationTest extends BaseCqlR4Test { return this.myMeasureOperationsProvider.evaluateMeasure(new IdType("Measure", theMeasureId), evaluationDate, evaluationDate, null, "subject", thePatientRef, null, thePractitionerRef, null, null, null, null, myRequestDetails); } - /** - * Disabled 2023-01-04 - Ticket to re-enable: - * https://github.com/hapifhir/hapi-fhir/issues/4401 - */ @Test - @Disabled public void test_Immunization_Ontario_Schedule() throws IOException { //given loadBundle(MY_FHIR_COMMON); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/client/ClientThreadedCapabilitiesTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/client/ClientThreadedCapabilitiesTest.java index 24e3b970916..864151a5725 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/client/ClientThreadedCapabilitiesTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/client/ClientThreadedCapabilitiesTest.java @@ -63,7 +63,8 @@ public class ClientThreadedCapabilitiesTest { @BeforeEach public void beforeEach() throws Exception { - ourServer.getFhirClient().registerInterceptor(myCountingMetaClientInterceptor); + myClient = ourServer.getFhirClient(); + myClient.registerInterceptor(myCountingMetaClientInterceptor); } @@ -92,7 +93,7 @@ public class ClientThreadedCapabilitiesTest { private Object searchPatient(String last) { - return ourServer.getFhirClient().search() + return myClient.search() .forResource("Patient") .returnBundle(Bundle.class) .where(Patient.FAMILY.matches().value(last)) diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java index fef007889c7..3e54510af86 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java @@ -53,9 +53,11 @@ import java.util.List; import static java.util.Arrays.asList; import static org.apache.commons.lang3.time.DateUtils.MILLIS_PER_SECOND; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -244,14 +246,17 @@ public class ResourceProviderInterceptorR4Test extends BaseResourceProviderR4Tes p.setActive(true); IIdType pid = myClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); - Bundle observations = myClient - .search() - .forResource("Observation") - .where(Observation.SUBJECT.hasId(pid)) - .returnBundle(Bundle.class) - .execute(); - assertEquals(1, observations.getEntry().size()); - ourLog.info(myFhirContext.newXmlParser().setPrettyPrint(true).encodeResourceToString(observations)); + await() + .until(()->{ + Bundle observations = myClient + .search() + .forResource("Observation") + .where(Observation.SUBJECT.hasId(pid)) + .returnBundle(Bundle.class) + .execute(); + return observations.getEntry().size(); + }, + equalTo(1)); } finally { myServer.getRestfulServer().unregisterInterceptor(interceptor); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IRestfulResponse.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IRestfulResponse.java index b27357dd913..24241e9f0d2 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IRestfulResponse.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IRestfulResponse.java @@ -20,42 +20,96 @@ package ca.uhn.fhir.rest.api.server; * #L% */ -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.SummaryEnum; -import org.hl7.fhir.instance.model.api.IBaseBinary; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.instance.model.api.IPrimitiveType; - +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.io.Closeable; import java.io.IOException; +import java.io.OutputStream; import java.io.Writer; -import java.util.Date; import java.util.List; import java.util.Map; -import java.util.Set; +/** + * Implementations of this interface represent a response back to the client from the server. It is + * conceptually similar to {@link javax.servlet.http.HttpServletResponse} but intended to be agnostic + * of the server framework being used. + *

+ * This class is a bit of an awkward abstraction given the two styles of servers it supports. + * Servlets work by writing to a servlet response that is provided as a parameter by the container. JAX-RS + * works by returning an object via a method return back to the containing framework. However using it correctly should + * make for compatible code across both approaches. + *

+ */ public interface IRestfulResponse { - Object streamResponseAsResource(IBaseResource theActualResourceToReturn, boolean thePrettyPrint, Set theSummaryMode, int theStatusCode, String theStatusMessage, boolean theRespondGzip, boolean theAddContentLocation) throws IOException; + /** + * Initiate a new textual response. The Writer returned by this method must be finalized by + * calling {@link #commitResponse(Closeable)} later. + *

+ * Note that the caller should not close the returned object, but should instead just + * return it to {@link #commitResponse(Closeable)} upon successful completion. This is + * different from normal Java practice where you would request it in a try with resource + * block, since in Servlets you are not actually required to close the writer/stream, and + * doing so automatically may prevent you from correctly handling exceptions. + *

+ * + * @param theStatusCode The HTTP status code. + * @param theContentType The HTTP response content type. + * @param theCharset The HTTP response charset. + * @param theRespondGzip Should the response be GZip encoded? + * @return Returns a {@link Writer} that can accept the response body. + */ + @Nonnull + Writer getResponseWriter(int theStatusCode, String theContentType, String theCharset, boolean theRespondGzip) throws IOException; /** - * This is only used for DSTU1 getTags operations, so it can be removed at some point when we - * drop DSTU1 + * Initiate a new binary response. The OutputStream returned by this method must be finalized by + * calling {@link #commitResponse(Closeable)} later. This method should only be used for non-textual + * responses, for those use {@link #getResponseWriter(int, String, String, boolean)}. + *

+ * Note that the caller should not close the returned object, but should instead just + * return it to {@link #commitResponse(Closeable)} upon successful completion. This is + * different from normal Java practice where you would request it in a try with resource + * block, since in Servlets you are not actually required to close the writer/stream, and + * doing so automatically may prevent you from correctly handling exceptions. + *

+ * + * @param theStatusCode The HTTP status code. + * @param theContentType The HTTP response content type. + * @param theContentLength If known, the number of bytes that will be written. {@literal null} otherwise. + * @return Returns an {@link OutputStream} that can accept the response body. */ - Object returnResponse(BaseParseAction outcome, int operationStatus, boolean allowPrefer, MethodOutcome response, String resourceName) throws IOException; + @Nonnull + OutputStream getResponseOutputStream(int theStatusCode, String theContentType, @Nullable Integer theContentLength) throws IOException; - Writer getResponseWriter(int theStatusCode, String theStatusMessage, String theContentType, String theCharset, boolean theRespondGzip) throws IOException; - - Object sendWriterResponse(int status, String contentType, String charset, Writer writer) throws IOException; + /** + * Finalizes the response streaming using the writer that was returned by calling either + * {@link #getResponseWriter(int, String, String, boolean)} or + * {@link #getResponseOutputStream(int, String, Integer)}. This method should only be + * called if the response writing/streaming actually completed successfully. If an error + * occurred you do not need to commit the response. + * + * @param theWriterOrOutputStream The {@link Writer} or {@link OutputStream} that was returned by this object, or a Writer/OutputStream + * which decorates the one returned by this object. + * @return If the server style requires a returned response object (i.e. JAX-RS Server), this method + * returns that object. If the server style does not require one (i.e. {@link ca.uhn.fhir.rest.server.RestfulServer}), + * this method returns {@literal null}. + */ + Object commitResponse(@Nonnull Closeable theWriterOrOutputStream) throws IOException; + /** + * Adds a response header. This method must be called prior to calling + * {@link #getResponseWriter(int, String, String, boolean)} or {@link #getResponseOutputStream(int, String, Integer)}. + * + * @param headerKey The header name + * @param headerValue The header value + */ void addHeader(String headerKey, String headerValue); - Object sendAttachmentResponse(IBaseBinary bin, int stausCode, String contentType) throws IOException; - - void setOperationResourceLastUpdated(IPrimitiveType theOperationResourceLastUpdated); + /** + * Returns the headers added to this response + */ Map> getHeaders(); - void setOperationResourceId(IIdType theOperationResourceId); - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/BaseRestfulResponse.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/BaseRestfulResponse.java index e2cec8df3df..28a30e0df8e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/BaseRestfulResponse.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/BaseRestfulResponse.java @@ -20,30 +20,20 @@ package ca.uhn.fhir.rest.server; * #L% */ -import ca.uhn.fhir.rest.api.SummaryEnum; import ca.uhn.fhir.rest.api.server.IRestfulResponse; import ca.uhn.fhir.rest.api.server.RequestDetails; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.instance.model.api.IPrimitiveType; -import java.io.IOException; import java.util.ArrayList; -import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; public abstract class BaseRestfulResponse implements IRestfulResponse { - - private IIdType myOperationResourceId; - private IPrimitiveType myOperationResourceLastUpdated; private final Map> myHeaders = new HashMap<>(); - private T theRequestDetails; + private T myRequestDetails; - public BaseRestfulResponse(T requestDetails) { - this.theRequestDetails = requestDetails; + public BaseRestfulResponse(T theRequestDetails) { + this.myRequestDetails = theRequestDetails; } @Override @@ -53,6 +43,7 @@ public abstract class BaseRestfulResponse implements I /** * Get the http headers + * * @return the headers */ @Override @@ -62,36 +53,20 @@ public abstract class BaseRestfulResponse implements I /** * Get the requestDetails + * * @return the requestDetails */ public T getRequestDetails() { - return theRequestDetails; - } - - @Override - public void setOperationResourceId(IIdType theOperationResourceId) { - myOperationResourceId = theOperationResourceId; - } - - @Override - public void setOperationResourceLastUpdated(IPrimitiveType theOperationResourceLastUpdated) { - myOperationResourceLastUpdated = theOperationResourceLastUpdated; + return myRequestDetails; } /** * Set the requestDetails + * * @param requestDetails the requestDetails to set */ public void setRequestDetails(T requestDetails) { - this.theRequestDetails = requestDetails; - } - - @Override - public final Object streamResponseAsResource(IBaseResource theResource, boolean thePrettyPrint, Set theSummaryMode, - int theStatusCode, String theStatusMessage, boolean theRespondGzip, boolean theAddContentLocation) - throws IOException { - return RestfulServerUtils.streamResponseAsResource(theRequestDetails.getServer(), theResource, theSummaryMode, theStatusCode, theStatusMessage, theAddContentLocation, theRespondGzip, getRequestDetails(), myOperationResourceId, myOperationResourceLastUpdated); - + this.myRequestDetails = requestDetails; } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index 9d4232a130c..12b0258a5de 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -60,6 +60,7 @@ import ca.uhn.fhir.rest.server.method.MethodMatchEnum; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.tenant.ITenantIdentificationStrategy; import ca.uhn.fhir.util.CoverageIgnore; +import ca.uhn.fhir.util.IoUtil; import ca.uhn.fhir.util.OperationOutcomeUtil; import ca.uhn.fhir.util.ReflectionUtil; import ca.uhn.fhir.util.UrlPathTokenizer; @@ -84,6 +85,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.Closeable; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.Writer; @@ -1166,16 +1168,13 @@ public class RestfulServer extends HttpServlet implements IRestfulServer TEXT_ENCODE_ELEMENTS = new HashSet<>(Arrays.asList("*.text", "*.id", "*.meta", "*.(mandatory)")); + private static final HashSet TEXT_ENCODE_ELEMENTS = new HashSet<>(Arrays.asList("*.text", "*.id", "*.meta", "*.(mandatory)" )); private static Map myFhirContextMap = Collections.synchronizedMap(new HashMap<>()); private static EnumSet ourOperationsWhichAllowPreferHeader = EnumSet.of(RestOperationTypeEnum.CREATE, RestOperationTypeEnum.UPDATE, RestOperationTypeEnum.PATCH); @@ -155,6 +157,7 @@ public class RestfulServerUtils { } } + @SuppressWarnings("EnumSwitchStatementWhichMissesCases" ) public static void configureResponseParser(RequestDetails theRequestDetails, IParser parser) { // Pretty print boolean prettyPrint = RestfulServerUtils.prettyPrintResponse(theRequestDetails.getServer(), theRequestDetails); @@ -168,7 +171,7 @@ public class RestfulServerUtils { // _elements Set elements = ElementsParameter.getElementsValueOrNull(theRequestDetails, false); if (elements != null && !summaryMode.equals(Collections.singleton(SummaryEnum.FALSE))) { - throw new InvalidRequestException(Msg.code(304) + "Cannot combine the " + Constants.PARAM_SUMMARY + " and " + Constants.PARAM_ELEMENTS + " parameters"); + throw new InvalidRequestException(Msg.code(304) + "Cannot combine the " + Constants.PARAM_SUMMARY + " and " + Constants.PARAM_ELEMENTS + " parameters" ); } // _elements:exclude @@ -186,7 +189,7 @@ public class RestfulServerUtils { } if (summaryModeCount) { - parser.setEncodeElements(Sets.newHashSet("Bundle.total", "Bundle.type")); + parser.setEncodeElements(Sets.newHashSet("Bundle.total", "Bundle.type" )); } else if (summaryMode.contains(SummaryEnum.TEXT) && summaryMode.size() == 1) { parser.setEncodeElements(TEXT_ENCODE_ELEMENTS); parser.setEncodeElementsAppliesToChildResourcesOnly(true); @@ -222,7 +225,7 @@ public class RestfulServerUtils { */ boolean haveExplicitBundleElement = false; for (String next : newElements) { - if (next.startsWith("Bundle.")) { + if (next.startsWith("Bundle." )) { haveExplicitBundleElement = true; break; } @@ -263,7 +266,7 @@ public class RestfulServerUtils { if (isNotBlank(theRequest.getRequestPath())) { b.append('/'); - if (isNotBlank(theRequest.getTenantId()) && theRequest.getRequestPath().startsWith(theRequest.getTenantId() + "/")) { + if (isNotBlank(theRequest.getTenantId()) && theRequest.getRequestPath().startsWith(theRequest.getTenantId() + "/" )) { b.append(theRequest.getRequestPath().substring(theRequest.getTenantId().length() + 1)); } else { b.append(theRequest.getRequestPath()); @@ -300,7 +303,7 @@ public class RestfulServerUtils { if (isNotBlank(requestPath)) { b.append('/'); - if (isNotBlank(tenantId) && requestPath.startsWith(tenantId + "/")) { + if (isNotBlank(tenantId) && requestPath.startsWith(tenantId + "/" )) { b.append(requestPath.substring(tenantId.length() + 1)); } else { b.append(requestPath); @@ -374,7 +377,7 @@ public class RestfulServerUtils { b.append(Constants.PARAM_FORMAT); b.append('='); String format = strings[0]; - format = replace(format, " ", "+"); + format = replace(format, " ", "+" ); b.append(UrlUtil.escapeUrlParam(format)); } if (theBundleLinks.prettyPrint) { @@ -412,7 +415,7 @@ public class RestfulServerUtils { .stream() .sorted() .map(UrlUtil::escapeUrlParam) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("," )); b.append(nextValue); } @@ -427,7 +430,7 @@ public class RestfulServerUtils { .stream() .sorted() .map(UrlUtil::escapeUrlParam) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("," )); b.append(nextValue); } } @@ -458,7 +461,7 @@ public class RestfulServerUtils { while (acceptValues.hasNext() && retVal == null) { String nextAcceptHeaderValue = acceptValues.next(); if (nextAcceptHeaderValue != null && isNotBlank(nextAcceptHeaderValue)) { - for (String nextPart : nextAcceptHeaderValue.split(",")) { + for (String nextPart : nextAcceptHeaderValue.split("," )) { int scIdx = nextPart.indexOf(';'); if (scIdx == 0) { continue; @@ -533,7 +536,7 @@ public class RestfulServerUtils { ResponseEncoding retVal = null; if (acceptValues != null) { for (String nextAcceptHeaderValue : acceptValues) { - StringTokenizer tok = new StringTokenizer(nextAcceptHeaderValue, ","); + StringTokenizer tok = new StringTokenizer(nextAcceptHeaderValue, "," ); while (tok.hasMoreTokens()) { String nextToken = tok.nextToken(); int startSpaceIndex = -1; @@ -567,14 +570,14 @@ public class RestfulServerUtils { } else { encoding = getEncodingForContentType(theReq.getServer().getFhirContext(), strict, nextToken.substring(startSpaceIndex, endSpaceIndex), thePreferContentType); String remaining = nextToken.substring(endSpaceIndex + 1); - StringTokenizer qualifierTok = new StringTokenizer(remaining, ";"); + StringTokenizer qualifierTok = new StringTokenizer(remaining, ";" ); while (qualifierTok.hasMoreTokens()) { String nextQualifier = qualifierTok.nextToken(); int equalsIndex = nextQualifier.indexOf('='); if (equalsIndex != -1) { String nextQualifierKey = nextQualifier.substring(0, equalsIndex).trim(); String nextQualifierValue = nextQualifier.substring(equalsIndex + 1, nextQualifier.length()).trim(); - if (nextQualifierKey.equals("q")) { + if (nextQualifierKey.equals("q" )) { try { q = Float.parseFloat(nextQualifierValue); q = Math.max(q, 0.0f); @@ -814,7 +817,7 @@ public class RestfulServerUtils { PreferHeader retVal = new PreferHeader(); if (isNotBlank(theValue)) { - StringTokenizer tok = new StringTokenizer(theValue, ";,"); + StringTokenizer tok = new StringTokenizer(theValue, ";," ); while (tok.hasMoreTokens()) { String next = trim(tok.nextToken()); int eqIndex = next.indexOf('='); @@ -876,7 +879,7 @@ public class RestfulServerUtils { List acceptValues = theRequest.getHeaders(Constants.HEADER_ACCEPT); if (acceptValues != null) { for (String nextAcceptHeaderValue : acceptValues) { - if (nextAcceptHeaderValue.contains("pretty=true")) { + if (nextAcceptHeaderValue.contains("pretty=true" )) { prettyPrint = true; } } @@ -885,12 +888,12 @@ public class RestfulServerUtils { return prettyPrint; } - public static Object streamResponseAsResource(IRestfulServerDefaults theServer, IBaseResource theResource, Set theSummaryMode, int stausCode, boolean theAddContentLocationHeader, + public static Object streamResponseAsResource(IRestfulServerDefaults theServer, IBaseResource theResource, Set theSummaryMode, int theStatusCode, boolean theAddContentLocationHeader, boolean respondGzip, RequestDetails theRequestDetails) throws IOException { - return streamResponseAsResource(theServer, theResource, theSummaryMode, stausCode, null, theAddContentLocationHeader, respondGzip, theRequestDetails, null, null); + return streamResponseAsResource(theServer, theResource, theSummaryMode, theStatusCode, theAddContentLocationHeader, respondGzip, theRequestDetails, null, null); } - public static Object streamResponseAsResource(IRestfulServerDefaults theServer, IBaseResource theResource, Set theSummaryMode, int theStatusCode, String theStatusMessage, + public static Object streamResponseAsResource(IRestfulServerDefaults theServer, IBaseResource theResource, Set theSummaryMode, int theStatusCode, boolean theAddContentLocationHeader, boolean respondGzip, RequestDetails theRequestDetails, IIdType theOperationResourceId, IPrimitiveType theOperationResourceLastUpdated) throws IOException { IRestfulResponse response = theRequestDetails.getResponse(); @@ -954,9 +957,18 @@ public class RestfulServerUtils { // Force binary resources to download - This is a security measure to prevent // malicious images or HTML blocks being served up as content. contentType = getBinaryContentTypeOrDefault(bin); - response.addHeader(Constants.HEADER_CONTENT_DISPOSITION, "Attachment;"); + response.addHeader(Constants.HEADER_CONTENT_DISPOSITION, "Attachment;" ); - return response.sendAttachmentResponse(bin, theStatusCode, contentType); + Integer contentLength = null; + if (bin.hasData()) { + contentLength = bin.getContent().length; + } + + OutputStream outputStream = response.getResponseOutputStream(theStatusCode, contentType, contentLength); + if (bin.hasData()) { + outputStream.write(bin.getContent()); + } + return response.commitResponse(outputStream); } } @@ -1004,7 +1016,7 @@ public class RestfulServerUtils { } String charset = Constants.CHARSET_NAME_UTF8; - Writer writer = response.getResponseWriter(theStatusCode, theStatusMessage, contentType, charset, respondGzip); + Writer writer = response.getResponseWriter(theStatusCode, contentType, charset, respondGzip); // Interceptor call: SERVER_OUTGOING_WRITER_CREATED if (theServer.getInterceptorService() != null && theServer.getInterceptorService().hasHooks(Pointcut.SERVER_OUTGOING_WRITER_CREATED)) { @@ -1036,7 +1048,7 @@ public class RestfulServerUtils { parser.encodeResourceToWriter(theResource, writer); } - return response.sendWriterResponse(theStatusCode, contentType, charset, writer); + return response.commitResponse(writer); } private static String getBinaryContentTypeOrDefault(IBaseBinary theBinary) { @@ -1057,14 +1069,15 @@ public class RestfulServerUtils { * - Otherwise, return false. * * @param theResponseEncoding the requested {@link EncodingEnum} determined by the incoming Content-Type header. - * @param theBinary the {@link IBaseBinary} resource to be streamed out. + * @param theBinary the {@link IBaseBinary} resource to be streamed out. * @return True if response can be streamed as the requested encoding type, false otherwise. */ private static boolean shouldStreamContents(ResponseEncoding theResponseEncoding, IBaseBinary theBinary) { String contentType = theBinary.getContentType(); if (theResponseEncoding == null) { return true; - } if (isBlank(contentType)) { + } + if (isBlank(contentType)) { return Constants.CT_OCTET_STREAM.equals(theResponseEncoding.getContentType()); } else if (contentType.equalsIgnoreCase(theResponseEncoding.getContentType())) { return true; @@ -1092,7 +1105,7 @@ public class RestfulServerUtils { public static void validateResourceListNotNull(List theResourceList) { if (theResourceList == null) { - throw new InternalErrorException(Msg.code(306) + "IBundleProvider returned a null list of resources - This is not allowed"); + throw new InternalErrorException(Msg.code(306) + "IBundleProvider returned a null list of resources - This is not allowed" ); } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptor.java index aa79b3713b0..d2f70eb78b8 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptor.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -39,6 +40,7 @@ import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.parser.DataFormatException; +import ca.uhn.fhir.rest.server.RestfulServerUtils; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; import ca.uhn.fhir.rest.server.servlet.ServletRestfulResponse; @@ -61,12 +63,12 @@ public class ExceptionHandlingInterceptor { public static final String PROCESSING = Constants.OO_INFOSTATUS_PROCESSING; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionHandlingInterceptor.class); + public static final Set SUMMARY_MODE = Collections.singleton(SummaryEnum.FALSE); private Class[] myReturnStackTracesForExceptionTypes; @Hook(Pointcut.SERVER_HANDLE_EXCEPTION) public boolean handleException(RequestDetails theRequestDetails, BaseServerResponseException theException, HttpServletRequest theRequest, HttpServletResponse theResponse) throws ServletException, IOException { - Closeable writer = (Closeable) handleException(theRequestDetails, theException); - writer.close(); + handleException(theRequestDetails, theException); return false; } @@ -110,8 +112,7 @@ public class ExceptionHandlingInterceptor { ourLog.error("HAPI-FHIR was unable to reset the output stream during exception handling. The root causes follows:", t); } - return response.streamResponseAsResource(oo, true, Collections.singleton(SummaryEnum.FALSE), statusCode, statusMessage, false, false); - + return RestfulServerUtils.streamResponseAsResource(theRequestDetails.getServer(), oo, SUMMARY_MODE, statusCode, false, false, theRequestDetails, null, null); } /** diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java index d8fe1daf30a..68b5353d2de 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java @@ -36,10 +36,12 @@ import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.instance.model.api.IPrimitiveType; import java.io.IOException; import java.lang.reflect.Method; import java.util.Collections; +import java.util.Date; import java.util.Set; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @@ -219,12 +221,14 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding { IRestfulResponse restfulResponse = theRequest.getResponse(); + IIdType responseId = null; + IPrimitiveType operationResourceLastUpdated = null; if (theMethodOutcome != null) { if (theMethodOutcome.getResource() != null) { - restfulResponse.setOperationResourceLastUpdated(RestfulServerUtils.extractLastUpdatedFromResource(theMethodOutcome.getResource())); + operationResourceLastUpdated = RestfulServerUtils.extractLastUpdatedFromResource(theMethodOutcome.getResource()); } - IIdType responseId = theMethodOutcome.getId(); + responseId = theMethodOutcome.getId(); if (responseId != null && responseId.getResourceType() == null && responseId.hasIdPart()) { responseId = responseId.withResourceType(getResourceName()); } @@ -232,14 +236,11 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding { if (responseId != null) { String serverBase = theRequest.getFhirServerBase(); responseId = RestfulServerUtils.fullyQualifyResourceIdOrReturnNull(theServer, theMethodOutcome.getResource(), serverBase, responseId); - restfulResponse.setOperationResourceId(responseId); } } - boolean prettyPrint = RestfulServerUtils.prettyPrintResponse(theServer, theRequest); Set summaryMode = Collections.emptySet(); - - return restfulResponse.streamResponseAsResource(responseDetails.getResponseResource(), prettyPrint, summaryMode, responseDetails.getResponseCode(), null, theRequest.isRespondGzip(), true); + return RestfulServerUtils.streamResponseAsResource(theServer, responseDetails.getResponseResource(), summaryMode, responseDetails.getResponseCode(), true, theRequest.isRespondGzip(), theRequest, responseId, operationResourceLastUpdated); } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java index 9f43fbdd7e1..6331e8c8276 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java @@ -445,9 +445,8 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi if (!callOutgoingResponseHook(theRequest, responseDetails)) { return null; } - boolean prettyPrint = RestfulServerUtils.prettyPrintResponse(theServer, theRequest); - return theRequest.getResponse().streamResponseAsResource(responseDetails.getResponseResource(), prettyPrint, summaryMode, responseDetails.getResponseCode(), null, theRequest.isRespondGzip(), isAddContentLocationHeader()); + return RestfulServerUtils.streamResponseAsResource(theServer, responseDetails.getResponseResource(), summaryMode, responseDetails.getResponseCode(), isAddContentLocationHeader(), theRequest.isRespondGzip(), theRequest, null, null); } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/GraphQLMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/GraphQLMethodBinding.java index b896a14dd30..dc45bedb0ca 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/GraphQLMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/GraphQLMethodBinding.java @@ -167,7 +167,7 @@ public class GraphQLMethodBinding extends OperationMethodBinding { } // Write the response - Writer writer = theRequest.getResponse().getResponseWriter(statusCode, statusMessage, contentType, charset, respondGzip); + Writer writer = theRequest.getResponse().getResponseWriter(statusCode, contentType, charset, respondGzip); writer.write(responseString); writer.close(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponse.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponse.java index 4b95e1a5d5d..9e8f9b6e047 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponse.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponse.java @@ -21,14 +21,15 @@ package ca.uhn.fhir.rest.server.servlet; */ import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.server.BaseParseAction; import ca.uhn.fhir.rest.server.BaseRestfulResponse; +import ca.uhn.fhir.util.IoUtil; import org.apache.commons.lang3.StringUtils; -import org.hl7.fhir.instance.model.api.IBaseBinary; +import org.apache.commons.lang3.Validate; +import javax.annotation.Nonnull; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; +import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -40,6 +41,9 @@ import java.util.zip.GZIPOutputStream; public class ServletRestfulResponse extends BaseRestfulResponse { + private Writer myWriter; + private OutputStream myOutputStream; + /** * Constructor */ @@ -47,24 +51,29 @@ public class ServletRestfulResponse extends BaseRestfulResponse> header : getHeaders().entrySet()) { String key = header.getKey(); key = sanitizeHeaderField(key); @@ -91,27 +102,23 @@ public class ServletRestfulResponse extends BaseRestfulResponse outcome, int operationStatus, boolean allowPrefer, MethodOutcome response, String resourceName) throws IOException { - addHeaders(); - return getRequestDetails().getServer().returnResponse(getRequestDetails(), outcome, operationStatus, allowPrefer, response, resourceName); - } } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponseTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponseTest.java index 41135dbaa14..bb28d105053 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponseTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/servlet/ServletRestfulResponseTest.java @@ -46,7 +46,7 @@ public class ServletRestfulResponseTest { response.addHeader("Authorization", "Bearer"); response.addHeader("Cache-Control", "no-cache, no-store"); - response.getResponseWriter(200, "Status", "text/plain", "UTF-8", false); + response.getResponseWriter(200, "text/plain", "UTF-8", false); final InOrder orderVerifier = Mockito.inOrder(servletResponse); orderVerifier.verify(servletResponse).setHeader(eq("Authorization"), eq("Basic")); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4Test.java similarity index 57% rename from hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4.java rename to hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4Test.java index 3a22e18df5e..4d17818a999 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/CustomTypeServerR4Test.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.rest.server; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.rest.annotation.ConditionalUrlParam; @@ -11,54 +12,38 @@ import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.test.utilities.JettyUtil; +import ca.uhn.fhir.test.utilities.HttpClientExtension; +import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; import ca.uhn.fhir.util.TestUtil; import org.apache.commons.io.IOUtils; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpPost; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -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.r4.model.IdType; import org.hl7.fhir.r4.model.OperationOutcome; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.assertEquals; -public class CustomTypeServerR4 { - private static CloseableHttpClient ourClient; - - private static FhirContext ourCtx = FhirContext.forR4(); - private static String ourLastConditionalUrl; - private static IdType ourLastId; - private static IdType ourLastIdParam; - private static boolean ourLastRequestWasSearch; - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(CustomTypeServerR4.class); - private static int ourPort; - private static Server ourServer; - - @BeforeEach - public void before() { - ourLastId = null; - ourLastConditionalUrl = null; - ourLastIdParam = null; - ourLastRequestWasSearch = false; - } +public class CustomTypeServerR4Test { + private static FhirContext ourCtx = FhirContext.forR4Cached(); + @RegisterExtension + private static RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) + .registerProvider(new PatientProvider()) + .keepAliveBetweenTests(); + @RegisterExtension + private static HttpClientExtension ourClient = new HttpClientExtension(); + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(CustomTypeServerR4Test.class); @Test public void testCreateWithIdInBody() throws Exception { @@ -67,8 +52,7 @@ public class CustomTypeServerR4 { patient.setId("2"); patient.addIdentifier().setValue("002"); - HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient"); -// httpPost.addHeader(Constants.HEADER_IF_NONE_EXIST, "Patient?identifier=system%7C001"); + HttpPost httpPost = new HttpPost(ourServer.getBaseUrl() + "/Patient"); httpPost.setEntity(new StringEntity(ourCtx.newXmlParser().encodeResourceToString(patient), ContentType.create(Constants.CT_FHIR_XML, "UTF-8"))); HttpResponse status = ourClient.execute(httpPost); @@ -87,8 +71,7 @@ public class CustomTypeServerR4 { Patient patient = new Patient(); patient.addIdentifier().setValue("002"); - HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/2"); -// httpPost.addHeader(Constants.HEADER_IF_NONE_EXIST, "Patient?identifier=system%7C001"); + HttpPost httpPost = new HttpPost(ourServer.getBaseUrl() + "/Patient/2"); httpPost.setEntity(new StringEntity(ourCtx.newXmlParser().encodeResourceToString(patient), ContentType.create(Constants.CT_FHIR_XML, "UTF-8"))); HttpResponse status = ourClient.execute(httpPost); @@ -100,7 +83,7 @@ public class CustomTypeServerR4 { assertEquals(400, status.getStatusLine().getStatusCode()); OperationOutcome oo = ourCtx.newXmlParser().parseResource(OperationOutcome.class, responseContent); - assertEquals("Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)", oo.getIssue().get(0).getDiagnostics()); + assertEquals(Msg.code(365) + "Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)", oo.getIssue().get(0).getDiagnostics()); } @Test @@ -109,7 +92,7 @@ public class CustomTypeServerR4 { Patient patient = new Patient(); patient.addIdentifier().setValue("002"); - HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/2"); + HttpPost httpPost = new HttpPost(ourServer.getBaseUrl() + "/Patient/2"); httpPost.addHeader(Constants.HEADER_IF_NONE_EXIST, "Patient?identifier=system%7C001"); httpPost.setEntity(new StringEntity(ourCtx.newXmlParser().encodeResourceToString(patient), ContentType.create(Constants.CT_FHIR_XML, "UTF-8"))); @@ -122,45 +105,19 @@ public class CustomTypeServerR4 { assertEquals(400, status.getStatusLine().getStatusCode()); OperationOutcome oo = ourCtx.newXmlParser().parseResource(OperationOutcome.class, responseContent); - assertEquals("Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)", oo.getIssue().get(0).getDiagnostics()); + assertEquals(Msg.code(365) + "Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)", oo.getIssue().get(0).getDiagnostics()); } @AfterAll public static void afterClassClearContext() throws Exception { - JettyUtil.closeServer(ourServer); TestUtil.randomizeLocaleAndTimezone(); } - @BeforeAll - public static void beforeClass() throws Exception { - ourServer = new Server(0); - - PatientProvider patientProvider = new PatientProvider(); - - ServletHandler proxyHandler = new ServletHandler(); - RestfulServer servlet = new RestfulServer(ourCtx); - servlet.setResourceProviders(patientProvider); - ServletHolder servletHolder = new ServletHolder(servlet); - proxyHandler.addServletWithMapping(servletHolder, "/*"); - ourServer.setHandler(proxyHandler); - JettyUtil.startServer(ourServer); - ourPort = JettyUtil.getPortForStartedServer(ourServer); - - PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); - HttpClientBuilder builder = HttpClientBuilder.create(); - builder.setConnectionManager(connectionManager); - ourClient = builder.build(); - - } - public static class PatientProvider implements IResourceProvider { @Create() public MethodOutcome createPatient(@ResourceParam Patient thePatient, @ConditionalUrlParam String theConditional, @IdParam IdType theIdParam) { - ourLastConditionalUrl = theConditional; - ourLastId = thePatient.getIdElement(); - ourLastIdParam = theIdParam; return new MethodOutcome(new IdType("Patient/001/_history/002")); } @@ -171,7 +128,6 @@ public class CustomTypeServerR4 { @Search public List search(@OptionalParam(name = "foo") StringDt theString) { - ourLastRequestWasSearch = true; return new ArrayList<>(); } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/OperationServerR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/OperationServerR4Test.java index ad4f75cbc6b..2632a60f767 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/OperationServerR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/OperationServerR4Test.java @@ -13,7 +13,8 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.test.utilities.JettyUtil; +import ca.uhn.fhir.test.utilities.HttpClientExtension; +import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; import ca.uhn.fhir.util.TestUtil; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; @@ -25,30 +26,15 @@ import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -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.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; -import org.hl7.fhir.r4.model.Binary; -import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.CapabilityStatement; -import org.hl7.fhir.r4.model.IdType; -import org.hl7.fhir.r4.model.IntegerType; -import org.hl7.fhir.r4.model.MoneyQuantity; -import org.hl7.fhir.r4.model.OperationDefinition; +import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.OperationDefinition.OperationParameterUse; -import org.hl7.fhir.r4.model.Parameters; -import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.StringType; -import org.hl7.fhir.r4.model.UnsignedIntType; import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -56,7 +42,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInRelativeOrder; @@ -68,10 +53,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; public class OperationServerR4Test { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(OperationServerR4Test.class); private static final String TEXT_HTML = "text/html"; - private static CloseableHttpClient ourClient; - private static FhirContext ourCtx; + private static final FhirContext ourCtx = FhirContext.forR4Cached(); + @RegisterExtension + private static final RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) + .registerProvider(new PatientProvider()) + .registerProvider(new PlainProvider()) + .withPagingProvider(new FifoMemoryPagingProvider(10).setDefaultPageSize(2)) + .withDefaultResponseEncoding(EncodingEnum.XML); + @RegisterExtension + private static final HttpClientExtension ourClient = new HttpClientExtension(); private static IdType ourLastId; private static String ourLastMethod; private static StringType ourLastParam1; @@ -79,8 +72,6 @@ public class OperationServerR4Test { private static List ourLastParam3; private static MoneyQuantity ourLastParamMoney1; private static UnsignedIntType ourLastParamUnsignedInt1; - private static int ourPort; - private static Server ourServer; private static IBaseResource ourNextResponse; private static RestOperationTypeEnum ourLastRestOperation; private IGenericClient myFhirClient; @@ -97,7 +88,7 @@ public class OperationServerR4Test { ourNextResponse = null; ourLastRestOperation = null; - myFhirClient = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort); + myFhirClient = ourServer.getFhirClient(); } @Test @@ -109,13 +100,13 @@ public class OperationServerR4Test { CapabilityStatement p = myFhirClient.fetchConformance().ofType(CapabilityStatement.class).prettyPrint().execute(); ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(p)); - List ops = p.getRestFirstRep().getResource().stream().filter(t->t.getType().equals("Patient")).findFirst().orElseThrow(()->new IllegalArgumentException()).getOperation(); + List ops = p.getRestFirstRep().getResource().stream().filter(t -> t.getType().equals("Patient" )).findFirst().orElseThrow(() -> new IllegalArgumentException()).getOperation(); assertThat(ops.size(), greaterThan(1)); List opNames = toOpNames(ops); - assertThat(opNames.toString(), opNames, containsInRelativeOrder("OP_TYPE")); + assertThat(opNames.toString(), opNames, containsInRelativeOrder("OP_TYPE" )); - OperationDefinition def = myFhirClient.read().resource(OperationDefinition.class).withId(ops.get(opNames.indexOf("OP_TYPE")).getDefinition()).execute(); + OperationDefinition def = myFhirClient.read().resource(OperationDefinition.class).withId(ops.get(opNames.indexOf("OP_TYPE" )).getDefinition()).execute(); assertEquals("OP_TYPE", def.getCode()); } @@ -124,7 +115,7 @@ public class OperationServerR4Test { */ @Test public void testOperationDefinition() { - OperationDefinition def = myFhirClient.read().resource(OperationDefinition.class).withId("OperationDefinition/Patient-t-OP_TYPE").execute(); + OperationDefinition def = myFhirClient.read().resource(OperationDefinition.class).withId("OperationDefinition/Patient-t-OP_TYPE" ).execute(); ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(def)); @@ -171,12 +162,12 @@ public class OperationServerR4Test { ourNextResponse = bundle; Patient patient = new Patient(); - patient.addName().setFamily("FAMILY").addGiven("GIVEN"); - patient.addIdentifier().setSystem("SYSTEM").setValue("VALUE"); + patient.addName().setFamily("FAMILY" ).addGiven("GIVEN" ); + patient.addIdentifier().setSystem("SYSTEM" ).setValue("VALUE" ); bundle.addEntry().setResource(patient); - HttpGet httpPost = new HttpGet("http://localhost:" + ourPort + "/Patient/$OP_TYPE_RETURNING_BUNDLE" - + "?_pretty=true&_elements=identifier"); + HttpGet httpPost = new HttpGet(ourServer.getBaseUrl() + "/Patient/$OP_TYPE_RETURNING_BUNDLE" + + "?_pretty=true&_elements=identifier" ); try (CloseableHttpResponse status = ourClient.execute(httpPost)) { assertEquals(200, status.getStatusLine().getStatusCode()); @@ -194,7 +185,7 @@ public class OperationServerR4Test { public void testManualResponseWithPrimitiveParam() throws Exception { // Try with a GET - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123/$manualResponseWithPrimitiveParam?path=THIS_IS_A_PATH"); + HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Patient/123/$manualResponseWithPrimitiveParam?path=THIS_IS_A_PATH" ); try (CloseableHttpResponse status = ourClient.execute(httpGet)) { assertEquals(200, status.getStatusLine().getStatusCode()); String response = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); @@ -210,11 +201,11 @@ public class OperationServerR4Test { public void testInstanceEverythingGet() throws Exception { // Try with a GET - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123/$everything"); + HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Patient/123/$everything" ); try (CloseableHttpResponse status = ourClient.execute(httpGet)) { assertEquals(200, status.getStatusLine().getStatusCode()); String response = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); - assertThat(response, startsWith(" theParam3, @OperationParam(name = "PARAM4", min = 1) List theParam4 ) { @@ -805,27 +796,27 @@ public class OperationServerR4Test { ourLastParam2 = theParam2; Parameters retVal = new Parameters(); - retVal.addParameter().setName("RET1").setValue(new StringType("RETVAL1")); + retVal.addParameter().setName("RET1" ).setValue(new StringType("RETVAL1" )); return retVal; } @Operation(name = "$OP_TYPE_ONLY_STRING", idempotent = true) public Parameters opTypeOnlyString( - @OperationParam(name = "PARAM1") StringType theParam1 + @OperationParam(name = "PARAM1" ) StringType theParam1 ) { ourLastMethod = "$OP_TYPE"; ourLastParam1 = theParam1; Parameters retVal = new Parameters(); - retVal.addParameter().setName("RET1").setValue(new StringType("RETVAL1")); + retVal.addParameter().setName("RET1" ).setValue(new StringType("RETVAL1" )); return retVal; } - @Operation(name = "$OP_TYPE_RET_BUNDLE") + @Operation(name = "$OP_TYPE_RET_BUNDLE" ) public Bundle opTypeRetBundle( - @OperationParam(name = "PARAM1") StringType theParam1, - @OperationParam(name = "PARAM2") Patient theParam2 + @OperationParam(name = "PARAM1" ) StringType theParam1, + @OperationParam(name = "PARAM2" ) Patient theParam2 ) { ourLastMethod = "$OP_TYPE_RET_BUNDLE"; @@ -833,7 +824,7 @@ public class OperationServerR4Test { ourLastParam2 = theParam2; Bundle retVal = new Bundle(); - retVal.addEntry().getResponse().setStatus("100"); + retVal.addEntry().getResponse().setStatus("100" ); return retVal; } @@ -858,7 +849,7 @@ public class OperationServerR4Test { @Operation(name = "$manualInputAndOutputWithParam", manualResponse = true, manualRequest = true) public void manualInputAndOutputWithParam( - @OperationParam(name = "param1") StringType theParam1, + @OperationParam(name = "param1" ) StringType theParam1, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse ) throws IOException { @@ -938,10 +929,10 @@ public class OperationServerR4Test { theServletResponse.setStatus(200); } - @Operation(name = "$OP_SERVER") + @Operation(name = "$OP_SERVER" ) public Parameters opServer( - @OperationParam(name = "PARAM1") StringType theParam1, - @OperationParam(name = "PARAM2") Patient theParam2 + @OperationParam(name = "PARAM1" ) StringType theParam1, + @OperationParam(name = "PARAM2" ) Patient theParam2 ) { ourLastMethod = "$OP_SERVER"; @@ -949,14 +940,14 @@ public class OperationServerR4Test { ourLastParam2 = theParam2; Parameters retVal = new Parameters(); - retVal.addParameter().setName("RET1").setValue(new StringType("RETVAL1")); + retVal.addParameter().setName("RET1" ).setValue(new StringType("RETVAL1" )); return retVal; } - @Operation(name = "$OP_SERVER_WITH_RAW_STRING") + @Operation(name = "$OP_SERVER_WITH_RAW_STRING" ) public Parameters opServer( - @OperationParam(name = "PARAM1") String theParam1, - @OperationParam(name = "PARAM2") Patient theParam2 + @OperationParam(name = "PARAM1" ) String theParam1, + @OperationParam(name = "PARAM2" ) Patient theParam2 ) { ourLastMethod = "$OP_SERVER"; @@ -964,14 +955,14 @@ public class OperationServerR4Test { ourLastParam2 = theParam2; Parameters retVal = new Parameters(); - retVal.addParameter().setName("RET1").setValue(new StringType("RETVAL1")); + retVal.addParameter().setName("RET1" ).setValue(new StringType("RETVAL1" )); return retVal; } - @Operation(name = "$OP_SERVER_LIST_PARAM") + @Operation(name = "$OP_SERVER_LIST_PARAM" ) public Parameters opServerListParam( - @OperationParam(name = "PARAM2") Patient theParam2, - @OperationParam(name = "PARAM3") List theParam3 + @OperationParam(name = "PARAM2" ) Patient theParam2, + @OperationParam(name = "PARAM3" ) List theParam3 ) { ourLastMethod = "$OP_SERVER_LIST_PARAM"; @@ -979,7 +970,7 @@ public class OperationServerR4Test { ourLastParam3 = theParam3; Parameters retVal = new Parameters(); - retVal.addParameter().setName("RET1").setValue(new StringType("RETVAL1")); + retVal.addParameter().setName("RET1" ).setValue(new StringType("RETVAL1" )); return retVal; } @@ -1001,36 +992,7 @@ public class OperationServerR4Test { @AfterAll public static void afterClassClearContext() throws Exception { - JettyUtil.closeServer(ourServer); TestUtil.randomizeLocaleAndTimezone(); } - @BeforeAll - public static void beforeClass() throws Exception { - ourCtx = FhirContext.forR4(); - ourServer = new Server(0); - - ServletHandler proxyHandler = new ServletHandler(); - RestfulServer servlet = new RestfulServer(ourCtx); - - servlet.setDefaultResponseEncoding(EncodingEnum.XML); - servlet.setPagingProvider(new FifoMemoryPagingProvider(10).setDefaultPageSize(2)); - - servlet.setFhirContext(ourCtx); - PlainProvider plainProvider = new PlainProvider(); - PatientProvider patientProvider = new PatientProvider(); - servlet.registerProviders(patientProvider, plainProvider); - ServletHolder servletHolder = new ServletHolder(servlet); - proxyHandler.addServletWithMapping(servletHolder, "/*"); - ourServer.setHandler(proxyHandler); - JettyUtil.startServer(ourServer); - ourPort = JettyUtil.getPortForStartedServer(ourServer); - - PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); - HttpClientBuilder builder = HttpClientBuilder.create(); - builder.setConnectionManager(connectionManager); - ourClient = builder.build(); - - } - } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerConcurrencyTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerConcurrencyTest.java new file mode 100644 index 00000000000..45ed11d0b3e --- /dev/null +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerConcurrencyTest.java @@ -0,0 +1,199 @@ +package ca.uhn.fhir.rest.server; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.annotation.Create; +import ca.uhn.fhir.rest.annotation.ResourceParam; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.rest.api.RequestTypeEnum; +import ca.uhn.fhir.test.utilities.HttpClientExtension; +import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; +import com.helger.commons.collection.iterate.EmptyEnumeration; +import org.apache.commons.collections4.iterators.IteratorEnumeration; +import org.apache.commons.lang3.RandomStringUtils; +import org.hl7.fhir.r4.model.IdType; +import org.hl7.fhir.r4.model.OperationOutcome; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.util.Assert; + +import javax.annotation.Nonnull; +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.ByteArrayInputStream; +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.HashMap; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class ServerConcurrencyTest { + + private static final FhirContext ourCtx = FhirContext.forR4Cached(); + private static final Logger ourLog = LoggerFactory.getLogger(ServerConcurrencyTest.class); + @RegisterExtension + private static final RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) + .registerProvider(new MyPatientProvider()); + @RegisterExtension + private final HttpClientExtension myHttpClient = new HttpClientExtension(); + + @Mock + private HttpServletRequest myRequest; + @Mock + private HttpServletResponse myResponse; + @Mock + private PrintWriter myWriter; + private HashMap myHeaders; + + @Test + public void testExceptionClosingInputStream() throws IOException { + initRequestMocks(); + DelegatingServletInputStream inputStream = createMockPatientBodyServletInputStream(); + inputStream.setExceptionOnClose(true); + when(myRequest.getInputStream()).thenReturn(inputStream); + when(myResponse.getWriter()).thenReturn(myWriter); + + assertDoesNotThrow(() -> + ourServer.getRestfulServer().handleRequest(RequestTypeEnum.POST, myRequest, myResponse) + ); + } + + @Test + public void testExceptionClosingOutputStream() throws IOException { + initRequestMocks(); + when(myRequest.getInputStream()).thenReturn(createMockPatientBodyServletInputStream()); + when(myResponse.getWriter()).thenReturn(myWriter); + + // Throw an exception when the stream is closed + doThrow(new EOFException()).when(myWriter).close(); + + assertDoesNotThrow(() -> + ourServer.getRestfulServer().handleRequest(RequestTypeEnum.POST, myRequest, myResponse) + ); + } + + private void initRequestMocks() { + myHeaders = new HashMap<>(); + myHeaders.put(Constants.HEADER_CONTENT_TYPE, Constants.CT_FHIR_JSON_NEW); + + when(myRequest.getRequestURI()).thenReturn("/Patient"); + when(myRequest.getRequestURL()).thenReturn(new StringBuffer(ourServer.getBaseUrl() + "/Patient")); + when(myRequest.getHeader(any())).thenAnswer(t -> { + String header = t.getArgument(0, String.class); + String value = myHeaders.get(header); + ourLog.info("Request for header '{}' produced: {}", header, value); + return value; + }); + when(myRequest.getHeaders(any())).thenAnswer(t -> { + String header = t.getArgument(0, String.class); + String value = myHeaders.get(header); + ourLog.info("Request for header '{}' produced: {}", header, value); + if (value != null) { + return new IteratorEnumeration<>(Collections.singleton(value).iterator()); + } + return new EmptyEnumeration<>(); + }); + } + + /** + * Based on the class from Spring Test with the same name + */ + public static class DelegatingServletInputStream extends ServletInputStream { + private final InputStream mySourceStream; + private boolean myFinished = false; + private boolean myExceptionOnClose = false; + + public DelegatingServletInputStream(InputStream sourceStream) { + Assert.notNull(sourceStream, "Source InputStream must not be null"); + this.mySourceStream = sourceStream; + } + + public void setExceptionOnClose(boolean theExceptionOnClose) { + myExceptionOnClose = theExceptionOnClose; + } + + @Override + public int read() throws IOException { + int data = this.mySourceStream.read(); + if (data == -1) { + this.myFinished = true; + } + + return data; + } + + @Override + public int available() throws IOException { + return this.mySourceStream.available(); + } + + @Override + public void close() throws IOException { + super.close(); + this.mySourceStream.close(); + if (myExceptionOnClose) { + throw new IOException("Failed!"); + } + } + + @Override + public boolean isFinished() { + return this.myFinished; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener readListener) { + throw new UnsupportedOperationException(); + } + } + + @SuppressWarnings("unused") + public static class MyPatientProvider implements IResourceProvider { + + @Create + public MethodOutcome create(@ResourceParam Patient thePatient) throws InterruptedException { + OperationOutcome oo = new OperationOutcome(); + oo.addIssue().setDiagnostics(RandomStringUtils.randomAlphanumeric(1000)); + + return new MethodOutcome() + .setId(new IdType("Patient/A")) + .setOperationOutcome(oo); + } + + + @Override + public Class getResourceType() { + return Patient.class; + } + } + + @Nonnull + public static DelegatingServletInputStream createMockPatientBodyServletInputStream() { + Patient input = new Patient(); + input.addName().setFamily(RandomStringUtils.randomAlphanumeric(100000)); + String patient = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(input); + ByteArrayInputStream bais = new ByteArrayInputStream(patient.getBytes(StandardCharsets.UTF_8)); + return new DelegatingServletInputStream(bais); + } +} diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java index e90f14c3769..e81f5a357da 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java @@ -2,11 +2,14 @@ package ca.uhn.fhir.rest.server.interceptor; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.interceptor.api.Hook; +import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.annotation.Operation; import ca.uhn.fhir.rest.annotation.OperationParam; import ca.uhn.fhir.rest.annotation.RequiredParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.param.StringParam; @@ -19,8 +22,11 @@ import ca.uhn.fhir.rest.server.interceptor.consent.IConsentService; import ca.uhn.fhir.rest.server.provider.HashMapResourceProvider; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.test.utilities.HttpClientExtension; +import ca.uhn.fhir.test.utilities.LoggingExtension; import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; import com.google.common.base.Charsets; +import com.helger.commons.collection.iterate.EmptyEnumeration; +import org.apache.commons.collections4.iterators.IteratorEnumeration; import org.apache.commons.io.IOUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; @@ -41,16 +47,28 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.util.Assert; +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.EOFException; import java.io.IOException; +import java.io.InputStream; +import java.io.PrintWriter; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; @@ -65,13 +83,15 @@ public class ConsentInterceptorTest { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ConsentInterceptorTest.class); @RegisterExtension private final HttpClientExtension myClient = new HttpClientExtension(); + @RegisterExtension + private final LoggingExtension myLoggingExtension = new LoggingExtension(); private static final FhirContext ourCtx = FhirContext.forR4Cached(); private int myPort; private static final DummyPatientResourceProvider ourPatientProvider = new DummyPatientResourceProvider(ourCtx); private static final DummySystemProvider ourSystemProvider = new DummySystemProvider(); @RegisterExtension - private static RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) + private static final RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) .registerProvider(ourPatientProvider) .registerProvider(ourSystemProvider) .withPagingProvider(new FifoMemoryPagingProvider(10)); @@ -129,8 +149,8 @@ public class ConsentInterceptorTest { ourLog.info("Response: {}", responseContent); } - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); } @Test @@ -243,10 +263,10 @@ public class ConsentInterceptorTest { ourLog.info("Response: {}", responseContent); } - verify(myConsentSvc, times(0)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(0)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(0)).startOperation(any(), any()); - verify(myConsentSvc, times(2)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).completeOperationSuccess(any(), any()); } @@ -268,12 +288,12 @@ public class ConsentInterceptorTest { assertThat(responseContent, containsString("PTA")); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(0)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(3)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(3)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -296,12 +316,12 @@ public class ConsentInterceptorTest { assertThat(responseContent, containsString("PTA")); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(2)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(3)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(3)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -329,7 +349,7 @@ public class ConsentInterceptorTest { } verify(myConsentSvc, timeout(10000).times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); verify(myConsentSvc, timeout(10000).times(2)).canSeeResource(any(), any(), any()); verify(myConsentSvc, timeout(10000).times(3)).willSeeResource(any(), any(), any()); verify(myConsentSvc, timeout(10000).times(1)).completeOperationSuccess(any(), any()); @@ -357,12 +377,12 @@ public class ConsentInterceptorTest { assertNull(status.getFirstHeader(Constants.HEADER_CONTENT_TYPE)); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(2)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(3)).willSeeResource(any(), any(), any()); // the two patients + the bundle - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(3)).willSeeResource(any(), any(), any()); // the two patients + the bundle + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -398,11 +418,11 @@ public class ConsentInterceptorTest { } verify(myConsentSvc, timeout(1000).times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); verify(myConsentSvc, timeout(1000).times(2)).canSeeResource(any(), any(), any()); verify(myConsentSvc, timeout(1000).times(3)).willSeeResource(any(), any(), any()); verify(myConsentSvc, timeout(1000).times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -438,12 +458,12 @@ public class ConsentInterceptorTest { assertEquals("PTB", response.getEntry().get(1).getResource().getIdElement().getIdPart()); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(2)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(4)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(4)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -476,12 +496,12 @@ public class ConsentInterceptorTest { assertEquals("PTB", response.getEntry().get(1).getResource().getIdElement().getIdPart()); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(2)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(3)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(3)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -529,7 +549,7 @@ public class ConsentInterceptorTest { assertEquals("REPLACEMENT", ((Patient)response.getEntry().get(0).getResource()).getIdentifierFirstRep().getSystem()); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); } @@ -554,18 +574,18 @@ public class ConsentInterceptorTest { assertNull(response.getTotalElement().getValue()); assertEquals(0, response.getEntry().size()); - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc2, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc2, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(0)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc2, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc2, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); - verify(myConsentSvc2, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc2, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); verifyNoMoreInteractions(myConsentSvc2); } @@ -602,18 +622,18 @@ public class ConsentInterceptorTest { assertThat(responseContent, not(containsString("\"entry\""))); } - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc2, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc2, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(0)).canSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(2)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(3)).willSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(2)).willSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc2, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); - verify(myConsentSvc2, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(2)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(3)).willSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(2)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); } @@ -638,18 +658,18 @@ public class ConsentInterceptorTest { assertNull(response.getTotalElement().getValue()); assertEquals(1, response.getEntry().size()); - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc2, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc2, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(0)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc2, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc2, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); - verify(myConsentSvc2, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc2, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); verifyNoMoreInteractions(myConsentSvc2); } @@ -676,18 +696,18 @@ public class ConsentInterceptorTest { assertNull(response.getTotalElement().getValue()); assertEquals(0, response.getEntry().size()); - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc2, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc2, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc2, times(1)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc2, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); - verify(myConsentSvc2, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc2, timeout(2000).times(1)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); verifyNoMoreInteractions(myConsentSvc2); } @@ -719,22 +739,111 @@ public class ConsentInterceptorTest { Patient patient = (Patient) response.getEntry().get(0).getResource(); assertEquals(2, patient.getIdentifier().size()); - verify(myConsentSvc, times(1)).startOperation(any(), any()); - verify(myConsentSvc2, times(1)).startOperation(any(), any()); - verify(myConsentSvc, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc2, times(1)).shouldProcessCanSeeResource(any(), any()); - verify(myConsentSvc, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc2, times(1)).canSeeResource(any(), any(), any()); - verify(myConsentSvc, times(2)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc2, times(2)).willSeeResource(any(), any(), any()); // On bundle - verify(myConsentSvc, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc2, times(1)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(0)).completeOperationFailure(any(), any(), any()); - verify(myConsentSvc2, times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).startOperation(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).shouldProcessCanSeeResource(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, timeout(2000).times(2)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc2, timeout(2000).times(2)).willSeeResource(any(), any(), any()); // On bundle + verify(myConsentSvc, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc2, timeout(2000).times(1)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); + verify(myConsentSvc2, timeout(2000).times(0)).completeOperationFailure(any(), any(), any()); verifyNoMoreInteractions(myConsentSvc); verifyNoMoreInteractions(myConsentSvc2); } + @Mock + private HttpServletRequest myRequest; + @Mock + private HttpServletResponse myResponse; + @Mock + private PrintWriter myWriter; + private HashMap myHeaders; + + private void initRequestMocks() { + myHeaders = new HashMap<>(); + myHeaders.put(Constants.HEADER_CONTENT_TYPE, Constants.CT_FHIR_JSON_NEW); + + when(myRequest.getRequestURI()).thenReturn("/Patient"); + when(myRequest.getRequestURL()).thenReturn(new StringBuffer(ourServer.getBaseUrl() + "/Patient")); + when(myRequest.getHeader(any())).thenAnswer(t -> { + String header = t.getArgument(0, String.class); + String value = myHeaders.get(header); + ourLog.info("Request for header '{}' produced: {}", header, value); + return value; + }); + when(myRequest.getHeaders(any())).thenAnswer(t -> { + String header = t.getArgument(0, String.class); + String value = myHeaders.get(header); + ourLog.info("Request for header '{}' produced: {}", header, value); + if (value != null) { + return new IteratorEnumeration<>(Collections.singleton(value).iterator()); + } + return new EmptyEnumeration<>(); + }); + } + + /** + * Based on the class from Spring Test with the same name + */ + public static class DelegatingServletInputStream extends ServletInputStream { + private final InputStream mySourceStream; + private boolean myFinished = false; + + public void setExceptionOnClose(boolean theExceptionOnClose) { + myExceptionOnClose = theExceptionOnClose; + } + + private boolean myExceptionOnClose = false; + + public DelegatingServletInputStream(InputStream sourceStream) { + Assert.notNull(sourceStream, "Source InputStream must not be null"); + this.mySourceStream = sourceStream; + } + + @Override + public int read() throws IOException { + int data = this.mySourceStream.read(); + if (data == -1) { + this.myFinished = true; + } + + return data; + } + + @Override + public int available() throws IOException { + return this.mySourceStream.available(); + } + + @Override + public void close() throws IOException { + super.close(); + this.mySourceStream.close(); + if (myExceptionOnClose) { + throw new IOException("Failed!"); + } + } + + @Override + public boolean isFinished() { + return this.myFinished; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener readListener) { + throw new UnsupportedOperationException(); + } + } + @Test public void testOutcomeException() throws IOException { when(myConsentSvc.startOperation(any(), any())).thenReturn(ConsentOutcome.PROCEED); @@ -747,8 +856,8 @@ public class ConsentInterceptorTest { ourLog.info("Response: {}", responseContent); } - verify(myConsentSvc, times(0)).completeOperationSuccess(any(), any()); - verify(myConsentSvc, times(1)).completeOperationFailure(any(), myExceptionCaptor.capture(), any()); + verify(myConsentSvc, timeout(2000).times(0)).completeOperationSuccess(any(), any()); + verify(myConsentSvc, timeout(2000).times(1)).completeOperationFailure(any(), myExceptionCaptor.capture(), any()); assertEquals(Msg.code(389) + "Failed to call access method: java.lang.NullPointerException: A MESSAGE", myExceptionCaptor.getValue().getMessage()); } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptorTest.java index 09567bee5ad..6348e919ef1 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ExceptionHandlingInterceptorTest.java @@ -15,7 +15,9 @@ import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.test.utilities.HttpClientExtension; import ca.uhn.fhir.test.utilities.JettyUtil; +import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; import ca.uhn.fhir.util.TestUtil; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; @@ -35,6 +37,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.opentest4j.AssertionFailedError; import java.io.IOException; @@ -46,36 +49,42 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class ExceptionHandlingInterceptorTest { - private static ExceptionHandlingInterceptor myInterceptor; - private static final String OPERATION_OUTCOME_DETAILS = "OperationOutcomeDetails"; - private static CloseableHttpClient ourClient; private static FhirContext ourCtx = FhirContext.forR4(); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionHandlingInterceptorTest.class); + + @RegisterExtension + private static final RestfulServerExtension ourServer = new RestfulServerExtension(ourCtx) + .withDefaultResponseEncoding(EncodingEnum.XML) + .registerProvider(new DummyPatientResourceProvider()); + @RegisterExtension + private static final HttpClientExtension ourClient = new HttpClientExtension(); + + private ExceptionHandlingInterceptor myInterceptor; + private static final String OPERATION_OUTCOME_DETAILS = "OperationOutcomeDetails"; private static Class ourExceptionType; private static boolean ourGenerateOperationOutcome; - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionHandlingInterceptorTest.class); - private static int ourPort; - private static Server ourServer; - private static RestfulServer servlet; - + @BeforeEach - public void before() { + public void beforeEach() { ourGenerateOperationOutcome = false; ourExceptionType=null; - + + myInterceptor = new ExceptionHandlingInterceptor(); myInterceptor.setReturnStackTracesForExceptionTypes(Throwable.class); + ourServer.registerInterceptor(myInterceptor); } @Test public void testInternalError() throws Exception { myInterceptor.setReturnStackTracesForExceptionTypes(Throwable.class); { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?throwInternalError=aaa"); + HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Patient?throwInternalError=aaa"); HttpResponse status = ourClient.execute(httpGet); String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8); IOUtils.closeQuietly(status.getEntity().getContent()); ourLog.info(responseContent); assertEquals(500, status.getStatusLine().getStatusCode()); - OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent); + OperationOutcome oo = (OperationOutcome) ourCtx.newXmlParser().parseResource(responseContent); assertThat(oo.getIssueFirstRep().getDiagnosticsElement().getValue(), StringContains.containsString("Exception Text")); assertThat(oo.getIssueFirstRep().getDiagnosticsElement().getValue(), (StringContains.containsString("InternalErrorException: Exception Text"))); } @@ -86,36 +95,34 @@ public class ExceptionHandlingInterceptorTest { //Given: We have an interceptor which causes a failure after the response output stream has been started. ProblemGeneratingInterceptor interceptor = new ProblemGeneratingInterceptor(); - servlet.registerInterceptor(interceptor); - myInterceptor.setReturnStackTracesForExceptionTypes(Throwable.class); + ourServer.registerInterceptor(interceptor); //When: We make a request to the server, triggering this exception to be thrown on an otherwise successful request - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?succeed=true"); + HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Patient?succeed=true"); httpGet.setHeader("Accept-encoding", "gzip"); HttpResponse status = ourClient.execute(httpGet); - servlet.unregisterInterceptor(interceptor); + ourServer.unregisterInterceptor(interceptor); //Then: This should still return an OperationOutcome, and not explode with an HTML IllegalState response. String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8); IOUtils.closeQuietly(status.getEntity().getContent()); ourLog.info(responseContent); assertEquals(500, status.getStatusLine().getStatusCode()); - OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent); - ourLog.info(servlet.getFhirContext().newXmlParser().encodeResourceToString(oo)); + OperationOutcome oo = (OperationOutcome) ourCtx.newXmlParser().parseResource(responseContent); + ourLog.info(ourCtx.newXmlParser().encodeResourceToString(oo)); assertThat(oo.getIssueFirstRep().getDiagnosticsElement().getValue(), StringContains.containsString("Simulated IOException")); } @Test public void testInternalErrorFormatted() throws Exception { - myInterceptor.setReturnStackTracesForExceptionTypes(Throwable.class); { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?throwInternalError=aaa&_format=true"); + HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Patient?throwInternalError=aaa&_format=true"); HttpResponse status = ourClient.execute(httpGet); String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8); IOUtils.closeQuietly(status.getEntity().getContent()); ourLog.info(responseContent); assertEquals(500, status.getStatusLine().getStatusCode()); - OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent); + OperationOutcome oo = (OperationOutcome) ourCtx.newXmlParser().parseResource(responseContent); assertThat(oo.getIssueFirstRep().getDiagnosticsElement().getValue(), StringContains.containsString("Exception Text")); assertThat(oo.getIssueFirstRep().getDiagnosticsElement().getValue(), (StringContains.containsString("InternalErrorException: Exception Text"))); } @@ -125,33 +132,8 @@ public class ExceptionHandlingInterceptorTest { @AfterAll public static void afterClassClearContext() throws Exception { - JettyUtil.closeServer(ourServer); TestUtil.randomizeLocaleAndTimezone(); } - @BeforeAll - public static void beforeClass() throws Exception { - ourServer = new Server(0); - - DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider(); - - ServletHandler proxyHandler = new ServletHandler(); - servlet = new RestfulServer(ourCtx); - servlet.setResourceProviders(patientProvider); - servlet.setDefaultResponseEncoding(EncodingEnum.XML); - ServletHolder servletHolder = new ServletHolder(servlet); - proxyHandler.addServletWithMapping(servletHolder, "/*"); - ourServer.setHandler(proxyHandler); - JettyUtil.startServer(ourServer); - ourPort = JettyUtil.getPortForStartedServer(ourServer); - - PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); - HttpClientBuilder builder = HttpClientBuilder.create(); - builder.setConnectionManager(connectionManager); - ourClient = builder.build(); - - myInterceptor = new ExceptionHandlingInterceptor(); - servlet.registerInterceptor(myInterceptor); - } public static class ProblemGeneratingInterceptor { @Hook(Pointcut.SERVER_OUTGOING_WRITER_CREATED) diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerExtension.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerExtension.java index 5fed45659e8..5cf65bcd262 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerExtension.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerExtension.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; import ca.uhn.fhir.rest.server.IPagingProvider; @@ -45,12 +46,12 @@ public class RestfulServerExtension extends BaseJettyServerExtension myProviders = new ArrayList<>(); private FhirVersionEnum myFhirVersion; - private IGenericClient myFhirClient; private RestfulServer myServlet; private List> myConsumers = new ArrayList<>(); private Map myRunningServerUserData = new HashMap<>(); private ServerValidationModeEnum myServerValidationMode = ServerValidationModeEnum.NEVER; private IPagingProvider myPagingProvider; + /** * Constructor */ @@ -83,7 +84,6 @@ public class RestfulServerExtension extends BaseJettyServerExtension t.getInterceptorService().registerAnonymousInterceptor(thePointcut, theInterceptor)); } + + public RestfulServerExtension withDefaultResponseEncoding(EncodingEnum theEncodingEnum) { + return withServer(t -> t.setDefaultResponseEncoding(theEncodingEnum)); + } }