Populate RequestAttributes before invoking Filters in MockMvc

When using the Spring TestContext Framework (TCF) to load a
WebApplicationContext and the Spring MVC Test Framework (MockMvc) to
test a controller, two instances of MockHttpServletRequest will be
created. Due to an ordering issue with regard to population of the
RequestAttributes, it is therefore possible that a filter accesses the
mocked request managed by the TCF, while the controller accesses the
mocked request managed by MockMvc, and this leads to test failures if
the controller expects data from the filter to be present in the
request.

This commit fixes this bug by ensuring that the RequestAttributes
backed by the mocked request managed by MockMvc are stored in the
RequestContextHolder before any filters are invoked by MockMvc.

Issue: SPR-13217

Author: sbrannen

spring-test/src/main/java/org/springframework/test/web/servlet/MockMvc.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -26,6 +26,8 @@
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.util.Assert;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;
 
 /**
  * <strong>Main entry point for server-side Spring MVC test support.</strong>
@@ -48,6 +50,7 @@
  *
  * @author Rossen Stoyanchev
  * @author Rob Winch
+ * @author Sam Brannen
  * @since 3.2
  */
 public final class MockMvc {
@@ -140,6 +143,10 @@ public ResultActions perform(RequestBuilder requestBuilder) throws Exception {
 		final MvcResult mvcResult = new DefaultMvcResult(request, response);
 		request.setAttribute(MVC_RESULT_ATTRIBUTE, mvcResult);
 
+		// [SPR-13217] Simulate RequestContextFilter to ensure that RequestAttributes are
+		// populated before filters are invoked.
+		RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request, response));
+
 		MockFilterChain filterChain = new MockFilterChain(this.servlet, this.filters);
 		filterChain.doFilter(request, response);
 

spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/RequestContextHolderTests.java

@@ -22,7 +22,6 @@
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -57,7 +56,8 @@
 import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
 
 /**
- * Tests for SPR-10025 (access to request attributes via RequestContextHolder)
+ * Tests for SPR-10025 (access to request attributes via RequestContextHolder),
+ * SPR-13217 (Populate RequestAttributes before invoking Filters in MockMvc),
  * and SPR-13211 (re-use of mock request from the TestContext framework).
  *
  * @author Rossen Stoyanchev
@@ -72,9 +72,8 @@ public class RequestContextHolderTests {
 	private static final String FROM_TCF_MOCK = "fromTestContextFrameworkMock";
 	private static final String FROM_MVC_TEST_DEFAULT = "fromSpringMvcTestDefault";
 	private static final String FROM_MVC_TEST_MOCK = "fromSpringMvcTestMock";
-	private static final String FROM_FILTER = "fromFilter";
-
-	private static final String ENIGMA = "puzzle";
+	private static final String FROM_REQUEST_FILTER = "fromRequestFilter";
+	private static final String FROM_REQUEST_ATTRIBUTES_FILTER = "fromRequestAttributesFilter";
 
 	@Autowired
 	private WebApplicationContext wac;
@@ -91,41 +90,44 @@ public class RequestContextHolderTests {
 	@Autowired
 	private SessionScopedService sessionScopedService;
 
+	@Autowired
+	private FilterWithSessionScopedService filterWithSessionScopedService;
+
 	private MockMvc mockMvc;
 
 
 	@Before
 	public void setup() {
-		this.mockRequest.setAttribute(FROM_TCF_MOCK, ENIGMA);
+		this.mockRequest.setAttribute(FROM_TCF_MOCK, FROM_TCF_MOCK);
 
 		this.mockMvc = webAppContextSetup(this.wac)
-				.addFilter(new AbcFilter())
-				.defaultRequest(get("/").requestAttr(FROM_MVC_TEST_DEFAULT, ENIGMA))
+				.addFilters(new RequestFilter(), new RequestAttributesFilter(), this.filterWithSessionScopedService)
+				.defaultRequest(get("/").requestAttr(FROM_MVC_TEST_DEFAULT, FROM_MVC_TEST_DEFAULT))
 				.alwaysExpect(status().isOk())
 				.build();
 	}
 
 	@Test
 	public void singletonController() throws Exception {
-		this.mockMvc.perform(get("/singletonController").requestAttr(FROM_MVC_TEST_MOCK, ENIGMA));
+		this.mockMvc.perform(get("/singletonController").requestAttr(FROM_MVC_TEST_MOCK, FROM_MVC_TEST_MOCK));
 	}
 
 	@Test
 	public void requestScopedController() throws Exception {
 		assertTrue("request-scoped controller must be a CGLIB proxy", AopUtils.isCglibProxy(this.requestScopedController));
-		this.mockMvc.perform(get("/requestScopedController").requestAttr(FROM_MVC_TEST_MOCK, ENIGMA));
+		this.mockMvc.perform(get("/requestScopedController").requestAttr(FROM_MVC_TEST_MOCK, FROM_MVC_TEST_MOCK));
 	}
 
 	@Test
 	public void requestScopedService() throws Exception {
 		assertTrue("request-scoped service must be a CGLIB proxy", AopUtils.isCglibProxy(this.requestScopedService));
-		this.mockMvc.perform(get("/requestScopedService").requestAttr(FROM_MVC_TEST_MOCK, ENIGMA));
+		this.mockMvc.perform(get("/requestScopedService").requestAttr(FROM_MVC_TEST_MOCK, FROM_MVC_TEST_MOCK));
 	}
 
 	@Test
 	public void sessionScopedService() throws Exception {
 		assertTrue("session-scoped service must be a CGLIB proxy", AopUtils.isCglibProxy(this.sessionScopedService));
-		this.mockMvc.perform(get("/sessionScopedService").requestAttr(FROM_MVC_TEST_MOCK, ENIGMA));
+		this.mockMvc.perform(get("/sessionScopedService").requestAttr(FROM_MVC_TEST_MOCK, FROM_MVC_TEST_MOCK));
 	}
 
 
@@ -167,6 +169,11 @@ public SessionScopedService sessionScopedService() {
 		public ControllerWithSessionScopedService controllerWithSessionScopedService() {
 			return new ControllerWithSessionScopedService();
 		}
+
+		@Bean
+		public FilterWithSessionScopedService filterWithSessionScopedService() {
+			return new FilterWithSessionScopedService();
+		}
 	}
 
 	@RestController
@@ -182,7 +189,7 @@ public void handle() {
 	private static class RequestScopedController {
 
 		@Autowired
-		private HttpServletRequest request;
+		private ServletRequest request;
 
 
 		@RequestMapping("/requestScopedController")
@@ -195,7 +202,7 @@ public void handle() {
 	private static class RequestScopedService {
 
 		@Autowired
-		private HttpServletRequest request;
+		private ServletRequest request;
 
 
 		void process() {
@@ -206,7 +213,7 @@ void process() {
 	private static class SessionScopedService {
 
 		@Autowired
-		private HttpServletRequest request;
+		private ServletRequest request;
 
 
 		void process() {
@@ -242,11 +249,35 @@ public void handle() {
 		}
 	}
 
-	private static class AbcFilter extends GenericFilterBean {
+	private static class FilterWithSessionScopedService extends GenericFilterBean {
+
+		@Autowired
+		private SessionScopedService service;
+
+
+		@Override
+		public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
+			this.service.process();
+			assertRequestAttributes(request);
+			assertRequestAttributes();
+			chain.doFilter(request, response);
+		}
+	}
+
+	private static class RequestFilter extends GenericFilterBean {
+
+		@Override
+		public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
+			request.setAttribute(FROM_REQUEST_FILTER, FROM_REQUEST_FILTER);
+			chain.doFilter(request, response);
+		}
+	}
+
+	private static class RequestAttributesFilter extends GenericFilterBean {
 
 		@Override
 		public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-			request.setAttribute(FROM_FILTER, ENIGMA);
+			RequestContextHolder.getRequestAttributes().setAttribute(FROM_REQUEST_ATTRIBUTES_FILTER, FROM_REQUEST_ATTRIBUTES_FILTER, RequestAttributes.SCOPE_REQUEST);
 			chain.doFilter(request, response);
 		}
 	}
@@ -258,13 +289,13 @@ private static void assertRequestAttributes() {
 		assertRequestAttributes(((ServletRequestAttributes) requestAttributes).getRequest());
 	}
 
-	private static void assertRequestAttributes(HttpServletRequest request) {
-		// TODO [SPR-13211] Assert that FOO is ENIGMA, instead of NULL.
-		// assertThat(this.request.getAttribute(FOO), is(ENIGMA));
+	private static void assertRequestAttributes(ServletRequest request) {
+		// TODO [SPR-13211] Assert that FROM_TCF_MOCK is FROM_TCF_MOCK, instead of NULL.
 		assertThat(request.getAttribute(FROM_TCF_MOCK), is(nullValue()));
-		assertThat(request.getAttribute(FROM_MVC_TEST_DEFAULT), is(ENIGMA));
-		assertThat(request.getAttribute(FROM_MVC_TEST_MOCK), is(ENIGMA));
-		assertThat(request.getAttribute(FROM_FILTER), is(ENIGMA));
+		assertThat(request.getAttribute(FROM_MVC_TEST_DEFAULT), is(FROM_MVC_TEST_DEFAULT));
+		assertThat(request.getAttribute(FROM_MVC_TEST_MOCK), is(FROM_MVC_TEST_MOCK));
+		assertThat(request.getAttribute(FROM_REQUEST_FILTER), is(FROM_REQUEST_FILTER));
+		assertThat(request.getAttribute(FROM_REQUEST_ATTRIBUTES_FILTER), is(FROM_REQUEST_ATTRIBUTES_FILTER));
 	}
 
 }