Do not reuse mock requests in Spring MVC Test

SPR-13211 introduced support for reusing mock requests in Spring MVC
Test if the request was created by the the Spring TestContext
Framework. Unfortunately, that change makes it impossible for
MockMvc.perform() to be invoked multiple times within the same test
method without side effects. For example, session attributes and
request parameters are transparently and unexpectedly retained for
subsequent invocations of perform(), causing certain categories of
tests to fail.

This commit reverts the changes introduced in SPR-13211 and introduces
a new MockMvcReuseTests class to serve as regression tests within
Spring's test suite.

Issue: SPR-13260, SPR-13211

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

@@ -29,7 +29,6 @@ import java.util.Map.Entry;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.beans.Mergeable;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -39,7 +38,6 @@ import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockHttpSession;
-import org.springframework.test.context.web.ServletTestExecutionListener;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.util.Assert;
 import org.springframework.util.LinkedMultiValueMap;
@@ -48,9 +46,6 @@ import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.ValueConstants;
 import org.springframework.web.context.WebApplicationContext;
-import org.springframework.web.context.request.RequestAttributes;
-import org.springframework.web.context.request.RequestContextHolder;
-import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.context.support.WebApplicationContextUtils;
 import org.springframework.web.servlet.DispatcherServlet;
 import org.springframework.web.servlet.FlashMap;
@@ -645,31 +640,11 @@ public class MockHttpServletRequestBuilder
 	}
 
 	/**
-	 * Create a {@link MockHttpServletRequest}.
-	 * <p>If an instance of {@code MockHttpServletRequest} that was created
-	 * by the <em>Spring TestContext Framework</em> is available via the
-	 * {@link RequestAttributes} bound to the current thread in
-	 * {@link RequestContextHolder}, this method simply returns that instance.
-	 * <p>Otherwise, this method creates a new {@code MockHttpServletRequest}
-	 * based on the supplied {@link ServletContext}.
+	 * Create a new {@link MockHttpServletRequest} based on the supplied
+	 * {@code ServletContext}.
 	 * <p>Can be overridden in subclasses.
-	 * @see RequestContextHolder#getRequestAttributes()
-	 * @see ServletRequestAttributes
-	 * @see ServletTestExecutionListener#CREATED_BY_THE_TESTCONTEXT_FRAMEWORK
 	 */
 	protected MockHttpServletRequest createServletRequest(ServletContext servletContext) {
-		RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
-		if (requestAttributes instanceof ServletRequestAttributes) {
-			HttpServletRequest request = ((ServletRequestAttributes) requestAttributes).getRequest();
-			if (request instanceof MockHttpServletRequest) {
-				MockHttpServletRequest mockRequest = (MockHttpServletRequest) request;
-				Object createdByTcf = mockRequest.getAttribute(ServletTestExecutionListener.CREATED_BY_THE_TESTCONTEXT_FRAMEWORK);
-				if (Boolean.TRUE.equals(createdByTcf)) {
-					return mockRequest;
-				}
-			}
-		}
-
 		return new MockHttpServletRequest(servletContext);
 	}
 

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

@@ -109,7 +109,6 @@ public class MockMultipartHttpServletRequestBuilder extends MockHttpServletReque
 	 * Create a new {@link MockMultipartHttpServletRequest} based on the
 	 * supplied {@code ServletContext} and the {@code MockMultipartFiles}
 	 * added to this builder.
-	 * <p>Can be overridden in subclasses.
 	 */
 	@Override
 	protected final MockHttpServletRequest createServletRequest(ServletContext servletContext) {

spring-test/src/test/java/org/springframework/test/web/servlet/MockMvcReuseTests.java

+/*
+ * 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.
+ * 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.test.web.servlet;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.servlet.config.annotation.EnableWebMvc;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
+
+/**
+ * Integration tests that verify that {@link MockMvc} can be reused multiple
+ * times within the same test method without side effects between independent
+ * requests.
+ * <p>See <a href="https://jira.spring.io/browse/SPR-13260" target="_blank">SPR-13260</a>.
+ *
+ * @author Sam Brannen
+ * @author Rob Winch
+ * @since 4.2
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration
+@WebAppConfiguration
+public class MockMvcReuseTests {
+
+	private static final String HELLO = "hello";
+	private static final String ENIGMA = "enigma";
+	private static final String FOO = "foo";
+	private static final String BAR = "bar";
+
+	@Autowired
+	private WebApplicationContext wac;
+
+	private MockMvc mvc;
+
+
+	@Before
+	public void setUp() {
+		this.mvc = webAppContextSetup(this.wac).build();
+	}
+
+	@Test
+	public void sessionAttributesAreClearedBetweenInvocations() throws Exception {
+
+		this.mvc.perform(get("/"))
+			.andExpect(content().string(HELLO))
+			.andExpect(request().sessionAttribute(FOO, nullValue()));
+
+		this.mvc.perform(get("/").sessionAttr(FOO, BAR))
+			.andExpect(content().string(HELLO))
+			.andExpect(request().sessionAttribute(FOO, BAR));
+
+		this.mvc.perform(get("/"))
+			.andExpect(content().string(HELLO))
+			.andExpect(request().sessionAttribute(FOO, nullValue()));
+	}
+
+	@Test
+	public void requestParametersAreClearedBetweenInvocations() throws Exception {
+		this.mvc.perform(get("/"))
+			.andExpect(content().string(HELLO));
+
+		this.mvc.perform(get("/").param(ENIGMA, ""))
+			.andExpect(content().string(ENIGMA));
+
+		this.mvc.perform(get("/"))
+			.andExpect(content().string(HELLO));
+	}
+
+
+	@Configuration
+	@EnableWebMvc
+	static class Config {
+
+		@Bean
+		public MyController myController() {
+			return new MyController();
+		}
+	}
+
+	@RestController
+	static class MyController {
+
+		@RequestMapping("/")
+		public String hello() {
+			return HELLO;
+		}
+
+		@RequestMapping(path = "/", params = ENIGMA)
+		public String enigma() {
+			return ENIGMA;
+		}
+	}
+
+}

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

@@ -30,7 +30,6 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockServletContext;
-import org.springframework.test.context.web.ServletTestExecutionListener;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
@@ -50,8 +49,7 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
 
 /**
  * Integration tests for SPR-13211 which verify that a custom mock request
- * (i.e., one not created by {@link ServletTestExecutionListener}) is not
- * reused by MockMvc.
+ * is not reused by MockMvc.
  *
  * @author Sam Brannen
  * @since 4.2

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

@@ -61,7 +61,7 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
  * <ul>
  * <li>SPR-10025: Access to request attributes via RequestContextHolder</li>
  * <li>SPR-13217: Populate RequestAttributes before invoking Filters in MockMvc</li>
- * <li>SPR-13211: Reuse of mock request from the TestContext framework</li>
+ * <li>SPR-13260: No reuse of mock requests</li>
  * </ul>
  *
  * @author Rossen Stoyanchev
@@ -137,7 +137,7 @@ public class RequestContextHolderTests {
 
 	@After
 	public void verifyRestoredRequestAttributes() {
-		assertRequestAttributes();
+		assertRequestAttributes(false);
 	}
 
 
@@ -294,17 +294,34 @@ public class RequestContextHolderTests {
 
 
 	private static void assertRequestAttributes() {
+		assertRequestAttributes(true);
+	}
+
+	private static void assertRequestAttributes(boolean withinMockMvc) {
 		RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
 		assertThat(requestAttributes, instanceOf(ServletRequestAttributes.class));
-		assertRequestAttributes(((ServletRequestAttributes) requestAttributes).getRequest());
+		assertRequestAttributes(((ServletRequestAttributes) requestAttributes).getRequest(), withinMockMvc);
 	}
 
 	private static void assertRequestAttributes(ServletRequest request) {
-		assertThat(request.getAttribute(FROM_TCF_MOCK), is(FROM_TCF_MOCK));
-		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));
+		assertRequestAttributes(request, true);
+	}
+
+	private static void assertRequestAttributes(ServletRequest request, boolean withinMockMvc) {
+		if (withinMockMvc) {
+			assertThat(request.getAttribute(FROM_TCF_MOCK), is(nullValue()));
+			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));
+		}
+		else {
+			assertThat(request.getAttribute(FROM_TCF_MOCK), is(FROM_TCF_MOCK));
+			assertThat(request.getAttribute(FROM_MVC_TEST_DEFAULT), is(nullValue()));
+			assertThat(request.getAttribute(FROM_MVC_TEST_MOCK), is(nullValue()));
+			assertThat(request.getAttribute(FROM_REQUEST_FILTER), is(nullValue()));
+			assertThat(request.getAttribute(FROM_REQUEST_ATTRIBUTES_FILTER), is(nullValue()));
+		}
 	}
 
 }