From 800018a817297314e58446d55c3a15aef6f756cb Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 11 Dec 2013 01:00:04 +0100 Subject: [PATCH] Do not repopulate RequestContextHolder in ServTEL This commit fixes a bug introduced in the last commit. ServletTestExecutionListener (STEL) now tracks whether it has already populated the RequestContextHolder. Issue: SPR-11144 --- .../web/ServletTestExecutionListener.java | 25 ++++++++++++++++--- .../ServletTestExecutionListenerTests.java | 6 ++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java index 5b6a08a4f8..8f3fc809f8 100644 --- a/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java @@ -20,7 +20,6 @@ import javax.servlet.ServletContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; @@ -74,6 +73,16 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener public static final String RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE = Conventions.getQualifiedAttributeName( ServletTestExecutionListener.class, "resetRequestContextHolder"); + /** + * Attribute name for a {@link TestContext} attribute which indicates that + * {@code ServletTestExecutionListener} has already populated Spring Web's + * {@code RequestContextHolder}. + * + *

Permissible values include {@link Boolean#TRUE} and {@link Boolean#FALSE}. + */ + public static final String POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE = Conventions.getQualifiedAttributeName( + ServletTestExecutionListener.class, "populatedRequestContextHolder"); + private static final Log logger = LogFactory.getLog(ServletTestExecutionListener.class); @@ -111,8 +120,10 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener * {@code RequestContextHolder}, but only if the {@link * #RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} in the supplied {@code TestContext} * has a value of {@link Boolean#TRUE}. - *

The {@link #RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} will be - * subsequently removed from the test context, regardless of its value. + * + *

The {@link #RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} and + * {@link #POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE} will be subsequently + * removed from the test context, regardless of their values. * * @see TestExecutionListener#afterTestMethod(TestContext) */ @@ -124,6 +135,7 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener } RequestContextHolder.resetRequestAttributes(); } + testContext.removeAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE); testContext.removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE); } @@ -131,8 +143,12 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener return AnnotationUtils.findAnnotation(testContext.getTestClass(), WebAppConfiguration.class) == null; } + private boolean alreadyPopulatedRequestContextHolder(TestContext testContext) { + return Boolean.TRUE.equals(testContext.getAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)); + } + private void setUpRequestContextIfNecessary(TestContext testContext) { - if (notAnnotatedWithWebAppConfiguration(testContext)) { + if (notAnnotatedWithWebAppConfiguration(testContext) || alreadyPopulatedRequestContextHolder(testContext)) { return; } @@ -157,6 +173,7 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener ServletWebRequest servletWebRequest = new ServletWebRequest(request, response); RequestContextHolder.setRequestAttributes(servletWebRequest); + testContext.setAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); testContext.setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); if (wac instanceof ConfigurableApplicationContext) { diff --git a/spring-test/src/test/java/org/springframework/test/context/web/ServletTestExecutionListenerTests.java b/spring-test/src/test/java/org/springframework/test/context/web/ServletTestExecutionListenerTests.java index 9972c42c7c..54b23941c9 100644 --- a/spring-test/src/test/java/org/springframework/test/context/web/ServletTestExecutionListenerTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/web/ServletTestExecutionListenerTests.java @@ -175,14 +175,18 @@ public class ServletTestExecutionListenerTests { private void assertWebAppConfigTestCase() throws Exception { listener.prepareTestInstance(testContext); assertAttributeDoesNotExist(); + verify(testContext, times(1)).setAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); verify(testContext, times(1)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); + when(testContext.getAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(Boolean.TRUE); when(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE)).thenReturn(Boolean.TRUE); listener.beforeTestMethod(testContext); assertAttributeDoesNotExist(); - verify(testContext, times(2)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); + verify(testContext, times(1)).setAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); + verify(testContext, times(1)).setAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE, Boolean.TRUE); listener.afterTestMethod(testContext); + verify(testContext).removeAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE); verify(testContext).removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE); assertAttributesNotAvailable(); }