From 5d04ef4c4acbd3ce35161ce3c500163ee8729abe Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sun, 28 Oct 2012 07:40:50 -0400 Subject: [PATCH] Support selective filtering of error dispatches OncePerRequestFilter now allows sub-classes to choose whether they should ever get involved in processing an error dispatch. Issue: SPR-9895 --- .../support/OpenSessionInViewFilter.java | 16 ++- .../support/OpenSessionInViewFilter.java | 15 ++- .../OpenEntityManagerInViewFilter.java | 15 ++- .../filter/AbstractRequestLoggingFilter.java | 7 +- .../web/filter/OncePerRequestFilter.java | 103 ++++++++++++++---- .../web/filter/RequestContextFilter.java | 14 ++- .../web/filter/ShallowEtagHeaderFilter.java | 7 +- .../filter/CharacterEncodingFilterTests.java | 6 + 8 files changed, 137 insertions(+), 46 deletions(-) diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewFilter.java b/spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewFilter.java index 0efc636ebf..f9cc4c3f8d 100644 --- a/spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewFilter.java +++ b/spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewFilter.java @@ -178,6 +178,15 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { return false; } + /** + * Returns "false" so that the filter may provide a Hibernate + * {@code Session} to each error dispatches. + */ + @Override + protected boolean shouldNotFilterErrorDispatch() { + return false; + } + @Override protected void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -187,7 +196,6 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { boolean participate = false; WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean isFirstRequest = !asyncManager.hasConcurrentResult(); String key = getAlreadyFilteredAttributeName(); if (isSingleSession()) { @@ -197,6 +205,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { participate = true; } else { + boolean isFirstRequest = !isAsyncDispatch(request); if (isFirstRequest || !applySessionBindingInterceptor(asyncManager, key)) { logger.debug("Opening single Hibernate Session in OpenSessionInViewFilter"); Session session = getSession(sessionFactory); @@ -210,8 +219,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { } else { // deferred close mode - Assert.state(!asyncManager.isConcurrentHandlingStarted(), - "Deferred close mode is not supported on async dispatches"); + Assert.state(!isAsyncStarted(request), "Deferred close mode is not supported on async dispatches"); if (SessionFactoryUtils.isDeferredCloseActive(sessionFactory)) { // Do not modify deferred close: just set the participate flag. participate = true; @@ -230,7 +238,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { // single session mode SessionHolder sessionHolder = (SessionHolder) TransactionSynchronizationManager.unbindResource(sessionFactory); - if (!asyncManager.isConcurrentHandlingStarted()) { + if (!isAsyncStarted(request)) { logger.debug("Closing single Hibernate Session in OpenSessionInViewFilter"); closeSession(sessionHolder.getSession(), sessionFactory); } diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewFilter.java b/spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewFilter.java index 5fdef4a18d..3c85540e20 100644 --- a/spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewFilter.java +++ b/spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewFilter.java @@ -102,7 +102,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { } /** - * The default value is "false" so that the filter may re-bind the opened + * Returns "false" so that the filter may re-bind the opened Hibernate * {@code Session} to each asynchronously dispatched thread and postpone * closing it until the very last asynchronous dispatch. */ @@ -111,6 +111,15 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { return false; } + /** + * Returns "false" so that the filter may provide a Hibernate + * {@code Session} to each error dispatches. + */ + @Override + protected boolean shouldNotFilterErrorDispatch() { + return false; + } + @Override protected void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -120,7 +129,6 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { boolean participate = false; WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean isFirstRequest = !asyncManager.hasConcurrentResult(); String key = getAlreadyFilteredAttributeName(); if (TransactionSynchronizationManager.hasResource(sessionFactory)) { @@ -128,6 +136,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { participate = true; } else { + boolean isFirstRequest = !isAsyncDispatch(request); if (isFirstRequest || !applySessionBindingInterceptor(asyncManager, key)) { logger.debug("Opening Hibernate Session in OpenSessionInViewFilter"); Session session = openSession(sessionFactory); @@ -147,7 +156,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter { if (!participate) { SessionHolder sessionHolder = (SessionHolder) TransactionSynchronizationManager.unbindResource(sessionFactory); - if (!asyncManager.isConcurrentHandlingStarted()) { + if (!isAsyncStarted(request)) { logger.debug("Closing Hibernate Session in OpenSessionInViewFilter"); SessionFactoryUtils.closeSession(sessionHolder.getSession()); } diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewFilter.java b/spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewFilter.java index 10752757f7..fbce1e10bb 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewFilter.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewFilter.java @@ -126,7 +126,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter { } /** - * The default value is "false" so that the filter may re-bind the opened + * Returns "false" so that the filter may re-bind the opened * {@code EntityManager} to each asynchronously dispatched thread and postpone * closing it until the very last asynchronous dispatch. */ @@ -135,6 +135,15 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter { return false; } + /** + * Returns "false" so that the filter may provide an {@code EntityManager} + * to each error dispatches. + */ + @Override + protected boolean shouldNotFilterErrorDispatch() { + return false; + } + @Override protected void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -144,7 +153,6 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter { boolean participate = false; WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean isFirstRequest = !asyncManager.hasConcurrentResult(); String key = getAlreadyFilteredAttributeName(); if (TransactionSynchronizationManager.hasResource(emf)) { @@ -152,6 +160,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter { participate = true; } else { + boolean isFirstRequest = !isAsyncDispatch(request); if (isFirstRequest || !applyEntityManagerBindingInterceptor(asyncManager, key)) { logger.debug("Opening JPA EntityManager in OpenEntityManagerInViewFilter"); try { @@ -175,7 +184,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter { if (!participate) { EntityManagerHolder emHolder = (EntityManagerHolder) TransactionSynchronizationManager.unbindResource(emf); - if (!asyncManager.isConcurrentHandlingStarted()) { + if (!isAsyncStarted(request)) { logger.debug("Closing JPA EntityManager in OpenEntityManagerInViewFilter"); EntityManagerFactoryUtils.closeEntityManager(emHolder.getEntityManager()); } diff --git a/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java b/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java index 2cdf4abe83..875490abf8 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java @@ -32,8 +32,6 @@ import javax.servlet.http.HttpSession; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.web.context.request.async.WebAsyncManager; -import org.springframework.web.context.request.async.WebAsyncUtils; import org.springframework.web.util.WebUtils; /** @@ -200,8 +198,7 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean isFirstRequest = !asyncManager.hasConcurrentResult(); + boolean isFirstRequest = !isAsyncDispatch(request); if (isIncludePayload()) { if (isFirstRequest) { @@ -216,7 +213,7 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter filterChain.doFilter(request, response); } finally { - if (!asyncManager.isConcurrentHandlingStarted()) { + if (!isAsyncStarted(request)) { afterRequest(request, getAfterMessage(request)); } } diff --git a/spring-web/src/main/java/org/springframework/web/filter/OncePerRequestFilter.java b/spring-web/src/main/java/org/springframework/web/filter/OncePerRequestFilter.java index 299cb0c99b..e8a9384717 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/OncePerRequestFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/OncePerRequestFilter.java @@ -27,21 +27,39 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.web.context.request.async.WebAsyncManager; import org.springframework.web.context.request.async.WebAsyncUtils; +import org.springframework.web.util.WebUtils; /** - * Filter base class that guarantees to be just executed once per request, - * on any servlet container. It provides a {@link #doFilterInternal} + * Filter base class that aims to guarantee a single execution per request + * dispatch, on any servlet container. It provides a {@link #doFilterInternal} * method with HttpServletRequest and HttpServletResponse arguments. * - *

In an async scenario a filter may be invoked again in additional threads - * as part of an {@linkplain javax.servlet.DispatcherType.ASYNC ASYNC} dispatch. - * Sub-classes may decide whether to be invoked once per request or once per - * request thread for as long as the same request is being processed. - * See {@link #shouldNotFilterAsyncDispatch()}. + *

As of Servlet 3.0, a filter may be invoked as part of a + * {@link javax.servlet.DispatcherType.REQUEST REQUEST} or + * {@link javax.servlet.DispatcherType.ASYNC ASYNC} dispatches that occur in + * separate threads. A filter can be configured in {@code web.xml} whether it + * should be involved in async dispatches. However, in some cases servlet + * containers assume different default configuration. Therefore sub-classes can + * override the method {@link #shouldNotFilterAsyncDispatch()} to declare + * statically if they shouuld indeed be invoked, once, during both types + * of dispatches in order to provide thread initialization, logging, security, + * and so on. This mechanism complements and does not replace the need to + * configure a filter in {@code web.xml} with dispatcher types. * - *

The {@link #getAlreadyFilteredAttributeName} method determines how - * to identify that a request is already filtered. The default implementation - * is based on the configured name of the concrete filter instance. + *

Sub-classes may use {@link #isAsyncDispatch(HttpServletRequest)} to + * determine when a filter is invoked as part of an async dispatch, and + * use {@link #isAsyncStarted(HttpServletRequest)} to determine when the + * request has been placed in async mode and therefore the current dispatch + * won't be the last one. + * + *

Yet another dispatch type that also occurs in its own thread is + * {@link javax.servlet.DispatcherType.ERROR ERROR}. Sub-classes can override + * {@link #shouldNotFilterErrorDispatch()} if they wish to declare statically + * if they should be invoked once during error dispatches. + * + *

The {@link #getAlreadyFilteredAttributeName} method determines how to + * identify that a request is already filtered. The default implementation is + * based on the configured name of the concrete filter instance. * * @author Juergen Hoeller * @author Rossen Stoyanchev @@ -74,13 +92,10 @@ public abstract class OncePerRequestFilter extends GenericFilterBean { HttpServletRequest httpRequest = (HttpServletRequest) request; HttpServletResponse httpResponse = (HttpServletResponse) response; - WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean processAsyncDispatch = asyncManager.hasConcurrentResult() && !shouldNotFilterAsyncDispatch(); - String alreadyFilteredAttributeName = getAlreadyFilteredAttributeName(); boolean hasAlreadyFilteredAttribute = request.getAttribute(alreadyFilteredAttributeName) != null; - if ((hasAlreadyFilteredAttribute && (!processAsyncDispatch)) || shouldNotFilter(httpRequest)) { + if (hasAlreadyFilteredAttribute || skipDispatch(httpRequest) || shouldNotFilter(httpRequest)) { // Proceed without invoking this filter... filterChain.doFilter(request, response); @@ -92,14 +107,46 @@ public abstract class OncePerRequestFilter extends GenericFilterBean { doFilterInternal(httpRequest, httpResponse, filterChain); } finally { - if (!asyncManager.isConcurrentHandlingStarted()) { - // Remove the "already filtered" request attribute for this request. - request.removeAttribute(alreadyFilteredAttributeName); - } + // Remove the "already filtered" request attribute for this request. + request.removeAttribute(alreadyFilteredAttributeName); } } } + private boolean skipDispatch(HttpServletRequest request) { + if (isAsyncDispatch(request) && shouldNotFilterAsyncDispatch()) { + return true; + } + if ((request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE) != null) && shouldNotFilterErrorDispatch()) { + return true; + } + return false; + } + + /** + * The dispatcher type {@code javax.servlet.DispatcherType.ASYNC} introduced + * in Servlet 3.0 means a filter can be invoked in more than one thread over + * the course of a single request. This method returns {@code true} if the + * filter is currently executing within an asynchronous dispatch. + * + * @param request the current request + * @see WebAsyncManager#hasConcurrentResult() + */ + protected boolean isAsyncDispatch(HttpServletRequest request) { + return WebAsyncUtils.getAsyncManager(request).hasConcurrentResult(); + } + + /** + * Whether request processing is in asynchronous mode meaning that the + * response will not be committed after the current thread is exited. + * + * @param request the current request + * @see WebAsyncManager#isConcurrentHandlingStarted() + */ + protected boolean isAsyncStarted(HttpServletRequest request) { + return WebAsyncUtils.getAsyncManager(request).isConcurrentHandlingStarted(); + } + /** * Return the name of the request attribute that identifies that a request * is already filtered. @@ -130,18 +177,18 @@ public abstract class OncePerRequestFilter extends GenericFilterBean { } /** - * Whether to filter async dispatches, which occur in a different thread. * The dispatcher type {@code javax.servlet.DispatcherType.ASYNC} introduced - * in Servlet 3.0 means a filter can be invoked in more than one thread (and - * exited) over the course of a single request. Some filters only need to - * filter the initial thread (e.g. request wrapping) while others may need + * in Servlet 3.0 means a filter can be invoked in more than one thread + * over the course of a single request. Some filters only need to filter + * the initial thread (e.g. request wrapping) while others may need * to be invoked at least once in each additional thread for example for * setting up thread locals or to perform final processing at the very end. *

Note that although a filter can be mapped to handle specific dispatcher * types via {@code web.xml} or in Java through the {@code ServletContext}, * servlet containers may enforce different defaults with regards to * dispatcher types. This flag enforces the design intent of the filter. - *

The default setting is "true", which means the filter will not be + * + *

The default return value is "true", which means the filter will not be * invoked during subsequent async dispatches. If "false", the filter will * be invoked during async dispatches with the same guarantees of being * invoked only once during a request within a single thread. @@ -150,6 +197,16 @@ public abstract class OncePerRequestFilter extends GenericFilterBean { return true; } + /** + * Whether to filter error dispatches such as when the servlet container + * processes and error mapped in {@code web.xml}. The default return value + * is "true", which means the filter will not be invoked in case of an error + * dispatch. + */ + protected boolean shouldNotFilterErrorDispatch() { + return true; + } + /** * Same contract as for doFilter, but guaranteed to be * just invoked once per request within a single request thread. diff --git a/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java b/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java index 2777604d1b..f5fc914dac 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java @@ -69,16 +69,24 @@ public class RequestContextFilter extends OncePerRequestFilter { this.threadContextInheritable = threadContextInheritable; } - /** - * The default value is "false" in which case the filter will set up the request - * context in each asynchronously dispatched thread. + * Returns "false" so that the filter may set up the request context in each + * asynchronously dispatched thread. */ @Override protected boolean shouldNotFilterAsyncDispatch() { return false; } + /** + * Returns "false" so that the filter may set up the request context in an + * error dispatch. + */ + @Override + protected boolean shouldNotFilterErrorDispatch() { + return false; + } + @Override protected void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) diff --git a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java index 8651043708..ca00387c96 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java @@ -68,16 +68,13 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request); - boolean isFirstRequest = !asyncManager.hasConcurrentResult(); - - if (isFirstRequest) { + if (!isAsyncDispatch(request)) { response = new ShallowEtagResponseWrapper(response); } filterChain.doFilter(request, response); - if (!asyncManager.isConcurrentHandlingStarted()) { + if (!isAsyncStarted(request)) { updateResponse(request, response); } } diff --git a/spring-web/src/test/java/org/springframework/web/filter/CharacterEncodingFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/CharacterEncodingFilterTests.java index a5fa128dd3..02e90dec80 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/CharacterEncodingFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/CharacterEncodingFilterTests.java @@ -34,6 +34,7 @@ import org.springframework.mock.web.MockFilterConfig; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.web.context.request.async.WebAsyncUtils; +import org.springframework.web.util.WebUtils; /** * @author Rick Evans @@ -50,6 +51,7 @@ public class CharacterEncodingFilterTests extends TestCase { HttpServletRequest request = createMock(HttpServletRequest.class); addAsyncManagerExpectations(request); request.setCharacterEncoding(ENCODING); + expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null); expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null); request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE); request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX); @@ -79,6 +81,7 @@ public class CharacterEncodingFilterTests extends TestCase { addAsyncManagerExpectations(request); expect(request.getCharacterEncoding()).andReturn(null); request.setCharacterEncoding(ENCODING); + expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null); expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null); request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE); request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX); @@ -104,6 +107,7 @@ public class CharacterEncodingFilterTests extends TestCase { HttpServletRequest request = createMock(HttpServletRequest.class); addAsyncManagerExpectations(request); expect(request.getCharacterEncoding()).andReturn(ENCODING); + expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null); expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null); request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE); request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX); @@ -129,6 +133,7 @@ public class CharacterEncodingFilterTests extends TestCase { addAsyncManagerExpectations(request); expect(request.getCharacterEncoding()).andReturn(null); request.setCharacterEncoding(ENCODING); + expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null); expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null); request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE); request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX); @@ -155,6 +160,7 @@ public class CharacterEncodingFilterTests extends TestCase { addAsyncManagerExpectations(request); expect(request.getCharacterEncoding()).andReturn(null); request.setCharacterEncoding(ENCODING); + expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null); expect(request.getAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null); request.setAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE); request.removeAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);