From d701464517d66b095485671413e4150d9b5f14e3 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 30 Oct 2012 21:55:38 -0400 Subject: [PATCH] Add onTimeout/onCompletion callbacks to DeferredResult Issue: SPR-9914 --- .../support/OpenSessionInViewTests.java | 4 +- .../support/OpenEntityManagerInViewTests.java | 7 +- .../web/servlet/TestDispatcherServlet.java | 2 +- .../servlet/result/RequestResultMatchers.java | 4 +- .../request/async/AsyncWebRequest.java | 6 +- .../async/CallableInterceptorChain.java | 4 +- .../context/request/async/DeferredResult.java | 93 +++++++++---- .../async/DeferredResultInterceptorChain.java | 6 +- .../DeferredResultProcessingInterceptor.java | 2 +- .../{AsyncTask.java => MvcAsyncTask.java} | 75 +++++++++-- .../async/NoSupportAsyncWebRequest.java | 2 +- .../async/StandardServletAsyncWebRequest.java | 10 +- .../request/async/WebAsyncManager.java | 124 ++++++++++-------- .../request/async/DeferredResultTests.java | 36 +++-- .../StandardServletAsyncWebRequestTests.java | 2 +- .../request/async/WebAsyncManagerTests.java | 6 +- .../async/WebAsyncManagerTimeoutTests.java | 60 +++++++-- .../web/servlet/FrameworkServlet.java | 4 +- .../annotation/AsyncSupportConfigurer.java | 4 +- .../AsyncTaskMethodReturnValueHandler.java | 12 +- .../RequestMappingHandlerAdapter.java | 4 +- 21 files changed, 309 insertions(+), 158 deletions(-) rename spring-web/src/main/java/org/springframework/web/context/request/async/{AsyncTask.java => MvcAsyncTask.java} (54%) diff --git a/spring-orm/src/test/java/org/springframework/orm/hibernate3/support/OpenSessionInViewTests.java b/spring-orm/src/test/java/org/springframework/orm/hibernate3/support/OpenSessionInViewTests.java index 5cc6d78bb45..bdf66918744 100644 --- a/spring-orm/src/test/java/org/springframework/orm/hibernate3/support/OpenSessionInViewTests.java +++ b/spring-orm/src/test/java/org/springframework/orm/hibernate3/support/OpenSessionInViewTests.java @@ -178,7 +178,7 @@ public class OpenSessionInViewTests { AsyncWebRequest asyncWebRequest = createStrictMock(AsyncWebRequest.class); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); - asyncWebRequest.setTimeoutHandler((Runnable) anyObject()); + asyncWebRequest.addTimeoutHandler((Runnable) anyObject()); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); asyncWebRequest.startAsync(); replay(asyncWebRequest); @@ -494,7 +494,7 @@ public class OpenSessionInViewTests { AsyncWebRequest asyncWebRequest = createMock(AsyncWebRequest.class); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); - asyncWebRequest.setTimeoutHandler(EasyMock.anyObject()); + asyncWebRequest.addTimeoutHandler(EasyMock.anyObject()); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); asyncWebRequest.startAsync(); expect(asyncWebRequest.isAsyncStarted()).andReturn(true).anyTimes(); diff --git a/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java b/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java index 28c56ea2b72..c0e971174a4 100644 --- a/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java +++ b/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java @@ -20,7 +20,6 @@ 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; @@ -49,8 +48,8 @@ import org.springframework.transaction.support.TransactionSynchronizationManager import org.springframework.web.context.WebApplicationContext; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.request.async.AsyncWebRequest; -import org.springframework.web.context.request.async.WebAsyncUtils; import org.springframework.web.context.request.async.WebAsyncManager; +import org.springframework.web.context.request.async.WebAsyncUtils; import org.springframework.web.context.support.StaticWebApplicationContext; /** @@ -155,7 +154,7 @@ public class OpenEntityManagerInViewTests extends TestCase { AsyncWebRequest asyncWebRequest = createStrictMock(AsyncWebRequest.class); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); - asyncWebRequest.setTimeoutHandler((Runnable) anyObject()); + asyncWebRequest.addTimeoutHandler((Runnable) anyObject()); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); asyncWebRequest.startAsync(); replay(asyncWebRequest); @@ -346,7 +345,7 @@ public class OpenEntityManagerInViewTests extends TestCase { AsyncWebRequest asyncWebRequest = createMock(AsyncWebRequest.class); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); - asyncWebRequest.setTimeoutHandler((Runnable) anyObject()); + asyncWebRequest.addTimeoutHandler((Runnable) anyObject()); asyncWebRequest.addCompletionHandler((Runnable) anyObject()); asyncWebRequest.startAsync(); expect(asyncWebRequest.isAsyncStarted()).andReturn(true).anyTimes(); diff --git a/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/TestDispatcherServlet.java b/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/TestDispatcherServlet.java index 795f32b35b9..6a22d24f4af 100644 --- a/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/TestDispatcherServlet.java +++ b/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/TestDispatcherServlet.java @@ -48,7 +48,7 @@ import org.springframework.web.servlet.ModelAndView; @SuppressWarnings("serial") final class TestDispatcherServlet extends DispatcherServlet { - private static final String KEY = TestDispatcherServlet.class.getName() + "-interceptor"; + private static final String KEY = TestDispatcherServlet.class.getName() + ".interceptor"; /** * Create a new instance with the given web application context. diff --git a/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/result/RequestResultMatchers.java b/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/result/RequestResultMatchers.java index 2f7c84390b0..3b6d1cf15ea 100644 --- a/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/result/RequestResultMatchers.java +++ b/spring-test-mvc/src/main/java/org/springframework/test/web/servlet/result/RequestResultMatchers.java @@ -28,7 +28,7 @@ import org.hamcrest.Matchers; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.ResultMatcher; -import org.springframework.web.context.request.async.AsyncTask; +import org.springframework.web.context.request.async.MvcAsyncTask; import org.springframework.web.context.request.async.DeferredResult; /** @@ -97,7 +97,7 @@ public class RequestResultMatchers { /** * Assert the result from asynchronous processing. * This method can be used when a controller method returns {@link Callable} - * or {@link AsyncTask}. The value matched is the value returned from the + * or {@link MvcAsyncTask}. The value matched is the value returned from the * {@code Callable} or the exception raised. */ public ResultMatcher asyncResult(Object expectedResult) { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java index 9c9470b25a7..8b2bc8b9e82 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java @@ -37,12 +37,12 @@ public interface AsyncWebRequest extends NativeWebRequest { void setTimeout(Long timeout); /** - * Set the handler to use when concurrent handling has timed out. + * Add a handler to invoke when concurrent handling has timed out. */ - void setTimeoutHandler(Runnable runnable); + void addTimeoutHandler(Runnable runnable); /** - * Add a Runnable to be invoked when request processing completes. + * Add a handle to invoke when request processing completes. */ void addCompletionHandler(Runnable runnable); diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java index 72ac05a0ed9..bd58585c7fc 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java @@ -39,8 +39,8 @@ class CallableInterceptorChain { private int preProcessIndex = -1; - public CallableInterceptorChain(Collection interceptors) { - this.interceptors = new ArrayList(interceptors); + public CallableInterceptorChain(List interceptors) { + this.interceptors = interceptors; } public void applyPreProcess(NativeWebRequest request, Callable task) throws Exception { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java index 9b4cc499bd3..1d63644e24d 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java @@ -20,6 +20,7 @@ import java.util.concurrent.Callable; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.Assert; +import org.springframework.web.context.request.NativeWebRequest; /** * {@code DeferredResult} provides an alternative to using a {@link Callable} @@ -41,6 +42,10 @@ public final class DeferredResult { private final Object timeoutResult; + private Runnable timeoutCallback; + + private Runnable completionCallback; + private DeferredResultHandler resultHandler; private Object result = RESULT_NONE; @@ -56,7 +61,7 @@ public final class DeferredResult { } /** - * Create a DeferredResult with a timeout. + * Create a DeferredResult with a timeout value. * @param timeout timeout value in milliseconds */ public DeferredResult(long timeout) { @@ -64,7 +69,8 @@ public final class DeferredResult { } /** - * Create a DeferredResult with a timeout and a default result to use on timeout. + * Create a DeferredResult with a timeout value and a default result to use + * in case of timeout. * @param timeout timeout value in milliseconds; ignored if {@code null} * @param timeoutResult the result to use */ @@ -73,13 +79,48 @@ public final class DeferredResult { this.timeout = timeout; } + /** + * Return {@code true} if this DeferredResult is no longer usable either + * because it was previously set or because the underlying request expired. + *

+ * The result may have been set with a call to {@link #setResult(Object)}, + * or {@link #setErrorResult(Object)}, or as a result of a timeout, if a + * timeout result was provided to the constructor. The request may also + * expire due to a timeout or network error. + */ + public boolean isSetOrExpired() { + return ((this.result != RESULT_NONE) || this.expired); + } + /** * Return the configured timeout value in milliseconds. */ - public Long getTimeoutMilliseconds() { + Long getTimeoutValue() { return this.timeout; } + /** + * Register code to invoke when the async request times out. This method is + * called from a container thread when an async request times out before the + * {@code DeferredResult} has been set. It may invoke + * {@link DeferredResult#setResult(Object) setResult} or + * {@link DeferredResult#setErrorResult(Object) setErrorResult} to resume + * processing. + */ + public void onTimeout(Runnable callback) { + this.timeoutCallback = callback; + } + + /** + * Register code to invoke when the async request completes. This method is + * called from a container thread when an async request completed for any + * reason including timeout and network error. This method is useful for + * detecting that a {@code DeferredResult} instance is no longer usable. + */ + public void onCompletion(Runnable callback) { + this.completionCallback = callback; + } + /** * Provide a handler to use to handle the result value. * @param resultHandler the handler @@ -138,33 +179,29 @@ public final class DeferredResult { return setResultInternal(result); } - /** - * Return {@code true} if this DeferredResult is no longer usable either - * because it was previously set or because the underlying request expired. - *

- * The result may have been set with a call to {@link #setResult(Object)}, - * or {@link #setErrorResult(Object)}, or as a result of a timeout, if a - * timeout result was provided to the constructor. The request may also - * expire due to a timeout or network error. - */ - public boolean isSetOrExpired() { - return ((this.result != RESULT_NONE) || this.expired); - } + DeferredResultProcessingInterceptor getInterceptor() { + return new DeferredResultProcessingInterceptorAdapter() { - /** - * Mark this instance expired so it may no longer be used. - * @return the previous value of the expiration flag - */ - boolean expire() { - synchronized (this) { - boolean previous = this.expired; - this.expired = true; - return previous; - } - } + @Override + public void afterTimeout(NativeWebRequest request, DeferredResult deferredResult) { + if (timeoutCallback != null) { + timeoutCallback.run(); + } + if (DeferredResult.this.timeoutResult != RESULT_NONE) { + setResultInternal(timeoutResult); + } + } - boolean applyTimeoutResult() { - return (this.timeoutResult != RESULT_NONE) ? setResultInternal(this.timeoutResult) : false; + @Override + public void afterCompletion(NativeWebRequest request, DeferredResult deferredResult) { + synchronized (this) { + expired = true; + } + if (completionCallback != null) { + completionCallback.run(); + } + } + }; } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java index f36659655e6..336eb6ddcc7 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java @@ -15,8 +15,6 @@ */ package org.springframework.web.context.request.async; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import org.apache.commons.logging.Log; @@ -38,8 +36,8 @@ class DeferredResultInterceptorChain { private int preProcessingIndex = -1; - public DeferredResultInterceptorChain(Collection interceptors) { - this.interceptors = new ArrayList(interceptors); + public DeferredResultInterceptorChain(List interceptors) { + this.interceptors = interceptors; } public void applyPreProcess(NativeWebRequest request, DeferredResult deferredResult) throws Exception { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptor.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptor.java index c50f2e2bfaf..f91e37ef285 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptor.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptor.java @@ -75,7 +75,7 @@ public interface DeferredResultProcessingInterceptor { * Invoked from a container thread when an async request times out before * the {@code DeferredResult} has been set. Implementations may invoke * {@link DeferredResult#setResult(Object) setResult} or - * {@link DeferredResult#setErrorResult(Object) to resume processing. + * {@link DeferredResult#setErrorResult(Object) setErrorResult} to resume processing. * * @param request the current request * @param deferredResult the DeferredResult for the current request; if the diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncTask.java b/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java similarity index 54% rename from spring-web/src/main/java/org/springframework/web/context/request/async/AsyncTask.java rename to spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java index 2383bf4c5f2..6720606c5ee 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncTask.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java @@ -20,6 +20,7 @@ import java.util.concurrent.Callable; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.util.Assert; +import org.springframework.web.context.request.NativeWebRequest; /** * Holder for a {@link Callable}, a timeout value, and a task executor. @@ -27,7 +28,7 @@ import org.springframework.util.Assert; * @author Rossen Stoyanchev * @since 3.2 */ -public class AsyncTask { +public class MvcAsyncTask { private final Callable callable; @@ -37,40 +38,51 @@ public class AsyncTask { private final AsyncTaskExecutor executor; + private Callable timeoutCallback; + + private Runnable completionCallback; + private BeanFactory beanFactory; /** - * Create an AsyncTask with a timeout value and a Callable. - * @param timeout timeout value in milliseconds + * Create an {@code MvcAsyncTask} wrapping the given {@link Callable}. * @param callable the callable for concurrent handling */ - public AsyncTask(long timeout, Callable callable) { - this(timeout, null, null, callable); - Assert.notNull(timeout, "Timeout must not be null"); + public MvcAsyncTask(Callable callable) { + this(null, null, null, callable); } /** - * Create an AsyncTask with a timeout value, an executor name, and a Callable. + * Create an {@code MvcAsyncTask} with a timeout value and a {@link Callable}. + * @param timeout timeout value in milliseconds + * @param callable the callable for concurrent handling + */ + public MvcAsyncTask(long timeout, Callable callable) { + this(timeout, null, null, callable); + } + + /** + * Create an {@code MvcAsyncTask} with a timeout value, an executor name, and a {@link Callable}. * @param timeout timeout value in milliseconds; ignored if {@code null} * @param callable the callable for concurrent handling */ - public AsyncTask(Long timeout, String executorName, Callable callable) { + public MvcAsyncTask(Long timeout, String executorName, Callable callable) { this(timeout, null, executorName, callable); Assert.notNull(executor, "Executor name must not be null"); } /** - * Create an AsyncTask with a timeout value, an executor instance, and a Callable. + * Create an {@code MvcAsyncTask} with a timeout value, an executor instance, and a Callable. * @param timeout timeout value in milliseconds; ignored if {@code null} * @param callable the callable for concurrent handling */ - public AsyncTask(Long timeout, AsyncTaskExecutor executor, Callable callable) { + public MvcAsyncTask(Long timeout, AsyncTaskExecutor executor, Callable callable) { this(timeout, executor, null, callable); Assert.notNull(executor, "Executor must not be null"); } - private AsyncTask(Long timeout, AsyncTaskExecutor executor, String executorName, Callable callable) { + private MvcAsyncTask(Long timeout, AsyncTaskExecutor executor, String executorName, Callable callable) { Assert.notNull(callable, "Callable must not be null"); this.callable = callable; this.timeout = timeout; @@ -111,11 +123,50 @@ public class AsyncTask { /** * A {@link BeanFactory} to use to resolve an executor name. Applications are - * not expected to have to set this property when AsyncTask is used in a + * not expected to have to set this property when {@code MvcAsyncTask} is used in a * Spring MVC controller. */ public void setBeanFactory(BeanFactory beanFactory) { this.beanFactory = beanFactory; } + + /** + * Register code to invoke when the async request times out. This method is + * called from a container thread when an async request times out before the + * {@code Callable} has completed. The callback is executed in the same + * thread and therefore should return without blocking. It may return an + * alternative value to use, including an {@link Exception} or return + * {@link CallableProcessingInterceptor#RESULT_NONE RESULT_NONE}. + */ + public void onTimeout(Callable callback) { + this.timeoutCallback = callback; + } + + /** + * Register code to invoke when the async request completes. This method is + * called from a container thread when an async request completed for any + * reason including timeout and network error. + */ + public void onCompletion(Runnable callback) { + this.completionCallback = callback; + } + + CallableProcessingInterceptor getInterceptor() { + return new CallableProcessingInterceptorAdapter() { + + @Override + public Object afterTimeout(NativeWebRequest request, Callable task) throws Exception { + return (timeoutCallback != null) ? timeoutCallback.call() : CallableProcessingInterceptor.RESULT_NONE; + } + + @Override + public void afterCompletion(NativeWebRequest request, Callable task) throws Exception { + if (completionCallback != null) { + completionCallback.run(); + } + } + }; + } + } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/NoSupportAsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/NoSupportAsyncWebRequest.java index 956b6b406d6..6158a829317 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/NoSupportAsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/NoSupportAsyncWebRequest.java @@ -41,7 +41,7 @@ public class NoSupportAsyncWebRequest extends ServletWebRequest implements Async // ignored } - public void setTimeoutHandler(Runnable runnable) { + public void addTimeoutHandler(Runnable runnable) { // ignored } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java index 7b90779314d..8e5e6d4d6b0 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java @@ -49,7 +49,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements private AtomicBoolean asyncCompleted = new AtomicBoolean(false); - private Runnable timeoutHandler; + private final List timeoutHandlers = new ArrayList(); private final List completionHandlers = new ArrayList(); @@ -73,8 +73,8 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements this.timeout = timeout; } - public void setTimeoutHandler(Runnable timeoutHandler) { - this.timeoutHandler = timeoutHandler; + public void addTimeoutHandler(Runnable timeoutHandler) { + this.timeoutHandlers.add(timeoutHandler); } public void addCompletionHandler(Runnable runnable) { @@ -127,8 +127,8 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } public void onTimeout(AsyncEvent event) throws IOException { - if (this.timeoutHandler != null) { - this.timeoutHandler.run(); + for (Runnable handler : this.timeoutHandlers) { + handler.run(); } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index aa6de062c7c..bbd23577ed5 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -15,7 +15,9 @@ */ package org.springframework.web.context.request.async; +import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -155,19 +157,27 @@ public final class WebAsyncManager { return this.concurrentResultContext; } + /** + * Get the {@link CallableProcessingInterceptor} registered under the given key. + * @param key the key + * @return the interceptor registered under that key or {@code null} + */ public CallableProcessingInterceptor getCallableInterceptor(Object key) { return this.callableInterceptors.get(key); } + /** + * Get the {@link DeferredResultProcessingInterceptor} registered under the given key. + * @param key the key + * @return the interceptor registered under that key or {@code null} + */ public DeferredResultProcessingInterceptor getDeferredResultInterceptor(Object key) { return this.deferredResultInterceptors.get(key); } /** - * Register a {@link CallableProcessingInterceptor} that will be applied - * when concurrent request handling with a {@link Callable} starts. - * - * @param key a unique the key under which to register the interceptor + * Register a {@link CallableProcessingInterceptor} under the given key. + * @param key the key * @param interceptor the interceptor to register */ public void registerCallableInterceptor(Object key, CallableProcessingInterceptor interceptor) { @@ -181,11 +191,8 @@ public final class WebAsyncManager { } /** - * Register a {@link DeferredResultProcessingInterceptor} that will be - * applied when concurrent request handling with a {@link DeferredResult} - * starts. - * - * @param key a unique the key under which to register the interceptor + * Register a {@link DeferredResultProcessingInterceptor} under the given key. + * @param key the key * @param interceptor the interceptor to register */ public void registerDeferredResultInterceptor(Object key, DeferredResultProcessingInterceptor interceptor) { @@ -221,16 +228,47 @@ public final class WebAsyncManager { * @see #getConcurrentResult() * @see #getConcurrentResultContext() */ + @SuppressWarnings({ "rawtypes", "unchecked" }) public void startCallableProcessing(final Callable callable, Object... processingContext) { Assert.notNull(callable, "Callable must not be null"); + startCallableProcessing(new MvcAsyncTask(callable), processingContext); + } + + /** + * Use the given {@link MvcAsyncTask} to configure the task executor as well as + * the timeout value of the {@code AsyncWebRequest} before delegating to + * {@link #startCallableProcessing(Callable, Object...)}. + * + * @param mvcAsyncTask an MvcAsyncTask containing the target {@code Callable} + * @param processingContext additional context to save that can be accessed + * via {@link #getConcurrentResultContext()} + */ + public void startCallableProcessing(final MvcAsyncTask mvcAsyncTask, Object... processingContext) { + Assert.notNull(mvcAsyncTask, "MvcAsyncTask must not be null"); Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null"); - final CallableInterceptorChain chain = new CallableInterceptorChain(this.callableInterceptors.values()); + final Callable callable = mvcAsyncTask.getCallable(); - this.asyncWebRequest.setTimeoutHandler(new Runnable() { + Long timeout = mvcAsyncTask.getTimeout(); + if (timeout != null) { + this.asyncWebRequest.setTimeout(timeout); + } + + AsyncTaskExecutor executor = mvcAsyncTask.getExecutor(); + if (executor != null) { + this.taskExecutor = executor; + } + + List interceptors = new ArrayList(); + interceptors.add(mvcAsyncTask.getInterceptor()); + interceptors.addAll(this.callableInterceptors.values()); + + final CallableInterceptorChain interceptorChain = new CallableInterceptorChain(interceptors); + + this.asyncWebRequest.addTimeoutHandler(new Runnable() { public void run() { logger.debug("Processing timeout"); - Object result = chain.triggerAfterTimeout(asyncWebRequest, callable); + Object result = interceptorChain.triggerAfterTimeout(asyncWebRequest, callable); if (result != CallableProcessingInterceptor.RESULT_NONE) { setConcurrentResultAndDispatch(result); } @@ -239,7 +277,7 @@ public final class WebAsyncManager { this.asyncWebRequest.addCompletionHandler(new Runnable() { public void run() { - chain.triggerAfterCompletion(asyncWebRequest, callable); + interceptorChain.triggerAfterCompletion(asyncWebRequest, callable); } }); @@ -249,14 +287,14 @@ public final class WebAsyncManager { public void run() { Object result = null; try { - chain.applyPreProcess(asyncWebRequest, callable); + interceptorChain.applyPreProcess(asyncWebRequest, callable); result = callable.call(); } catch (Throwable t) { result = t; } finally { - result = chain.applyPostProcess(asyncWebRequest, callable, result); + result = interceptorChain.applyPostProcess(asyncWebRequest, callable, result); } setConcurrentResultAndDispatch(result); } @@ -282,32 +320,6 @@ public final class WebAsyncManager { asyncWebRequest.dispatch(); } - /** - * Use the given {@link AsyncTask} to configure the task executor as well as - * the timeout value of the {@code AsyncWebRequest} before delegating to - * {@link #startCallableProcessing(Callable, Object...)}. - * - * @param asyncTask an asyncTask containing the target {@code Callable} - * @param processingContext additional context to save that can be accessed - * via {@link #getConcurrentResultContext()} - */ - public void startCallableProcessing(AsyncTask asyncTask, Object... processingContext) { - Assert.notNull(asyncTask, "AsyncTask must not be null"); - Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null"); - - Long timeout = asyncTask.getTimeout(); - if (timeout != null) { - this.asyncWebRequest.setTimeout(timeout); - } - - AsyncTaskExecutor executor = asyncTask.getExecutor(); - if (executor != null) { - this.taskExecutor = executor; - } - - startCallableProcessing(asyncTask.getCallable(), processingContext); - } - /** * Start concurrent request processing and initialize the given * {@link DeferredResult} with a {@link DeferredResultHandler} that saves @@ -329,41 +341,41 @@ public final class WebAsyncManager { Assert.notNull(deferredResult, "DeferredResult must not be null"); Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null"); - Long timeout = deferredResult.getTimeoutMilliseconds(); + Long timeout = deferredResult.getTimeoutValue(); if (timeout != null) { this.asyncWebRequest.setTimeout(timeout); } - final DeferredResultInterceptorChain chain = - new DeferredResultInterceptorChain(this.deferredResultInterceptors.values()); + List interceptors = new ArrayList(); + interceptors.add(deferredResult.getInterceptor()); + interceptors.addAll(this.deferredResultInterceptors.values()); - this.asyncWebRequest.setTimeoutHandler(new Runnable() { + final DeferredResultInterceptorChain interceptorChain = new DeferredResultInterceptorChain(interceptors); + + this.asyncWebRequest.addTimeoutHandler(new Runnable() { public void run() { - if (!deferredResult.applyTimeoutResult()) { - try { - chain.triggerAfterTimeout(asyncWebRequest, deferredResult); - } - catch (Throwable t) { - setConcurrentResultAndDispatch(t); - } + try { + interceptorChain.triggerAfterTimeout(asyncWebRequest, deferredResult); + } + catch (Throwable t) { + setConcurrentResultAndDispatch(t); } } }); this.asyncWebRequest.addCompletionHandler(new Runnable() { public void run() { - deferredResult.expire(); - chain.triggerAfterCompletion(asyncWebRequest, deferredResult); + interceptorChain.triggerAfterCompletion(asyncWebRequest, deferredResult); } }); startAsyncProcessing(processingContext); try { - chain.applyPreProcess(this.asyncWebRequest, deferredResult); + interceptorChain.applyPreProcess(this.asyncWebRequest, deferredResult); deferredResult.setResultHandler(new DeferredResultHandler() { public void handleResult(Object result) { - result = chain.applyPostProcess(asyncWebRequest, deferredResult, result); + result = interceptorChain.applyPostProcess(asyncWebRequest, deferredResult, result); setConcurrentResultAndDispatch(result); } }); diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java index c4e70096dfb..9820a98df8e 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java @@ -19,6 +19,7 @@ package org.springframework.web.context.request.async; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -80,27 +81,42 @@ public class DeferredResultTests { } @Test - public void setExpired() { - DeferredResult result = new DeferredResult(); - assertFalse(result.isSetOrExpired()); + public void onCompletion() throws Exception { + final StringBuilder sb = new StringBuilder(); + + DeferredResult result = new DeferredResult(); + result.onCompletion(new Runnable() { + public void run() { + sb.append("completion event"); + } + }); + + result.getInterceptor().afterCompletion(null, null); - result.expire(); assertTrue(result.isSetOrExpired()); - assertFalse(result.setResult("hello")); + assertEquals("completion event", sb.toString()); } @Test - public void applyTimeoutResult() { + public void onTimeout() throws Exception { + final StringBuilder sb = new StringBuilder(); + DeferredResultHandler handler = createMock(DeferredResultHandler.class); - handler.handleResult("timed out"); + handler.handleResult("timeout result"); replay(handler); - DeferredResult result = new DeferredResult(null, "timed out"); + DeferredResult result = new DeferredResult(null, "timeout result"); result.setResultHandler(handler); + result.onTimeout(new Runnable() { + public void run() { + sb.append("timeout event"); + } + }); - assertTrue(result.applyTimeoutResult()); - assertFalse("Shouldn't be able to set result after timeout", result.setResult("hello")); + result.getInterceptor().afterTimeout(null, null); + assertEquals("timeout event", sb.toString()); + assertFalse("Should not be able to set result a second time", result.setResult("hello")); verify(handler); } diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java index 2e91abf993c..d263a6289e2 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java @@ -128,7 +128,7 @@ public class StandardServletAsyncWebRequestTests { timeoutHandler.run(); replay(timeoutHandler); - this.asyncRequest.setTimeoutHandler(timeoutHandler); + this.asyncRequest.addTimeoutHandler(timeoutHandler); this.asyncRequest.onTimeout(new AsyncEvent(null)); verify(timeoutHandler); diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java index f4335766467..a4e5a334a94 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java @@ -208,13 +208,13 @@ public class WebAsyncManagerTests { replay(executor); this.asyncWebRequest.setTimeout(1000L); - this.asyncWebRequest.setTimeoutHandler(EasyMock.anyObject()); + this.asyncWebRequest.addTimeoutHandler(EasyMock.anyObject()); this.asyncWebRequest.addCompletionHandler(EasyMock.anyObject()); this.asyncWebRequest.startAsync(); replay(this.asyncWebRequest); @SuppressWarnings("unchecked") - AsyncTask asyncTask = new AsyncTask(1000L, executor, createMock(Callable.class)); + MvcAsyncTask asyncTask = new MvcAsyncTask(1000L, executor, createMock(Callable.class)); this.asyncManager.startCallableProcessing(asyncTask); verify(executor, this.asyncWebRequest); @@ -311,7 +311,7 @@ public class WebAsyncManagerTests { } private void setupDefaultAsyncScenario() { - this.asyncWebRequest.setTimeoutHandler((Runnable) notNull()); + this.asyncWebRequest.addTimeoutHandler((Runnable) notNull()); this.asyncWebRequest.addCompletionHandler((Runnable) notNull()); this.asyncWebRequest.startAsync(); expect(this.asyncWebRequest.isAsyncComplete()).andReturn(false); diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java index 691a213a6b8..9fd7c621e94 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java @@ -16,7 +16,6 @@ package org.springframework.web.context.request.async; -import static org.springframework.web.context.request.async.CallableProcessingInterceptor.RESULT_NONE; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.expect; @@ -25,6 +24,8 @@ import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.springframework.web.context.request.async.CallableProcessingInterceptor.RESULT_NONE; import java.util.concurrent.Callable; @@ -94,7 +95,27 @@ public class WebAsyncManagerTimeoutTests { } @Test - public void startCallableProcessingTimeoutAndResume() throws Exception { + public void startCallableProcessingTimeoutAndResumeThroughCallback() throws Exception { + + StubCallable callable = new StubCallable(); + MvcAsyncTask mvcAsyncTask = new MvcAsyncTask(callable); + mvcAsyncTask.onTimeout(new Callable() { + public Object call() throws Exception { + return 7; + } + }); + + this.asyncManager.startCallableProcessing(mvcAsyncTask); + + this.asyncWebRequest.onTimeout(ASYNC_EVENT); + + assertTrue(this.asyncManager.hasConcurrentResult()); + assertEquals(7, this.asyncManager.getConcurrentResult()); + assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); + } + + @Test + public void startCallableProcessingTimeoutAndResumeThroughInterceptor() throws Exception { StubCallable callable = new StubCallable(); @@ -107,6 +128,7 @@ public class WebAsyncManagerTimeoutTests { this.asyncWebRequest.onTimeout(ASYNC_EVENT); + assertTrue(this.asyncManager.hasConcurrentResult()); assertEquals(22, this.asyncManager.getConcurrentResult()); assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); @@ -128,6 +150,7 @@ public class WebAsyncManagerTimeoutTests { this.asyncWebRequest.onTimeout(ASYNC_EVENT); + assertTrue(this.asyncManager.hasConcurrentResult()); assertEquals(exception, this.asyncManager.getConcurrentResult()); assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); @@ -161,25 +184,38 @@ public class WebAsyncManagerTimeoutTests { public void startDeferredResultProcessingTimeoutAndResumeWithDefaultResult() throws Exception { DeferredResult deferredResult = new DeferredResult(null, 23); - - DeferredResultProcessingInterceptor interceptor = new DeferredResultProcessingInterceptorAdapter() { - public void afterTimeout(NativeWebRequest request, DeferredResult result) throws Exception { - result.setErrorResult("should not get here"); - } - }; - - this.asyncManager.registerDeferredResultInterceptor("interceptor", interceptor); this.asyncManager.startDeferredResultProcessing(deferredResult); AsyncEvent event = null; this.asyncWebRequest.onTimeout(event); + assertTrue(this.asyncManager.hasConcurrentResult()); assertEquals(23, this.asyncManager.getConcurrentResult()); assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); } @Test - public void startDeferredResultProcessingTimeoutAndResumeWithInterceptor() throws Exception { + public void startDeferredResultProcessingTimeoutAndResumeThroughCallback() throws Exception { + + final DeferredResult deferredResult = new DeferredResult(); + deferredResult.onTimeout(new Runnable() { + public void run() { + deferredResult.setResult(23); + } + }); + + this.asyncManager.startDeferredResultProcessing(deferredResult); + + AsyncEvent event = null; + this.asyncWebRequest.onTimeout(event); + + assertTrue(this.asyncManager.hasConcurrentResult()); + assertEquals(23, this.asyncManager.getConcurrentResult()); + assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); + } + + @Test + public void startDeferredResultProcessingTimeoutAndResumeThroughInterceptor() throws Exception { DeferredResult deferredResult = new DeferredResult(); @@ -195,6 +231,7 @@ public class WebAsyncManagerTimeoutTests { AsyncEvent event = null; this.asyncWebRequest.onTimeout(event); + assertTrue(this.asyncManager.hasConcurrentResult()); assertEquals(23, this.asyncManager.getConcurrentResult()); assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); } @@ -217,6 +254,7 @@ public class WebAsyncManagerTimeoutTests { AsyncEvent event = null; this.asyncWebRequest.onTimeout(event); + assertTrue(this.asyncManager.hasConcurrentResult()); assertEquals(exception, this.asyncManager.getConcurrentResult()); assertEquals("/test", ((MockAsyncContext) this.servletRequest.getAsyncContext()).getDispatchedPath()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index dc3652b6f67..a3b2f461ebb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -905,7 +905,7 @@ public abstract class FrameworkServlet extends HttpServletBean { initContextHolders(request, localeContext, requestAttributes); WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - asyncManager.registerCallableInterceptor(this.getClass().getName(), createRequestBindingInterceptor(request)); + asyncManager.registerCallableInterceptor(FrameworkServlet.class.getName(), getRequestBindingInterceptor(request)); try { doService(request, response); @@ -988,7 +988,7 @@ public abstract class FrameworkServlet extends HttpServletBean { } } - private CallableProcessingInterceptor createRequestBindingInterceptor(final HttpServletRequest request) { + private CallableProcessingInterceptor getRequestBindingInterceptor(final HttpServletRequest request) { return new CallableProcessingInterceptorAdapter() { @Override public void preProcess(NativeWebRequest webRequest, Callable task) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/AsyncSupportConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/AsyncSupportConfigurer.java index ddc199f6631..271c5b2c390 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/AsyncSupportConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/AsyncSupportConfigurer.java @@ -23,7 +23,7 @@ import java.util.concurrent.Callable; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.util.Assert; -import org.springframework.web.context.request.async.AsyncTask; +import org.springframework.web.context.request.async.MvcAsyncTask; import org.springframework.web.context.request.async.CallableProcessingInterceptor; import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.context.request.async.DeferredResultProcessingInterceptor; @@ -50,7 +50,7 @@ public class AsyncSupportConfigurer { /** * Set the default {@link AsyncTaskExecutor} to use when a controller method * returns a {@link Callable}. Controller methods can override this default on - * a per-request basis by returning an {@link AsyncTask}. + * a per-request basis by returning an {@link MvcAsyncTask}. * *

By default a {@link SimpleAsyncTaskExecutor} instance is used and it's * highly recommended to change that default in production since the simple diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java index c438394b655..d2a98220ddd 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java @@ -19,13 +19,13 @@ package org.springframework.web.servlet.mvc.method.annotation; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.MethodParameter; import org.springframework.web.context.request.NativeWebRequest; -import org.springframework.web.context.request.async.AsyncTask; +import org.springframework.web.context.request.async.MvcAsyncTask; import org.springframework.web.context.request.async.WebAsyncUtils; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** - * Handles return values of type {@link AsyncTask}. + * Handles return values of type {@link MvcAsyncTask}. * * @author Rossen Stoyanchev * @since 3.2 @@ -41,7 +41,7 @@ public class AsyncTaskMethodReturnValueHandler implements HandlerMethodReturnVal public boolean supportsReturnType(MethodParameter returnType) { Class paramType = returnType.getParameterType(); - return AsyncTask.class.isAssignableFrom(paramType); + return MvcAsyncTask.class.isAssignableFrom(paramType); } public void handleReturnValue(Object returnValue, @@ -52,9 +52,9 @@ public class AsyncTaskMethodReturnValueHandler implements HandlerMethodReturnVal return; } - AsyncTask asyncTask = (AsyncTask) returnValue; - asyncTask.setBeanFactory(this.beanFactory); - WebAsyncUtils.getAsyncManager(webRequest).startCallableProcessing(asyncTask, mavContainer); + MvcAsyncTask mvcAsyncTask = (MvcAsyncTask) returnValue; + mvcAsyncTask.setBeanFactory(this.beanFactory); + WebAsyncUtils.getAsyncManager(webRequest).startCallableProcessing(mvcAsyncTask, mavContainer); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 15ca91319bf..cda3705e61d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -63,7 +63,7 @@ import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.request.WebRequest; -import org.springframework.web.context.request.async.AsyncTask; +import org.springframework.web.context.request.async.MvcAsyncTask; import org.springframework.web.context.request.async.AsyncWebRequest; import org.springframework.web.context.request.async.CallableProcessingInterceptor; import org.springframework.web.context.request.async.DeferredResultProcessingInterceptor; @@ -350,7 +350,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i /** * Set the default {@link AsyncTaskExecutor} to use when a controller method * return a {@link Callable}. Controller methods can override this default on - * a per-request basis by returning an {@link AsyncTask}. + * a per-request basis by returning an {@link MvcAsyncTask}. *

By default a {@link SimpleAsyncTaskExecutor} instance is used. * It's recommended to change that default in production as the simple executor * does not re-use threads.