From 6c85cd1375ec77a5348a87951b07910533cdbc7b Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 12 Dec 2017 21:45:54 -0500 Subject: [PATCH] Improve documentation and mark redundant methods as deprecated in IServerInterceptor --- .../interceptor/IServerInterceptor.java | 120 ++++------------- .../rest/server/InterceptorDstu3Test.java | 126 +++++++++++++----- src/changes/changes.xml | 7 + 3 files changed, 127 insertions(+), 126 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java index 8b35f6413cb..3471d3e587b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java @@ -92,10 +92,7 @@ public interface IServerInterceptor { /** * This method is called just before the actual implementing server method is invoked. - *

- * Note about exceptions: - *

- * + * * @param theRequestDetails * A bean containing details about the request that is about to be processed, including details such as the * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been @@ -157,77 +154,40 @@ public interface IServerInterceptor { boolean incomingRequestPreProcessed(HttpServletRequest theRequest, HttpServletResponse theResponse); /** - * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client - * - * @param theRequestDetails - * A bean containing details about the request that is about to be processed, including details such as the - * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been - * pulled out of the {@link HttpServletRequest servlet request}. - * @return Return true if processing should continue normally. This is generally the right thing to do. - * If your interceptor is providing a response rather than letting HAPI handle the response normally, you - * must return false. In this case, no further processing will occur and no further interceptors - * will be called. - * @throws AuthenticationException - * This exception may be thrown to indicate that the interceptor has detected an unauthorized access - * attempt. If thrown, processing will stop and an HTTP 401 will be returned to the client. + * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead + * + * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR. */ + @Deprecated boolean outgoingResponse(RequestDetails theRequestDetails); /** - * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client - * - * @param theRequestDetails - * A bean containing details about the request that is about to be processed, including details such as the - * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been - * pulled out of the {@link HttpServletRequest servlet request}. - * @param theServletRequest - * The incoming request - * @param theServletResponse - * The response. Note that interceptors may choose to provide a response (i.e. by calling - * {@link HttpServletResponse#getWriter()}) but in that case it is important to return false - * to indicate that the server itself should not also provide a response. - * @return Return true if processing should continue normally. This is generally the right thing to do. - * If your interceptor is providing a response rather than letting HAPI handle the response normally, you - * must return false. In this case, no further processing will occur and no further interceptors - * will be called. - * @throws AuthenticationException - * This exception may be thrown to indicate that the interceptor has detected an unauthorized access - * attempt. If thrown, processing will stop and an HTTP 401 will be returned to the client. + * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead + * + * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR. */ + @Deprecated boolean outgoingResponse(RequestDetails theRequestDetails, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse) throws AuthenticationException; /** - * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client - * - * @param theRequestDetails - * A bean containing details about the request that is about to be processed, including details such as the - * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been - * pulled out of the {@link HttpServletRequest servlet request}. - * @param theResponseObject - * The actual object which is being streamed to the client as a response - * @return Return true if processing should continue normally. This is generally the right thing to do. - * If your interceptor is providing a response rather than letting HAPI handle the response normally, you - * must return false. In this case, no further processing will occur and no further interceptors - * will be called. - * @throws AuthenticationException - * This exception may be thrown to indicate that the interceptor has detected an unauthorized access - * attempt. If thrown, processing will stop and an HTTP 401 will be returned to the client. + * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead + * + * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR. */ + @Deprecated boolean outgoingResponse(RequestDetails theRequestDetails, IBaseResource theResponseObject); /** * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client + * response back to the client. * * @param theRequestDetails * A bean containing details about the request that is about to be processed, including details such as the * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been * pulled out of the {@link HttpServletRequest servlet request}. * @param theResponseObject - * The actual object which is being streamed to the client as a response + * The actual object which is being streamed to the client as a response. This may be + * null if the response does not include a resource. * @param theServletRequest * The incoming request * @param theServletResponse @@ -246,49 +206,19 @@ public interface IServerInterceptor { throws AuthenticationException; /** - * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client - * - * @param theRequestDetails - * A bean containing details about the request that is about to be processed, including details such as the - * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been - * pulled out of the {@link HttpServletRequest servlet request}. - * @param theResponseObject - * The actual object which is being streamed to the client as a response - * @return Return true if processing should continue normally. This is generally the right thing to do. - * If your interceptor is providing a response rather than letting HAPI handle the response normally, you - * must return false. In this case, no further processing will occur and no further interceptors - * will be called. - * @throws AuthenticationException - * This exception may be thrown to indicate that the interceptor has detected an unauthorized access - * attempt. If thrown, processing will stop and an HTTP 401 will be returned to the client. + * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead + * + * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR. */ + @Deprecated boolean outgoingResponse(RequestDetails theRequestDetails, TagList theResponseObject); /** - * This method is called after the server implementation method has been called, but before any attempt to stream the - * response back to the client - * - * @param theRequestDetails - * A bean containing details about the request that is about to be processed, including details such as the - * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been - * pulled out of the {@link HttpServletRequest servlet request}. - * @param theResponseObject - * The actual object which is being streamed to the client as a response - * @param theServletRequest - * The incoming request - * @param theServletResponse - * The response. Note that interceptors may choose to provide a response (i.e. by calling - * {@link HttpServletResponse#getWriter()}) but in that case it is important to return false - * to indicate that the server itself should not also provide a response. - * @return Return true if processing should continue normally. This is generally the right thing to do. - * If your interceptor is providing a response rather than letting HAPI handle the response normally, you - * must return false. In this case, no further processing will occur and no further interceptors - * will be called. - * @throws AuthenticationException - * This exception may be thrown to indicate that the interceptor has detected an unauthorized access - * attempt. If thrown, processing will stop and an HTTP 401 will be returned to the client. + * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead + * + * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR. */ + @Deprecated boolean outgoingResponse(RequestDetails theRequestDetails, TagList theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse) throws AuthenticationException; /** @@ -328,7 +258,7 @@ public interface IServerInterceptor { */ void processingCompletedNormally(ServletRequestDetails theRequestDetails); - public static class ActionRequestDetails { + class ActionRequestDetails { private final FhirContext myContext; private final IIdType myId; private final String myResourceType; diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/InterceptorDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/InterceptorDstu3Test.java index 497ced4f3e9..11df89bfa80 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/InterceptorDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/InterceptorDstu3Test.java @@ -1,13 +1,12 @@ package ca.uhn.fhir.rest.server; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import java.util.ArrayList; import java.util.concurrent.TimeUnit; @@ -15,6 +14,7 @@ import java.util.concurrent.TimeUnit; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import ca.uhn.fhir.rest.annotation.Create; import org.apache.commons.io.IOUtils; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpPost; @@ -26,7 +26,9 @@ 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.dstu3.model.OperationOutcome; import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.junit.*; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; @@ -56,7 +58,7 @@ public class InterceptorDstu3Test { @After public void after() { - for (IServerInterceptor next : new ArrayList(ourServlet.getInterceptors())) { + for (IServerInterceptor next : new ArrayList<>(ourServlet.getInterceptors())) { ourServlet.unregisterInterceptor(next); } } @@ -65,7 +67,6 @@ public class InterceptorDstu3Test { public void before() { myInterceptor1 = mock(IServerInterceptor.class); myInterceptor2 = mock(IServerInterceptor.class); - ourServlet.setInterceptors(myInterceptor1, myInterceptor2); } @SuppressWarnings("deprecation") @@ -79,7 +80,72 @@ public class InterceptorDstu3Test { } @Test - public void testValidate() throws Exception { + public void testResponseWithOperationOutcome() throws Exception { + ourServlet.setInterceptors(myInterceptor1); + + when(myInterceptor1.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); + when(myInterceptor1.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); + when(myInterceptor1.outgoingResponse(any(RequestDetails.class), any(IResource.class))).thenReturn(true); + + String input = createInput(); + + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/$validate"); + httpPost.setEntity(new StringEntity(input, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8"))); + HttpResponse status = ourClient.execute(httpPost); + IOUtils.closeQuietly(status.getEntity().getContent()); + + InOrder order = inOrder(myInterceptor1); + order.verify(myInterceptor1, times(1)).incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class)); + order.verify(myInterceptor1, times(1)).incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class)); + ArgumentCaptor opTypeCapt = ArgumentCaptor.forClass(RestOperationTypeEnum.class); + ArgumentCaptor arTypeCapt = ArgumentCaptor.forClass(ActionRequestDetails.class); + ArgumentCaptor resourceCapt = ArgumentCaptor.forClass(IBaseResource.class); + order.verify(myInterceptor1, times(1)).incomingRequestPreHandled(opTypeCapt.capture(), arTypeCapt.capture()); + order.verify(myInterceptor1, times(1)).outgoingResponse(any(RequestDetails.class), resourceCapt.capture()); + + assertEquals(1, resourceCapt.getAllValues().size()); + assertEquals(OperationOutcome.class, resourceCapt.getAllValues().get(0).getClass()); + } + + @Test + public void testResponseWithNothing() throws Exception { + ourServlet.setInterceptors(myInterceptor1); + + when(myInterceptor1.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); + when(myInterceptor1.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); + when(myInterceptor1.outgoingResponse(any(RequestDetails.class), any(IResource.class))).thenReturn(true); + + String input = createInput(); + + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient"); + httpPost.setEntity(new StringEntity(input, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8"))); + HttpResponse status = ourClient.execute(httpPost); + try { + assertEquals(201, status.getStatusLine().getStatusCode()); + } finally { + IOUtils.closeQuietly(status.getEntity().getContent()); + } + + InOrder order = inOrder(myInterceptor1); + verify(myInterceptor1, times(1)).incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class)); + verify(myInterceptor1, times(1)).incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class)); + ArgumentCaptor opTypeCapt = ArgumentCaptor.forClass(RestOperationTypeEnum.class); + ArgumentCaptor arTypeCapt = ArgumentCaptor.forClass(ActionRequestDetails.class); + ArgumentCaptor rdCapt = ArgumentCaptor.forClass(RequestDetails.class); + ArgumentCaptor resourceCapt = ArgumentCaptor.forClass(IBaseResource.class); + verify(myInterceptor1, times(1)).incomingRequestPreHandled(opTypeCapt.capture(), arTypeCapt.capture()); + verify(myInterceptor1, times(1)).outgoingResponse(any(RequestDetails.class), resourceCapt.capture()); + + assertEquals(1, resourceCapt.getAllValues().size()); + assertEquals(null, resourceCapt.getAllValues().get(0)); +// assertEquals("", rdCapt.getAllValues().get(0).get) + } + + + @Test + public void testResourceResponseIncluded() throws Exception { + ourServlet.setInterceptors(myInterceptor1, myInterceptor2); + when(myInterceptor1.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); when(myInterceptor1.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); when(myInterceptor1.outgoingResponse(any(RequestDetails.class), any(IResource.class))).thenReturn(true); @@ -87,27 +153,7 @@ public class InterceptorDstu3Test { when(myInterceptor2.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true); when(myInterceptor2.outgoingResponse(any(RequestDetails.class), any(IResource.class))).thenReturn(true); - //@formatter:off - String input = - "{\n" + - " \"resourceType\":\"Observation\",\n" + - " \"id\":\"1855669\",\n" + - " \"meta\":{\n" + - " \"versionId\":\"1\",\n" + - " \"lastUpdated\":\"2016-02-18T07:41:35.953-05:00\"\n" + - " },\n" + - " \"status\":\"final\",\n" + - " \"subject\":{\n" + - " \"reference\":\"Patient/1\"\n" + - " },\n" + - " \"effectiveDateTime\":\"2016-02-18T07:45:36-05:00\",\n" + - " \"valueQuantity\":{\n" + - " \"value\":57,\n" + - " \"system\":\"http://unitsofmeasure.org\",\n" + - " \"code\":\"{Beats}/min\"\n" + - " }\n" + - "}"; - //@formatter:on + String input = createInput(); HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/$validate"); httpPost.setEntity(new StringEntity(input, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8"))); @@ -123,8 +169,8 @@ public class InterceptorDstu3Test { ArgumentCaptor arTypeCapt = ArgumentCaptor.forClass(ActionRequestDetails.class); order.verify(myInterceptor1, times(1)).incomingRequestPreHandled(opTypeCapt.capture(), arTypeCapt.capture()); order.verify(myInterceptor2, times(1)).incomingRequestPreHandled(any(RestOperationTypeEnum.class), any(ActionRequestDetails.class)); - order.verify(myInterceptor2, times(1)).outgoingResponse(any(RequestDetails.class), any(IResource.class)); - order.verify(myInterceptor1, times(1)).outgoingResponse(any(RequestDetails.class), any(IResource.class)); + order.verify(myInterceptor2, times(1)).outgoingResponse(any(RequestDetails.class), any(IBaseResource.class)); + order.verify(myInterceptor1, times(1)).outgoingResponse(any(RequestDetails.class), any(IBaseResource.class)); // Avoid concurrency issues Thread.sleep(500); @@ -138,6 +184,18 @@ public class InterceptorDstu3Test { assertNotNull(arTypeCapt.getValue().getResource()); } + private String createInput() { + return "{\n" + + " \"resourceType\":\"Patient\",\n" + + " \"id\":\"1855669\",\n" + + " \"meta\":{\n" + + " \"versionId\":\"1\",\n" + + " \"lastUpdated\":\"2016-02-18T07:41:35.953-05:00\"\n" + + " },\n" + + " \"active\":true\n" + + "}"; + } + @AfterClass public static void afterClassClearContext() throws Exception { ourServer.stop(); @@ -177,6 +235,12 @@ public class InterceptorDstu3Test { public MethodOutcome validate(@ResourceParam Patient theResource) { return new MethodOutcome(); } + + @Create() + public MethodOutcome create(@ResourceParam Patient theResource) { + return new MethodOutcome(); + } + } } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index acd383703e0..86da618cead 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -33,6 +33,13 @@ A few log entries emitted by the JPA server suring every search have been reduced from INFO to DEBUG in order to reduce log noise + + A few redundant and no longer useful methods have been marked as + deprecated in + IServerInterceptor]]>. If you have implemented + custom interceptors you are recommended to migrate to the recommended + methods. +