Work on #532

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -651,8 +651,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao {
 			}
 		}
 
-		requestDetails.notifyIncomingRequestPreHandled(theOperationType);
-
 		List<IServerInterceptor> interceptors = getConfig().getInterceptors();
 		if (interceptors == null) {
 			return;

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderInterceptorDstu3Test.java

@@ -0,0 +1,171 @@
+package ca.uhn.fhir.jpa.provider.dstu3;
+
+import static org.hamcrest.Matchers.startsWith;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.hl7.fhir.dstu3.model.Bundle;
+import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
+import org.hl7.fhir.dstu3.model.Bundle.BundleType;
+import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb;
+import org.hl7.fhir.dstu3.model.Patient;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
+import ca.uhn.fhir.rest.method.RequestDetails;
+import ca.uhn.fhir.rest.server.Constants;
+import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
+import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
+import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter;
+import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails;
+import ca.uhn.fhir.util.TestUtil;
+
+public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDstu3Test {
+
+	private IServerInterceptor myServerInterceptor;
+	private IServerInterceptor myDaoInterceptor;
+
+	@Override
+	@After
+	public void after() throws Exception {
+		super.after();
+
+		myDaoConfig.getInterceptors().remove(myDaoInterceptor);
+		ourRestServer.unregisterInterceptor(myServerInterceptor);
+	}
+
+	@Override
+	public void before() throws Exception {
+		super.before();
+		
+		myServerInterceptor = mock(IServerInterceptor.class);
+		myDaoInterceptor = mock(IServerInterceptor.class);
+
+		when(myServerInterceptor.handleException(any(RequestDetails.class), any(BaseServerResponseException.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class))).thenReturn(true);
+		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		
+		myDaoConfig.getInterceptors().add(myDaoInterceptor);
+		ourRestServer.registerInterceptor(myServerInterceptor);
+		
+//		ourRestServer.registerInterceptor(new InterceptorAdapter() {
+//			@Override
+//			public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theProcessedRequest) {
+//				super.incomingRequestPreHandled(theOperation, theProcessedRequest);
+//			}});
+		
+	}
+	
+	
+private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderInterceptorDstu3Test.class);
+
+	@Test
+	public void testCreateResource() throws IOException {
+		String methodName = "testCreateResource";
+
+		Patient pt = new Patient();
+		pt.addName().setFamily(methodName);
+		String resource = myFhirCtx.newXmlParser().encodeResourceToString(pt);
+
+		HttpPost post = new HttpPost(ourServerBase + "/Patient");
+		post.setEntity(new StringEntity(resource, ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
+		CloseableHttpResponse response = ourHttpClient.execute(post);
+		try {
+			String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info("Response was: {}", resp);
+			assertEquals(201, response.getStatusLine().getStatusCode());
+			String newIdString = response.getFirstHeader(Constants.HEADER_LOCATION_LC).getValue();
+			assertThat(newIdString, startsWith(ourServerBase + "/Patient/"));
+		} finally {
+			response.close();
+		}
+
+		ArgumentCaptor<ActionRequestDetails> ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		ArgumentCaptor<RestOperationTypeEnum> opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		verify(myServerInterceptor, times(1)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture());
+		assertEquals(RestOperationTypeEnum.CREATE, opTypeCaptor.getValue());
+		assertEquals("Patient", ardCaptor.getValue().getResourceType());
+		assertNotNull(ardCaptor.getValue().getResource());
+		
+		ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		verify(myDaoInterceptor, times(1)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture());
+		assertEquals(RestOperationTypeEnum.CREATE, opTypeCaptor.getValue());
+		assertEquals("Patient", ardCaptor.getValue().getResourceType());
+		assertNotNull(ardCaptor.getValue().getResource());
+	}
+
+
+	@Test
+	public void testCreateResourceInTransaction() throws IOException {
+		String methodName = "testCreateResourceInTransaction";
+
+		Patient pt = new Patient();
+		pt.addName().setFamily(methodName);
+		
+		Bundle bundle = new Bundle();
+		bundle.setType(BundleType.TRANSACTION);
+		BundleEntryComponent entry = bundle.addEntry();
+		entry.setFullUrl("Patient");
+		entry.setResource(pt);
+		entry.getRequest().setMethod(HTTPVerb.POST);
+		entry.getRequest().setUrl("Patient");
+		
+		String resource = myFhirCtx.newXmlParser().encodeResourceToString(bundle);
+
+		HttpPost post = new HttpPost(ourServerBase + "/");
+		post.setEntity(new StringEntity(resource, ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
+		CloseableHttpResponse response = ourHttpClient.execute(post);
+		try {
+			assertEquals(200, response.getStatusLine().getStatusCode());
+		} finally {
+			response.close();
+		}
+
+		ArgumentCaptor<ActionRequestDetails> ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		ArgumentCaptor<RestOperationTypeEnum> opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		verify(myServerInterceptor, times(1)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture());
+		assertEquals(RestOperationTypeEnum.TRANSACTION, opTypeCaptor.getValue());
+		assertNotNull(ardCaptor.getValue().getResource());
+		
+		ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		verify(myDaoInterceptor, times(2)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture());
+		assertEquals(RestOperationTypeEnum.TRANSACTION, opTypeCaptor.getAllValues().get(0));
+		assertEquals("Bundle", ardCaptor.getAllValues().get(0).getResourceType());
+		assertNotNull(ardCaptor.getAllValues().get(0).getResource());
+		assertEquals(RestOperationTypeEnum.CREATE, opTypeCaptor.getAllValues().get(1));
+		assertEquals("Patient", ardCaptor.getAllValues().get(1).getResourceType());
+		assertNotNull(ardCaptor.getAllValues().get(1).getResource());
+	}
+
+	
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

src/site/xdoc/doc_rest_server_interceptor.xml

@@ -342,11 +342,11 @@
 			
 			<p>
 				The HAPI <a href="./doc_jpa.html">JPA Server</a> is an added layer on top of the HAPI
-				REST server framework. If you are using it, you may wish to also register interceptors
+				REST server framework. If you are using it, you may wish register interceptors
 				against the <a href="./apidocs-jpaserver/ca/uhn/fhir/jpa/dao/DaoConfig.html">DaoConfig</a>
 				bean that you create using Spring configuration. 
 			</p>
-		
+
 			<p>
 				By registering an interceptor against the DaoConfig, the server will invoke 
 				interceptor methods for operations such as <b>create</b>, <b>update</b>, etc even
@@ -354,13 +354,34 @@
 				if you are using interceptors to make access control decisions because
 				it avoids clients using transactions as a means of bypassing these controls.
 			</p>
+
+			<p>
+				When an interceptor is registered against the DaoConfig instead of the RestfulServer, 
+				the <code>incomingRequestPreHandled</code> method will be called once for most
+				operations (e.g. a FHIR <code>create</code>), but in the case where the client
+				performs a FHIR <code>transaction</code> that method might be called multiple
+				times over the course of a single client invocation. For example if the transaction
+				contained a single create, the <code>incomingRequestPreHandled</code> method will
+				be called twice: once to indicate the transaction, and once to indicate the create. 
+			</p>
+			
+			<p>
+				This use of interceptors is typically only useful for situations where it's more important
+				to understand the semantics of the request, as opposed to the physical contents. In other 
+				words, if you are creating an auditing interceptor for a JPA server, it probably
+				makes sense to register the interceptor with the DaoConfig. If you are creating
+				an HTTP Request Logger for a JPA server, it probably makes sense to register it
+				with the RestfulServer. Note that it <b>generally doesn't make sense to register
+				the same interceptor against both</b> the DaoConfig and the RestfulServer since 
+				both places will end up invoking the same methods. 
+			</p>
 			
 			<p>
 				You may also choose to create interceptors which implement the 
 				more specialized
 				<a href="./apidocs-jpaserver/ca/uhn/fhir/jpa/dao/IJpaServerInterceptor.html">IJpaServerInterceptor</a>
 				interface, as this interceptor adds additional methods which are called during the JPA
-				lifecycle. 
+				lifecycle.
 			</p>
 		
 		</section>