Skip to content

Commit 6ded287

Browse files
sbrannensbrannen
committed
Honor presence of @WebAppConfiguration in ServTEL
The previous commit for issue SPR-11144 revealed a bug in ServletTestExecutionListener (STEL). Specifically, STEL acted on the fact that the ApplicationContext for a given TestContext was an instance of WebApplicationContext. This behavior could potentially break test code from previous releases of the Spring Framework that relied on a custom setup of the RequestAttributes in the RequestContextHolder with a custom WebApplicationContext ContextLoader. This commit addresses this issue by ensuring that STEL only comes into play if the test class is annotated with @WebAppConfiguration (for prepareTestInstance() and beforeTestMethod()) or if the TestContext attribute named RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE is set to Boolean.TRUE (for afterTestMethod()). Issue: SPR-11144 Backport-Commit: 099b10d
1 parent 6e275b7 commit 6ded287

File tree

2 files changed

+118
-39
lines changed

2 files changed

+118
-39
lines changed

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

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@
2525
import org.springframework.context.ApplicationContext;
2626
import org.springframework.context.ConfigurableApplicationContext;
2727
import org.springframework.core.Conventions;
28+
import org.springframework.core.annotation.AnnotationUtils;
2829
import org.springframework.mock.web.MockHttpServletRequest;
2930
import org.springframework.mock.web.MockHttpServletResponse;
3031
import org.springframework.mock.web.MockServletContext;
3132
import org.springframework.test.context.TestContext;
3233
import org.springframework.test.context.TestExecutionListener;
3334
import org.springframework.test.context.support.AbstractTestExecutionListener;
35+
import org.springframework.util.Assert;
3436
import org.springframework.web.context.WebApplicationContext;
3537
import org.springframework.web.context.request.RequestContextHolder;
3638
import org.springframework.web.context.request.ServletWebRequest;
@@ -52,8 +54,9 @@
5254
* #afterTestMethod(TestContext) cleans up} thread-local state.
5355
*
5456
* <p>Note that {@code ServletTestExecutionListener} is enabled by default but
55-
* takes no action if the {@link ApplicationContext} loaded for the current test
56-
* is not a {@link WebApplicationContext}.
57+
* generally takes no action if the {@linkplain TestContext#getTestClass() test
58+
* class} is not annotated with {@link WebAppConfiguration @WebAppConfiguration}.
59+
* See the Javadoc for individual methods in this class for details.
5760
*
5861
* @author Sam Brannen
5962
* @since 3.2
@@ -76,7 +79,9 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener
7679

7780
/**
7881
* Sets up thread-local state during the <em>test instance preparation</em>
79-
* callback phase via Spring Web's {@link RequestContextHolder}.
82+
* callback phase via Spring Web's {@link RequestContextHolder}, but only if
83+
* the {@linkplain TestContext#getTestClass() test class} is annotated with
84+
* {@link WebAppConfiguration @WebAppConfiguration}.
8085
*
8186
* @see TestExecutionListener#prepareTestInstance(TestContext)
8287
* @see #setUpRequestContextIfNecessary(TestContext)
@@ -88,7 +93,9 @@ public void prepareTestInstance(TestContext testContext) throws Exception {
8893

8994
/**
9095
* Sets up thread-local state before each test method via Spring Web's
91-
* {@link RequestContextHolder}.
96+
* {@link RequestContextHolder}, but only if the
97+
* {@linkplain TestContext#getTestClass() test class} is annotated with
98+
* {@link WebAppConfiguration @WebAppConfiguration}.
9299
*
93100
* @see TestExecutionListener#beforeTestMethod(TestContext)
94101
* @see #setUpRequestContextIfNecessary(TestContext)
@@ -101,7 +108,11 @@ public void beforeTestMethod(TestContext testContext) throws Exception {
101108
/**
102109
* Cleans up thread-local state after each test method by {@linkplain
103110
* RequestContextHolder#resetRequestAttributes() resetting} Spring Web's
104-
* {@code RequestContextHolder}.
111+
* {@code RequestContextHolder}, but only if the {@link
112+
* #RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} in the supplied {@code TestContext}
113+
* has a value of {@link Boolean#TRUE}.
114+
* <p>The {@link #RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} will be
115+
* subsequently removed from the test context, regardless of its value.
105116
*
106117
* @see TestExecutionListener#afterTestMethod(TestContext)
107118
*/
@@ -111,45 +122,48 @@ public void afterTestMethod(TestContext testContext) throws Exception {
111122
logger.debug(String.format("Resetting RequestContextHolder for test context %s.", testContext));
112123
}
113124
RequestContextHolder.resetRequestAttributes();
114-
testContext.removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
115125
}
126+
testContext.removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
127+
}
128+
129+
private boolean notAnnotatedWithWebAppConfiguration(TestContext testContext) {
130+
return AnnotationUtils.findAnnotation(testContext.getTestClass(), WebAppConfiguration.class) == null;
116131
}
117132

118133
private void setUpRequestContextIfNecessary(TestContext testContext) {
134+
if (notAnnotatedWithWebAppConfiguration(testContext)) {
135+
return;
136+
}
119137

120138
ApplicationContext context = testContext.getApplicationContext();
121139

122140
if (context instanceof WebApplicationContext) {
123141
WebApplicationContext wac = (WebApplicationContext) context;
124142
ServletContext servletContext = wac.getServletContext();
125-
if (!(servletContext instanceof MockServletContext)) {
126-
throw new IllegalStateException(String.format(
127-
"The WebApplicationContext for test context %s must be configured with a MockServletContext.",
128-
testContext));
129-
}
143+
Assert.state(servletContext instanceof MockServletContext, String.format(
144+
"The WebApplicationContext for test context %s must be configured with a MockServletContext.",
145+
testContext));
130146

131147
if (logger.isDebugEnabled()) {
132148
logger.debug(String.format(
133149
"Setting up MockHttpServletRequest, MockHttpServletResponse, ServletWebRequest, and RequestContextHolder for test context %s.",
134150
testContext));
135151
}
136152

137-
if (RequestContextHolder.getRequestAttributes() == null) {
138-
MockServletContext mockServletContext = (MockServletContext) servletContext;
139-
MockHttpServletRequest request = new MockHttpServletRequest(mockServletContext);
140-
MockHttpServletResponse response = new MockHttpServletResponse();
141-
ServletWebRequest servletWebRequest = new ServletWebRequest(request, response);
142-
143-
RequestContextHolder.setRequestAttributes(servletWebRequest);
144-
testContext.setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
145-
146-
if (wac instanceof ConfigurableApplicationContext) {
147-
@SuppressWarnings("resource")
148-
ConfigurableApplicationContext configurableApplicationContext = (ConfigurableApplicationContext) wac;
149-
ConfigurableListableBeanFactory bf = configurableApplicationContext.getBeanFactory();
150-
bf.registerResolvableDependency(MockHttpServletResponse.class, response);
151-
bf.registerResolvableDependency(ServletWebRequest.class, servletWebRequest);
152-
}
153+
MockServletContext mockServletContext = (MockServletContext) servletContext;
154+
MockHttpServletRequest request = new MockHttpServletRequest(mockServletContext);
155+
MockHttpServletResponse response = new MockHttpServletResponse();
156+
ServletWebRequest servletWebRequest = new ServletWebRequest(request, response);
157+
158+
RequestContextHolder.setRequestAttributes(servletWebRequest);
159+
testContext.setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
160+
161+
if (wac instanceof ConfigurableApplicationContext) {
162+
@SuppressWarnings("resource")
163+
ConfigurableApplicationContext configurableApplicationContext = (ConfigurableApplicationContext) wac;
164+
ConfigurableListableBeanFactory bf = configurableApplicationContext.getBeanFactory();
165+
bf.registerResolvableDependency(MockHttpServletResponse.class, response);
166+
bf.registerResolvableDependency(ServletWebRequest.class, servletWebRequest);
153167
}
154168
}
155169
}

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

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import org.junit.Before;
2020
import org.junit.Test;
21+
import org.mockito.Mockito;
2122
import org.springframework.context.ApplicationContext;
2223
import org.springframework.mock.web.MockHttpServletRequest;
2324
import org.springframework.mock.web.MockHttpServletResponse;
@@ -48,6 +49,14 @@ public class ServletTestExecutionListenerTests {
4849
private final ServletTestExecutionListener listener = new ServletTestExecutionListener();
4950

5051

52+
private void assertAttributesAvailable() {
53+
assertNotNull("request attributes should be available", RequestContextHolder.getRequestAttributes());
54+
}
55+
56+
private void assertAttributesNotAvailable() {
57+
assertNull("request attributes should not be available", RequestContextHolder.getRequestAttributes());
58+
}
59+
5160
private void assertAttributeExists() {
5261
RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
5362
assertNotNull("request attributes should exist", requestAttributes);
@@ -80,9 +89,13 @@ public void setUp() {
8089
}
8190

8291
@Test
83-
public void withStandardApplicationContext() throws Exception {
92+
public void standardApplicationContext() throws Exception {
93+
Mockito.<Class<?>> when(testContext.getTestClass()).thenReturn(getClass());
8494
when(testContext.getApplicationContext()).thenReturn(mock(ApplicationContext.class));
8595

96+
listener.beforeTestClass(testContext);
97+
assertAttributeExists();
98+
8699
listener.prepareTestInstance(testContext);
87100
assertAttributeExists();
88101

@@ -94,26 +107,33 @@ public void withStandardApplicationContext() throws Exception {
94107
}
95108

96109
@Test
97-
public void withWebApplicationContextWithoutExistingRequestAttributes() throws Exception {
98-
assertAttributeExists();
110+
public void legacyWebTestCaseWithoutExistingRequestAttributes() throws Exception {
111+
Mockito.<Class<?>> when(testContext.getTestClass()).thenReturn(LegacyWebTestCase.class);
112+
99113
RequestContextHolder.resetRequestAttributes();
114+
assertAttributesNotAvailable();
115+
116+
listener.beforeTestClass(testContext);
100117

101118
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);
119+
assertAttributesNotAvailable();
120+
verify(testContext, times(0)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
121+
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(null);
105122

106123
listener.beforeTestMethod(testContext);
107-
assertAttributeDoesNotExist();
108-
verify(testContext).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
124+
assertAttributesNotAvailable();
125+
verify(testContext, times(0)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
109126

110127
listener.afterTestMethod(testContext);
111-
verify(testContext).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
112-
assertNull("request attributes should have been cleared", RequestContextHolder.getRequestAttributes());
128+
verify(testContext, times(1)).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
129+
assertAttributesNotAvailable();
113130
}
114131

115132
@Test
116-
public void withWebApplicationContextWithPresetRequestAttributes() throws Exception {
133+
public void legacyWebTestCaseWithPresetRequestAttributes() throws Exception {
134+
Mockito.<Class<?>> when(testContext.getTestClass()).thenReturn(LegacyWebTestCase.class);
135+
136+
listener.beforeTestClass(testContext);
117137
assertAttributeExists();
118138

119139
listener.prepareTestInstance(testContext);
@@ -127,7 +147,52 @@ public void withWebApplicationContextWithPresetRequestAttributes() throws Except
127147
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(null);
128148

129149
listener.afterTestMethod(testContext);
130-
verify(testContext, times(0)).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
150+
verify(testContext, times(1)).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
151+
assertAttributeExists();
152+
}
153+
154+
@Test
155+
public void atWebAppConfigTestCaseWithoutExistingRequestAttributes() throws Exception {
156+
Mockito.<Class<?>> when(testContext.getTestClass()).thenReturn(AtWebAppConfigWebTestCase.class);
157+
158+
RequestContextHolder.resetRequestAttributes();
159+
listener.beforeTestClass(testContext);
160+
assertAttributesNotAvailable();
161+
162+
assertWebAppConfigTestCase();
163+
}
164+
165+
@Test
166+
public void atWebAppConfigTestCaseWithPresetRequestAttributes() throws Exception {
167+
Mockito.<Class<?>> when(testContext.getTestClass()).thenReturn(AtWebAppConfigWebTestCase.class);
168+
169+
listener.beforeTestClass(testContext);
170+
assertAttributesAvailable();
171+
172+
assertWebAppConfigTestCase();
173+
}
174+
175+
private void assertWebAppConfigTestCase() throws Exception {
176+
listener.prepareTestInstance(testContext);
177+
assertAttributeDoesNotExist();
178+
verify(testContext, times(1)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
179+
when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(Boolean.TRUE);
180+
181+
listener.beforeTestMethod(testContext);
182+
assertAttributeDoesNotExist();
183+
verify(testContext, times(2)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE);
184+
185+
listener.afterTestMethod(testContext);
186+
verify(testContext).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
187+
assertAttributesNotAvailable();
188+
}
189+
190+
191+
static class LegacyWebTestCase {
192+
}
193+
194+
@WebAppConfiguration
195+
static class AtWebAppConfigWebTestCase {
131196
}
132197

133198
}

0 commit comments

Comments
 (0)