Improve documentation and mark redundant methods as deprecated in

IServerInterceptor

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java

@@ -92,9 +92,6 @@ public interface IServerInterceptor {
 
 	/**
 	 * This method is called just before the actual implementing server method is invoked.
-	 * <p>
-	 * Note about exceptions:
-	 * </p>
 	 *
 	 * @param theRequestDetails
 	 *           A bean containing details about the request that is about to be processed, including details such as the
@@ -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
+	 * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead
-	 * response back to the client
 	 *
-	 * @param theRequestDetails
+	 * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR.
-	 *           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 <code>true</code> 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 <code>false</code>. 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.
 	 */
+	@Deprecated
 	boolean outgoingResponse(RequestDetails theRequestDetails);
 
 	/**
-	 * This method is called after the server implementation method has been called, but before any attempt to stream the
+	 * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead
-	 * response back to the client
 	 *
-	 * @param theRequestDetails
+	 * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR.
-	 *           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 <code>false</code>
-	 *           to indicate that the server itself should not also provide a response.
-	 * @return Return <code>true</code> 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 <code>false</code>. 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.
 	 */
+	@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
+	 * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead
-	 * response back to the client
 	 *
-	 * @param theRequestDetails
+	 * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR.
-	 *           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 <code>true</code> 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 <code>false</code>. 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.
 	 */
+	@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
+	 *           <code>null</code> 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
+	 * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead
-	 * response back to the client
 	 *
-	 * @param theRequestDetails
+	 * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR.
-	 *           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 <code>true</code> 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 <code>false</code>. 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.
 	 */
+	@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
+	 * Use {@link #outgoingResponse(RequestDetails, IBaseResource, HttpServletRequest, HttpServletResponse)} instead
-	 * response back to the client
 	 *
-	 * @param theRequestDetails
+	 * @deprecated As of HAPI FHIR 3.2.0, this method is deprecated and will be removed in a future version of HAPI FHIR.
-	 *           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 <code>false</code>
-	 *           to indicate that the server itself should not also provide a response.
-	 * @return Return <code>true</code> 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 <code>false</code>. 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.
 	 */
+	@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;

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.*;
-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 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<IServerInterceptor>(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<RestOperationTypeEnum> opTypeCapt = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		ArgumentCaptor<ActionRequestDetails> arTypeCapt = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		ArgumentCaptor<IBaseResource> 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<RestOperationTypeEnum> opTypeCapt = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		ArgumentCaptor<ActionRequestDetails> arTypeCapt = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		ArgumentCaptor<RequestDetails> rdCapt = ArgumentCaptor.forClass(RequestDetails.class);
+		ArgumentCaptor<IBaseResource> 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 = createInput();
-		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
 
 		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<ActionRequestDetails> 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(myInterceptor2, times(1)).outgoingResponse(any(RequestDetails.class), any(IBaseResource.class));
-		order.verify(myInterceptor1, times(1)).outgoingResponse(any(RequestDetails.class), any(IResource.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();
+		}
+
 	}
 
 }

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
 			</action>
+			<action type="remove">
+				A few redundant and no longer useful methods have been marked as
+				deprecated in
+				<![CDATA[<code>IServerInterceptor</code>]]>. If you have implemented
+				custom interceptors you are recommended to migrate to the recommended
+				methods.
+			</action>
 		</release>
 		<release version="3.1.0" date="2017-11-23">
 			<action type="add">