Skip to content

Commit c89985c

Browse files
sbrannensbrannen
committed
Ensure ServTEL doesn't reset ReqAttrs by accident
Prior to this commit, the ServletTestExecutionListener did not overwrite RequestAttributes in the RequestContextHolder if the ApplicationContext associated with the given TestContext was not a WebApplicationContext; however, the ServletTestExecutionListener would clear the RequestAttributes after every test method execution, regardless of whether the context was a WebApplicationContext or not. This behavior breaks backwards compatibility with integration tests that managed the RequestAttributes in RequestContextHolder themselves. This commit addresses this issue by introducing a TestContext attribute named RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE in ServletTestExecutionListener. This attribute is used internally within ServletTestExecutionListener to ensure that the RequestContextHolder is only cleared (i.e., reset) if the ServletTestExecutionListener actually populated the RequestContextHolder. Issue: SPR-11144 Backport-Commit: a3b022a
1 parent d4b0751 commit c89985c

File tree

2 files changed

+152
-3
lines changed

2 files changed

+152
-3
lines changed

‎spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java

+19-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2525
import org.springframework.context.ApplicationContext;
2626
import org.springframework.context.ConfigurableApplicationContext;
27+
import org.springframework.core.Conventions;
2728
import org.springframework.mock.web.MockHttpServletRequest;
2829
import org.springframework.mock.web.MockHttpServletResponse;
2930
import org.springframework.mock.web.MockServletContext;
@@ -59,6 +60,17 @@
5960
*/
6061
public class ServletTestExecutionListener extends AbstractTestExecutionListener {
6162

63+
/**
64+
* Attribute name for a {@link TestContext} attribute which indicates
65+
* whether or not the {@code ServletTestExecutionListener} should {@linkplain
66+
* RequestContextHolder#resetRequestAttributes() reset} Spring Web's
67+
* {@code RequestContextHolder} in {@link #afterTestMethod(TestContext)}.
68+
*
69+
* <p>Permissible values include {@link Boolean#TRUE} and {@link Boolean#FALSE}.
70+
*/
71+
public static final String RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE = Conventions.getQualifiedAttributeName(
72+
ServletTestExecutionListener.class, "resetRequestContextHolder");
73+
6274
private static final Log logger = LogFactory.getLog(ServletTestExecutionListener.class);
6375

6476

@@ -94,10 +106,13 @@ public void beforeTestMethod(TestContext testContext) throws Exception {
94106
* @see TestExecutionListener#afterTestMethod(TestContext)
95107
*/
96108
public void afterTestMethod(TestContext testContext) throws Exception {
97-
if (logger.isDebugEnabled()) {
98-
logger.debug(String.format("Resetting RequestContextHolder for test context %s.", testContext));
109+
if (Boolean.TRUE.equals(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE))) {
110+
if (logger.isDebugEnabled()) {
111+
logger.debug(String.format("Resetting RequestContextHolder for test context %s.", testContext));
112+
}
113+
RequestContextHolder.resetRequestAttributes();
114+
testContext.removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
99115
}
100-
RequestContextHolder.resetRequestAttributes();
101116
}
102117

103118
private void setUpRequestContextIfNecessary(TestContext testContext) {
@@ -126,6 +141,7 @@ private void setUpRequestContextIfNecessary(TestContext testContext) {
126141
ServletWebRequest servletWebRequest = new ServletWebRequest(request, response);
127142

128143
RequestContextHolder.setRequestAttributes(servletWebRequest);
144+
testContext.setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
129145

130146
if (wac instanceof ConfigurableApplicationContext) {
131147
@SuppressWarnings("resource")

‎spring-test/src/test/java/org/springframework/test/context/web/ServletTestExecutionListenerTests.java

+133
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*
2+
* Copyright 2002-2013 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.test.context.web;
18+
19+
import org.junit.Before;
20+
import org.junit.Test;
21+
import org.springframework.context.ApplicationContext;
22+
import org.springframework.mock.web.MockHttpServletRequest;
23+
import org.springframework.mock.web.MockHttpServletResponse;
24+
import org.springframework.mock.web.MockServletContext;
25+
import org.springframework.test.context.TestContext;
26+
import org.springframework.web.context.WebApplicationContext;
27+
import org.springframework.web.context.request.RequestAttributes;
28+
import org.springframework.web.context.request.RequestContextHolder;
29+
import org.springframework.web.context.request.ServletWebRequest;
30+
31+
import static org.junit.Assert.*;
32+
import static org.mockito.Mockito.*;
33+
import static org.springframework.test.context.web.ServletTestExecutionListener.*;
34+
35+
/**
36+
* Unit tests for {@link ServletTestExecutionListener}.
37+
*
38+
* @author Sam Brannen
39+
* @since 3.2.6
40+
*/
41+
public class ServletTestExecutionListenerTests {
42+
43+
private static final String SET_UP_OUTSIDE_OF_STEL = "SET_UP_OUTSIDE_OF_STEL";
44+
45+
private final WebApplicationContext wac = mock(WebApplicationContext.class);
46+
private final MockServletContext mockServletContext = new MockServletContext();
47+
private final TestContext testContext = mock(TestContext.class);
48+
private final ServletTestExecutionListener listener = new ServletTestExecutionListener();
49+
50+
51+
private void assertAttributeExists() {
52+
RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
53+
assertNotNull("request attributes should exist", requestAttributes);
54+
Object setUpOutsideOfStel = requestAttributes.getAttribute(SET_UP_OUTSIDE_OF_STEL,
55+
RequestAttributes.SCOPE_REQUEST);
56+
assertNotNull(SET_UP_OUTSIDE_OF_STEL + " should exist as a request attribute", setUpOutsideOfStel);
57+
}
58+
59+
private void assertAttributeDoesNotExist() {
60+
RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
61+
assertNotNull("request attributes should exist", requestAttributes);
62+
Object setUpOutsideOfStel = requestAttributes.getAttribute(SET_UP_OUTSIDE_OF_STEL,
63+
RequestAttributes.SCOPE_REQUEST);
64+
assertNull(SET_UP_OUTSIDE_OF_STEL + " should NOT exist as a request attribute", setUpOutsideOfStel);
65+
}
66+
67+
@Before
68+
public void setUp() {
69+
when(wac.getServletContext()).thenReturn(mockServletContext);
70+
when(testContext.getApplicationContext()).thenReturn(wac);
71+
72+
MockHttpServletRequest request = new MockHttpServletRequest(mockServletContext);
73+
MockHttpServletResponse response = new MockHttpServletResponse();
74+
ServletWebRequest servletWebRequest = new ServletWebRequest(request, response);
75+
76+
request.setAttribute(SET_UP_OUTSIDE_OF_STEL, "true");
77+
78+
RequestContextHolder.setRequestAttributes(servletWebRequest);
79+
assertAttributeExists();
80+
}
81+
82+
@Test
83+
public void withStandardApplicationContext() throws Exception {
84+
when(testContext.getApplicationContext()).thenReturn(mock(ApplicationContext.class));
85+
86+
listener.prepareTestInstance(testContext);
87+
assertAttributeExists();
88+
89+
listener.beforeTestMethod(testContext);
90+
assertAttributeExists();
91+
92+
listener.afterTestMethod(testContext);
93+
assertAttributeExists();
94+
}
95+
96+
@Test
97+
public void withWebApplicationContextWithoutExistingRequestAttributes() throws Exception {
98+
assertAttributeExists();
99+
RequestContextHolder.resetRequestAttributes();
100+
101+
listener.prepareTestInstance(testContext);
102+
assertAttributeDoesNotExist();
103+
verify(testContext).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
104+
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(Boolean.TRUE);
105+
106+
listener.beforeTestMethod(testContext);
107+
assertAttributeDoesNotExist();
108+
verify(testContext).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
109+
110+
listener.afterTestMethod(testContext);
111+
verify(testContext).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
112+
assertNull("request attributes should have been cleared", RequestContextHolder.getRequestAttributes());
113+
}
114+
115+
@Test
116+
public void withWebApplicationContextWithPresetRequestAttributes() throws Exception {
117+
assertAttributeExists();
118+
119+
listener.prepareTestInstance(testContext);
120+
assertAttributeExists();
121+
verify(testContext, times(0)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
122+
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(null);
123+
124+
listener.beforeTestMethod(testContext);
125+
assertAttributeExists();
126+
verify(testContext, times(0)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
127+
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(null);
128+
129+
listener.afterTestMethod(testContext);
130+
verify(testContext, times(0)).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
131+
}
132+
133+
}

0 commit comments

Comments
 (0)