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 bd58585c7fc..a79a5a34c73 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 @@ -15,8 +15,6 @@ */ package org.springframework.web.context.request.async; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.concurrent.Callable; @@ -63,10 +61,13 @@ class CallableInterceptorChain { } public Object triggerAfterTimeout(NativeWebRequest request, Callable task) { - for (int i = this.interceptors.size()-1; i >= 0; i--) { + for (CallableProcessingInterceptor interceptor : this.interceptors) { try { - Object result = this.interceptors.get(i).afterTimeout(request, task); - if (result != CallableProcessingInterceptor.RESULT_NONE) { + Object result = interceptor.handleTimeout(request, task); + if (result == CallableProcessingInterceptor.RESPONSE_HANDLED) { + break; + } + else if (result != CallableProcessingInterceptor.RESULT_NONE) { return result; } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptor.java b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptor.java index 10a3099cd43..f727b8f8c83 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptor.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptor.java @@ -35,7 +35,7 @@ import org.springframework.web.context.request.NativeWebRequest; * the Exception instance as the concurrent result. Such exceptions will then * be processed through the {@code HandlerExceptionResolver} mechanism. * - *

The {@link #afterTimeout(NativeWebRequest, Callable) afterTimeout} method + *

The {@link #handleTimeout(NativeWebRequest, Callable) afterTimeout} method * can select a value to be used to resume processing. * * @author Rossen Stoyanchev @@ -43,7 +43,10 @@ import org.springframework.web.context.request.NativeWebRequest; */ public interface CallableProcessingInterceptor { - public static final Object RESULT_NONE = new Object(); + static final Object RESULT_NONE = new Object(); + + static final Object RESPONSE_HANDLED = new Object(); + /** * Invoked after the start of concurrent handling in the async @@ -79,11 +82,11 @@ public interface CallableProcessingInterceptor { * @param request the current request * @param task the task for the current async request * @return a concurrent result value; if the value is anything other than - * {@link #RESULT_NONE}, concurrent processing is resumed and subsequent - * interceptors are not invoked + * {@link #RESULT_NONE} or {@link #RESPONSE_HANDLED}, concurrent processing + * is resumed and subsequent interceptors are not invoked * @throws Exception in case of errors */ - Object afterTimeout(NativeWebRequest request, Callable task) throws Exception; + Object handleTimeout(NativeWebRequest request, Callable task) throws Exception; /** * Invoked from a container thread when async processing completes for any diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptorAdapter.java b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptorAdapter.java index 6f305edd732..2d27f4711c3 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptorAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableProcessingInterceptorAdapter.java @@ -44,7 +44,7 @@ public abstract class CallableProcessingInterceptorAdapter implements CallablePr * This implementation always returns * {@link CallableProcessingInterceptor#RESULT_NONE RESULT_NONE}. */ - public Object afterTimeout(NativeWebRequest request, Callable task) throws Exception { + public Object handleTimeout(NativeWebRequest request, Callable task) throws Exception { return RESULT_NONE; } 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 1d63644e24d..5166ce192ec 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 @@ -183,18 +183,19 @@ public final class DeferredResult { return new DeferredResultProcessingInterceptorAdapter() { @Override - public void afterTimeout(NativeWebRequest request, DeferredResult deferredResult) { + public boolean handleTimeout(NativeWebRequest request, DeferredResult deferredResult) { if (timeoutCallback != null) { timeoutCallback.run(); } if (DeferredResult.this.timeoutResult != RESULT_NONE) { setResultInternal(timeoutResult); } + return true; } @Override public void afterCompletion(NativeWebRequest request, DeferredResult deferredResult) { - synchronized (this) { + synchronized (DeferredResult.this) { expired = true; } if (completionCallback != null) { 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 336eb6ddcc7..09763330c0f 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 @@ -60,11 +60,13 @@ class DeferredResultInterceptorChain { } public void triggerAfterTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception { - for (int i = this.preProcessingIndex; i >= 0; i--) { + for (DeferredResultProcessingInterceptor interceptor : this.interceptors) { if (deferredResult.isSetOrExpired()) { return; } - this.interceptors.get(i).afterTimeout(request, deferredResult); + if (!interceptor.handleTimeout(request, deferredResult)){ + break; + } } } 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 f91e37ef285..281e478968b 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 @@ -32,7 +32,7 @@ import org.springframework.web.context.request.NativeWebRequest; * the Exception instance as the concurrent result. Such exceptions will then * be processed through the {@code HandlerExceptionResolver} mechanism. * - *

The {@link #afterTimeout(NativeWebRequest, DeferredResult) afterTimeout} + *

The {@link #handleTimeout(NativeWebRequest, DeferredResult) afterTimeout} * method can set the {@code DeferredResult} in order to resume processing. * * @author Rossen Stoyanchev @@ -81,9 +81,11 @@ public interface DeferredResultProcessingInterceptor { * @param deferredResult the DeferredResult for the current request; if the * {@code DeferredResult} is set, then concurrent processing is resumed and * subsequent interceptors are not invoked + * @return {@code true} if processing should continue, or {@code false} if + * other interceptors should not be invoked * @throws Exception in case of errors */ - void afterTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception; + boolean handleTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception; /** * Invoked from a container thread when an async request completed for any diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptorAdapter.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptorAdapter.java index 6120cc67afe..a6fa80d12d4 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptorAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultProcessingInterceptorAdapter.java @@ -40,9 +40,10 @@ public abstract class DeferredResultProcessingInterceptorAdapter implements Defe } /** - * This implementation is empty. + * This implementation returns {@code true} by default. */ - public void afterTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception { + public boolean handleTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception { + return true; } /** diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java b/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java index 6720606c5ee..a79e629185e 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/MvcAsyncTask.java @@ -156,7 +156,7 @@ public class MvcAsyncTask { return new CallableProcessingInterceptorAdapter() { @Override - public Object afterTimeout(NativeWebRequest request, Callable task) throws Exception { + public Object handleTimeout(NativeWebRequest request, Callable task) throws Exception { return (timeoutCallback != null) ? timeoutCallback.call() : CallableProcessingInterceptor.RESULT_NONE; } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutCallableProcessingInterceptor.java b/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutCallableProcessingInterceptor.java new file mode 100644 index 00000000000..1c8016eefc6 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutCallableProcessingInterceptor.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.async; + +import java.util.concurrent.Callable; + +import javax.servlet.http.HttpServletResponse; + +import org.springframework.http.HttpStatus; +import org.springframework.web.context.request.NativeWebRequest; + +/** + * Sends a 503 (SERVICE_UNAVAILABLE) in case of a timeout if the response is not + * already committed. Registered at the end, after all other interceptors and + * therefore invoked only if no other interceptor handles the timeout. + * + *

Note that according to RFC 2616, a 503 without a 'Retry-After' header is + * interpreted as a 500 error and the client should not retry. Applications + * can install their own interceptor to handle a timeout and add a 'Retry-After' + * header if necessary. + * + * @author Rossen Stoyanchev + * @since 3.2 + */ +public class TimeoutCallableProcessingInterceptor extends CallableProcessingInterceptorAdapter { + + @Override + public Object handleTimeout(NativeWebRequest request, Callable task) throws Exception { + HttpServletResponse servletResponse = request.getNativeResponse(HttpServletResponse.class); + if (!servletResponse.isCommitted()) { + servletResponse.sendError(HttpStatus.SERVICE_UNAVAILABLE.value()); + } + return CallableProcessingInterceptor.RESPONSE_HANDLED; + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutDeferredResultProcessingInterceptor.java b/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutDeferredResultProcessingInterceptor.java new file mode 100644 index 00000000000..62c0a5fbb7a --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/TimeoutDeferredResultProcessingInterceptor.java @@ -0,0 +1,47 @@ +/* + * 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.async; + +import javax.servlet.http.HttpServletResponse; + +import org.springframework.http.HttpStatus; +import org.springframework.web.context.request.NativeWebRequest; + +/** + * Sends a 503 (SERVICE_UNAVAILABLE) in case of a timeout if the response is not + * already committed. Registered at the end, after all other interceptors and + * therefore invoked only if no other interceptor handles the timeout. + * + *

Note that according to RFC 2616, a 503 without a 'Retry-After' header is + * interpreted as a 500 error and the client should not retry. Applications + * can install their own interceptor to handle a timeout and add a 'Retry-After' + * header if necessary. + * + * @author Rossen Stoyanchev + * @since 3.2 + */ +public class TimeoutDeferredResultProcessingInterceptor extends DeferredResultProcessingInterceptorAdapter { + + @Override + public boolean handleTimeout(NativeWebRequest request, DeferredResult deferredResult) throws Exception { + HttpServletResponse servletResponse = request.getNativeResponse(HttpServletResponse.class); + if (!servletResponse.isCommitted()) { + servletResponse.sendError(HttpStatus.SERVICE_UNAVAILABLE.value()); + } + return false; + } + +} 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 0e324b872dd..add36ba7215 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 @@ -63,6 +63,12 @@ public final class WebAsyncManager { private static final UrlPathHelper urlPathHelper = new UrlPathHelper(); + private static final CallableProcessingInterceptor timeoutCallableInterceptor = + new TimeoutCallableProcessingInterceptor(); + + private static final DeferredResultProcessingInterceptor timeoutDeferredResultInterceptor = + new TimeoutDeferredResultProcessingInterceptor(); + private AsyncWebRequest asyncWebRequest; @@ -266,8 +272,6 @@ public final class WebAsyncManager { Assert.notNull(mvcAsyncTask, "MvcAsyncTask must not be null"); Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null"); - final Callable callable = mvcAsyncTask.getCallable(); - Long timeout = mvcAsyncTask.getTimeout(); if (timeout != null) { this.asyncWebRequest.setTimeout(timeout); @@ -281,7 +285,9 @@ public final class WebAsyncManager { List interceptors = new ArrayList(); interceptors.add(mvcAsyncTask.getInterceptor()); interceptors.addAll(this.callableInterceptors.values()); + interceptors.add(timeoutCallableInterceptor); + final Callable callable = mvcAsyncTask.getCallable(); final CallableInterceptorChain interceptorChain = new CallableInterceptorChain(interceptors); this.asyncWebRequest.addTimeoutHandler(new Runnable() { @@ -368,6 +374,7 @@ public final class WebAsyncManager { List interceptors = new ArrayList(); interceptors.add(deferredResult.getInterceptor()); interceptors.addAll(this.deferredResultInterceptors.values()); + interceptors.add(timeoutDeferredResultInterceptor); final DeferredResultInterceptorChain interceptorChain = new DeferredResultInterceptorChain(interceptors); 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 9820a98df8e..bcb780ae9a3 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 @@ -113,7 +113,7 @@ public class DeferredResultTests { } }); - result.getInterceptor().afterTimeout(null, null); + result.getInterceptor().handleTimeout(null, null); assertEquals("timeout event", sb.toString()); assertFalse("Should not be able to set result a second time", result.setResult("hello")); 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 9fd7c621e94..aac5f5f7ddd 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 @@ -56,12 +56,14 @@ public class WebAsyncManagerTimeoutTests { private MockHttpServletRequest servletRequest; + private MockHttpServletResponse servletResponse; @Before public void setUp() { this.servletRequest = new MockHttpServletRequest("GET", "/test"); this.servletRequest.setAsyncSupported(true); - this.asyncWebRequest = new StandardServletAsyncWebRequest(servletRequest, new MockHttpServletResponse()); + this.servletResponse = new MockHttpServletResponse(); + this.asyncWebRequest = new StandardServletAsyncWebRequest(servletRequest, servletResponse); AsyncTaskExecutor executor = createMock(AsyncTaskExecutor.class); expect(executor.submit((Runnable) notNull())).andReturn(null); @@ -78,7 +80,7 @@ public class WebAsyncManagerTimeoutTests { StubCallable callable = new StubCallable(); CallableProcessingInterceptor interceptor = createStrictMock(CallableProcessingInterceptor.class); - expect(interceptor.afterTimeout(this.asyncWebRequest, callable)).andReturn(RESULT_NONE); + expect(interceptor.handleTimeout(this.asyncWebRequest, callable)).andReturn(RESULT_NONE); interceptor.afterCompletion(this.asyncWebRequest, callable); replay(interceptor); @@ -90,6 +92,7 @@ public class WebAsyncManagerTimeoutTests { assertFalse(this.asyncManager.hasConcurrentResult()); assertEquals(DispatcherType.REQUEST, this.servletRequest.getDispatcherType()); + assertEquals(503, this.servletResponse.getStatus()); verify(interceptor); } @@ -120,7 +123,7 @@ public class WebAsyncManagerTimeoutTests { StubCallable callable = new StubCallable(); CallableProcessingInterceptor interceptor = createStrictMock(CallableProcessingInterceptor.class); - expect(interceptor.afterTimeout(this.asyncWebRequest, callable)).andReturn(22); + expect(interceptor.handleTimeout(this.asyncWebRequest, callable)).andReturn(22); replay(interceptor); this.asyncManager.registerCallableInterceptor("timeoutInterceptor", interceptor); @@ -142,7 +145,7 @@ public class WebAsyncManagerTimeoutTests { Exception exception = new Exception(); CallableProcessingInterceptor interceptor = createStrictMock(CallableProcessingInterceptor.class); - expect(interceptor.afterTimeout(this.asyncWebRequest, callable)).andThrow(exception); + expect(interceptor.handleTimeout(this.asyncWebRequest, callable)).andThrow(exception); replay(interceptor); this.asyncManager.registerCallableInterceptor("timeoutInterceptor", interceptor); @@ -164,7 +167,7 @@ public class WebAsyncManagerTimeoutTests { DeferredResultProcessingInterceptor interceptor = createStrictMock(DeferredResultProcessingInterceptor.class); interceptor.preProcess(this.asyncWebRequest, deferredResult); - interceptor.afterTimeout(this.asyncWebRequest, deferredResult); + expect(interceptor.handleTimeout(this.asyncWebRequest, deferredResult)).andReturn(true); interceptor.afterCompletion(this.asyncWebRequest, deferredResult); replay(interceptor); @@ -176,6 +179,7 @@ public class WebAsyncManagerTimeoutTests { assertFalse(this.asyncManager.hasConcurrentResult()); assertEquals(DispatcherType.REQUEST, this.servletRequest.getDispatcherType()); + assertEquals(503, this.servletResponse.getStatus()); verify(interceptor); } @@ -220,8 +224,9 @@ public class WebAsyncManagerTimeoutTests { DeferredResult deferredResult = new DeferredResult(); DeferredResultProcessingInterceptor interceptor = new DeferredResultProcessingInterceptorAdapter() { - public void afterTimeout(NativeWebRequest request, DeferredResult result) throws Exception { + public boolean handleTimeout(NativeWebRequest request, DeferredResult result) throws Exception { result.setErrorResult(23); + return true; } }; @@ -243,7 +248,7 @@ public class WebAsyncManagerTimeoutTests { final Exception exception = new Exception(); DeferredResultProcessingInterceptor interceptor = new DeferredResultProcessingInterceptorAdapter() { - public void afterTimeout(NativeWebRequest request, DeferredResult result) throws Exception { + public boolean handleTimeout(NativeWebRequest request, DeferredResult result) throws Exception { throw exception; } };