Polish async request processing

This change fixes a cyclical package dependency.

The change also improves the implementation of
WebAsyncManager.hasConcurrentResult() following the resolution of
Apache issue id=53632 and the release of Apache Tomcat 7.0.30 that
contains the fix.

Author: rstoyanchev

spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewInterceptor.java

@@ -24,8 +24,8 @@
 import org.springframework.orm.hibernate3.SessionHolder;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.context.request.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.WebRequest;
-import org.springframework.web.context.request.async.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.async.AsyncWebUtils;
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncManager.WebAsyncThreadInitializer;

spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewInterceptor.java

@@ -28,8 +28,8 @@
 import org.springframework.orm.hibernate4.SessionHolder;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.context.request.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.WebRequest;
-import org.springframework.web.context.request.async.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.async.AsyncWebUtils;
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncManager.WebAsyncThreadInitializer;

spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewInterceptor.java

@@ -26,8 +26,8 @@
 import org.springframework.orm.jpa.EntityManagerHolder;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.context.request.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.WebRequest;
-import org.springframework.web.context.request.async.AsyncWebRequestInterceptor;
 import org.springframework.web.context.request.async.AsyncWebUtils;
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncManager.WebAsyncThreadInitializer;

spring-orm/src/test/java/org/springframework/orm/hibernate3/support/OpenSessionInViewTests.java

@@ -17,9 +17,9 @@
 package org.springframework.orm.hibernate3.support;
 
 import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.createStrictMock;
 import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.reset;
 import static org.easymock.EasyMock.verify;
@@ -196,6 +196,10 @@ public String call() throws Exception {
 
 		// Async dispatch thread
 
+		reset(asyncWebRequest);
+		expect(asyncWebRequest.isDispatched()).andReturn(true);
+		replay(asyncWebRequest);
+
 		interceptor.preHandle(this.webRequest);
 		assertTrue("Session not bound to async thread", TransactionSynchronizationManager.hasResource(sf));
 
@@ -488,11 +492,11 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
 			}
 		};
 
-		AsyncWebRequest asyncWebRequest = createStrictMock(AsyncWebRequest.class);
+		AsyncWebRequest asyncWebRequest = createMock(AsyncWebRequest.class);
 		asyncWebRequest.addCompletionHandler((Runnable) anyObject());
 		asyncWebRequest.startAsync();
-		expect(asyncWebRequest.isAsyncStarted()).andReturn(true);
-		expectLastCall().anyTimes();
+		expect(asyncWebRequest.isAsyncStarted()).andReturn(true).anyTimes();
+		expect(asyncWebRequest.isDispatched()).andReturn(false).anyTimes();
 		replay(asyncWebRequest);
 
 		WebAsyncManager asyncManager = AsyncWebUtils.getAsyncManager(this.request);
@@ -520,6 +524,7 @@ public String call() throws Exception {
 
 		expect(session.close()).andReturn(null);
 		expect(asyncWebRequest.isAsyncStarted()).andReturn(false).anyTimes();
+		expect(asyncWebRequest.isDispatched()).andReturn(true).anyTimes();
 
 		replay(sf);
 		replay(session);

spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java

@@ -24,8 +24,6 @@
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.reset;
 import static org.easymock.EasyMock.verify;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 
 import java.io.IOException;
 import java.util.concurrent.Callable;
@@ -161,22 +159,23 @@ public void testOpenEntityManagerInViewInterceptorAsyncScenario() throws Excepti
 
 		WebAsyncManager asyncManager = AsyncWebUtils.getAsyncManager(webRequest);
 		asyncManager.setAsyncWebRequest(asyncWebRequest);
-
 		asyncManager.startCallableProcessing(new Callable<String>() {
 			public String call() throws Exception {
 				return "anything";
 			}
 		});
 
 		verify(asyncWebRequest);
-		reset(asyncWebRequest);
-		replay(asyncWebRequest);
 
 		interceptor.afterConcurrentHandlingStarted(webRequest);
 		assertFalse(TransactionSynchronizationManager.hasResource(factory));
 
 		// Async dispatch thread
 
+		reset(asyncWebRequest);
+		expect(asyncWebRequest.isDispatched()).andReturn(true);
+		replay(asyncWebRequest);
+
 		reset(manager, factory);
 		replay(manager, factory);
 
@@ -345,11 +344,11 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
 
 		FilterChain filterChain3 = new PassThroughFilterChain(filter2, filterChain2);
 
-		AsyncWebRequest asyncWebRequest = createStrictMock(AsyncWebRequest.class);
+		AsyncWebRequest asyncWebRequest = createMock(AsyncWebRequest.class);
 		asyncWebRequest.addCompletionHandler((Runnable) anyObject());
 		asyncWebRequest.startAsync();
-		expect(asyncWebRequest.isAsyncStarted()).andReturn(true);
-		expectLastCall().anyTimes();
+		expect(asyncWebRequest.isAsyncStarted()).andReturn(true).anyTimes();
+		expect(asyncWebRequest.isDispatched()).andReturn(false).anyTimes();
 		replay(asyncWebRequest);
 
 		WebAsyncManager asyncManager = AsyncWebUtils.getAsyncManager(request);
@@ -373,6 +372,7 @@ public String call() throws Exception {
 
 		reset(asyncWebRequest);
 		expect(asyncWebRequest.isAsyncStarted()).andReturn(false).anyTimes();
+		expect(asyncWebRequest.isDispatched()).andReturn(true).anyTimes();
 		replay(asyncWebRequest);
 
 		assertFalse(TransactionSynchronizationManager.hasResource(factory));

spring-web/src/main/java/org/springframework/web/context/request/AsyncWebRequestInterceptor.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.web.context.request;
+
+/**
+ * Extends {@code WebRequestInterceptor} with a callback method invoked during
+ * asynchronous request handling.
+ *
+ * <p>When a handler starts asynchronous request handling, the DispatcherServlet
+ * exits without invoking {@code postHandle} and {@code afterCompletion}, as it
+ * normally does, since the results of request handling (e.g. ModelAndView) are
+ * not available in the current thread and handling is not yet complete.
+ * In such scenarios, the {@link #afterConcurrentHandlingStarted(WebRequest)}
+ * method is invoked instead allowing implementations to perform tasks such as
+ * cleaning up thread bound attributes.
+ *
+ * <p>When asynchronous handling completes, the request is dispatched to the
+ * container for further processing. At this stage the DispatcherServlet invokes
+ * {@code preHandle}, {@code postHandle} and {@code afterCompletion} as usual.
+ *
+ * @author Rossen Stoyanchev
+ * @since 3.2
+ *
+ * @see org.springframework.web.context.request.async.WebAsyncManager
+ */
+public interface AsyncWebRequestInterceptor extends WebRequestInterceptor{
+
+	/**
+	 * Called instead of {@code postHandle} and {@code afterCompletion}, when the
+	 * the handler started handling the request concurrently.
+	 *
+	 * @param request the current request
+	 */
+	void afterConcurrentHandlingStarted(WebRequest request);
+
+}

spring-web/src/main/java/org/springframework/web/context/request/Log4jNestedDiagnosticContextInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2012 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@
 import org.apache.log4j.Logger;
 import org.apache.log4j.NDC;
 import org.springframework.ui.ModelMap;
-import org.springframework.web.context.request.async.AsyncWebUtils;
 
 /**
  * Request logging interceptor that adds a request context message to the
@@ -31,7 +30,7 @@
  * @see org.apache.log4j.NDC#push(String)
  * @see org.apache.log4j.NDC#pop()
  */
-public class Log4jNestedDiagnosticContextInterceptor implements WebRequestInterceptor {
+public class Log4jNestedDiagnosticContextInterceptor implements AsyncWebRequestInterceptor {
 
 	/** Logger available to subclasses */
 	protected final Logger log4jLogger = Logger.getLogger(getClass());
@@ -60,11 +59,6 @@ protected boolean isIncludeClientInfo() {
 	 * Adds a message the Log4J NDC before the request is processed.
 	 */
 	public void preHandle(WebRequest request) throws Exception {
-
-		if (AsyncWebUtils.getAsyncManager(request).hasConcurrentResult()) {
-			return;
-		}
-
 		NDC.push(getNestedDiagnosticContextMessage(request));
 	}
 
@@ -93,4 +87,15 @@ public void afterCompletion(WebRequest request, Exception ex) throws Exception {
 		}
 	}
 
+	/**
+	 * Removes the log message from the Log4J NDC when the processing thread is
+	 * exited after the start of asynchronous request handling.
+	 */
+	public void afterConcurrentHandlingStarted(WebRequest request) {
+		NDC.pop();
+		if (NDC.getDepth() == 0) {
+			NDC.remove();
+		}
+	}
+
 }

spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java

@@ -37,7 +37,7 @@ public interface AsyncWebRequest extends NativeWebRequest {
 	void setTimeout(Long timeout);
 
 	/**
-	 * Set a handler to be invoked if concurrent processing times out.
+	 * Set the handler to use when concurrent handling has timed out.
 	 */
 	void setTimeoutHandler(Runnable runnable);
 

spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequestInterceptor.java

@@ -22,8 +22,7 @@
 import org.springframework.web.context.request.ServletWebRequest;
 
 /**
- * An implementation of {@link AsyncWebRequest} to use when no underlying support is available.
- * The  methods {@link #startAsync()} and {@link #dispatch()} raise {@link UnsupportedOperationException}.
+ * An {@code AsyncWebRequest} to use when there is no underlying async support.
  *
  * @author Rossen Stoyanchev
  * @since 3.2
@@ -54,11 +53,13 @@ public boolean isDispatched() {
 		return false;
 	}
 
-	public boolean isAsyncComplete() {
+	// Not supported
+
+	public void startAsync() {
 		throw new UnsupportedOperationException("No async support in a pre-Servlet 3.0 runtime");
 	}
 
-	public void startAsync() {
+	public boolean isAsyncComplete() {
 		throw new UnsupportedOperationException("No async support in a pre-Servlet 3.0 runtime");
 	}
 

spring-web/src/main/java/org/springframework/web/context/request/async/NoSupportAsyncWebRequest.java

@@ -67,13 +67,19 @@ public StandardServletAsyncWebRequest(HttpServletRequest request, HttpServletRes
 
 	/**
 	 * {@inheritDoc}
-	 * <p>The timeout period begins when the main processing thread has exited.
+	 * <p>In Servlet 3 async processing, the timeout period begins after the
+	 * container processing thread has exited.
 	 */
 	public void setTimeout(Long timeout) {
 		Assert.state(!isAsyncStarted(), "Cannot change the timeout with concurrent handling in progress");
 		this.timeout = timeout;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 * <p>If not set, by default a timeout is handled by returning
+	 * SERVICE_UNAVAILABLE (503).
+	 */
 	public void setTimeoutHandler(Runnable timeoutHandler) {
 		if (timeoutHandler != null) {
 			this.timeoutHandler = timeoutHandler;

spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java

@@ -52,7 +52,7 @@
  * @author Rossen Stoyanchev
  * @since 3.2
  *
- * @see org.springframework.web.context.request.async.AsyncWebRequestInterceptor
+ * @see org.springframework.web.context.request.AsyncWebRequestInterceptor
  * @see org.springframework.web.servlet.AsyncHandlerInterceptor
  * @see org.springframework.web.filter.OncePerRequestFilter#shouldFilterAsyncDispatches
  * @see org.springframework.web.filter.OncePerRequestFilter#isAsyncDispatch
@@ -115,26 +115,23 @@ public void setTaskExecutor(AsyncTaskExecutor taskExecutor) {
 	}
 
 	/**
-	 * Whether the target handler chose to handle the request asynchronously.
-	 * A return value of "true" indicates concurrent handling is under way and the
-	 * response will remain open. A return value of "false" will be returned again after concurrent
-	 * handling produces a result and the request is dispatched to resume processing.
+	 * Whether the selected handler for the current request chose to handle the
+	 * request asynchronously. A return value of "true" indicates concurrent
+	 * handling is under way and the response will remain open. A return value
+	 * of "false" means concurrent handling was either not started or possibly
+	 * that it has completed and the request was dispatched for further
+	 * processing of the concurrent result.
 	 */
 	public boolean isConcurrentHandlingStarted() {
-		return ((this.asyncWebRequest != null) && (this.asyncWebRequest.isAsyncStarted()));
+		return ((this.asyncWebRequest != null) && this.asyncWebRequest.isAsyncStarted());
 	}
 
 	/**
-	 * Whether the request was dispatched to resume processing the result of
+	 * Whether the request has been dispatched to process the result of
 	 * concurrent handling.
 	 */
 	public boolean hasConcurrentResult() {
-
-		// TODO:
-		//	Add check for asyncWebRequest.isDispatched() once Apache id=53632 is fixed.
-		// 	That ensure "true" is returned in the dispatched thread only.
-
-		return this.concurrentResult != RESULT_NONE;
+		return ((this.concurrentResult != RESULT_NONE) && this.asyncWebRequest.isDispatched());
 	}
 
 	/**