Proper support for Root WAC in Spring MVC Test

sbrannen · sbrannen · commit 33b9bd72cc4d · 2015-06-12T21:38:34.000+02:00
The modifications to DefaultMockMvcBuilder performed in conjunction with SPR-12553 introduced a breaking change: the WebApplicationContext supplied to DefaultMockMvcBuilder's constructor was *always* stored in the ServletContext as the root WebApplicationContext, overwriting a root WebApplicationContext that had been set by the user or by the Spring TestContext Framework (TCF) -- for example, in AbstractGenericWebContextLoader. Consequently, the changes in SPR-12553 cause tests that use @ContextHierarchy to fail if web components rely on the correct WebApplicationContext being stored under the WebApplicationContext#ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE key. This commit reverts the breaking changes introduced in SPR-12553: if the root WebApplicationContext has already been set in the ServletContext of the WebApplicationContext supplied to DefaultMockMvcBuilder, no action is taken. Furthermore, this commit introduces new code to address the initial intent of SPR-12553. Specifically, if the root WebApplicationContext has NOT been set in the ServletContext of the WebApplicationContext supplied to DefaultMockMvcBuilder, the application context hierarchy will be traversed in search of the root WebApplicationContext, and the root WebApplicationContext will then be stored under the ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE key. Issue: SPR-13075, SPR-12553 (cherry picked from commit f6d2fe4)
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/DefaultMockMvcBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/DefaultMockMvcBuilder.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -18,16 +18,25 @@
 
 import javax.servlet.ServletContext;
 
+import org.springframework.context.ApplicationContext;
 import org.springframework.util.Assert;
 import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.context.support.WebApplicationContextUtils;
 
 /**
- * An concrete implementation of {@link AbstractMockMvcBuilder} that simply
- * provides the WebApplicationContext given to it as a constructor argument.
+ * A concrete implementation of {@link AbstractMockMvcBuilder} that provides
+ * the {@link WebApplicationContext} supplied to it as a constructor argument.
+ *
+ * <p>In addition, if the {@link ServletContext} in the supplied
+ * {@code WebApplicationContext} does not contain an entry for the
+ * {@link WebApplicationContext#ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE}
+ * key, the root {@code WebApplicationContext} will be detected and stored
+ * in the {@code ServletContext} under the
+ * {@code ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE} key.
  *
  * @author Rossen Stoyanchev
  * @author Rob Winch
- * @author Sebastien Deleuze
+ * @author Sam Brannen
  * @since 3.2
  */
 public class DefaultMockMvcBuilder extends AbstractMockMvcBuilder<DefaultMockMvcBuilder> {
@@ -45,11 +54,25 @@ protected DefaultMockMvcBuilder(WebApplicationContext webAppContext) {
 		this.webAppContext = webAppContext;
 	}
 
-
 	@Override
 	protected WebApplicationContext initWebAppContext() {
+
 		ServletContext servletContext = this.webAppContext.getServletContext();
-		servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, this.webAppContext);
+		ApplicationContext rootWac = WebApplicationContextUtils.getWebApplicationContext(servletContext);
+
+		if (rootWac == null) {
+			rootWac = this.webAppContext;
+			ApplicationContext parent = this.webAppContext.getParent();
+			while (parent != null) {
+				if (parent instanceof WebApplicationContext && !(parent.getParent() instanceof WebApplicationContext)) {
+					rootWac = parent;
+					break;
+				}
+				parent = parent.getParent();
+			}
+			servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, rootWac);
+		}
+
 		return this.webAppContext;
 	}
 
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java
@@ -64,20 +64,20 @@
 import org.springframework.web.servlet.view.InternalResourceViewResolver;
 
 /**
- * A MockMvcBuilder that accepts {@code @Controller} registrations thus allowing
- * full control over the instantiation and the initialization of controllers and
- * their dependencies similar to plain unit tests, and also making it possible
- * to test one controller at a time.
+ * A {@code MockMvcBuilder} that accepts {@code @Controller} registrations
+ * thus allowing full control over the instantiation and initialization of
+ * controllers and their dependencies similar to plain unit tests, and also
+ * making it possible to test one controller at a time.
  *
  * <p>This builder creates the minimum infrastructure required by the
  * {@link DispatcherServlet} to serve requests with annotated controllers and
- * also provides methods to customize it. The resulting configuration and
- * customizations possible are equivalent to using the MVC Java config except
+ * also provides methods for customization. The resulting configuration and
+ * customization options are equivalent to using MVC Java config except
  * using builder style methods.
  *
  * <p>To configure view resolution, either select a "fixed" view to use for every
- * performed request (see {@link #setSingleView(View)}) or provide a list of
- * {@code ViewResolver}'s, see {@link #setViewResolvers(ViewResolver...)}.
+ * request performed (see {@link #setSingleView(View)}) or provide a list of
+ * {@code ViewResolver}s (see {@link #setViewResolvers(ViewResolver...)}).
  *
  * @author Rossen Stoyanchev
  * @since 3.2
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/context/JavaConfigTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/context/JavaConfigTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -16,12 +16,16 @@
 
 package org.springframework.test.web.servlet.samples.context;
 
+import javax.servlet.ServletContext;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+
 import org.mockito.Mockito;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.MediaType;
@@ -43,6 +47,7 @@
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
 import org.springframework.web.servlet.view.tiles3.TilesConfigurer;
 
+import static org.junit.Assert.*;
 import static org.mockito.BDDMockito.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
@@ -69,12 +74,16 @@ public class JavaConfigTests {
 	@Autowired
 	private PersonDao personDao;
 
+	@Autowired
+	private PersonController personController;
+
 	private MockMvc mockMvc;
 
 
 	@Before
 	public void setup() {
 		this.mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
+		verifyRootWacSupport();
 		given(this.personDao.getPerson(5L)).willReturn(new Person("Joe"));
 	}
 
@@ -88,9 +97,38 @@ public void person() throws Exception {
 
 	@Test
 	public void tilesDefinitions() throws Exception {
-		this.mockMvc.perform(get("/"))//
-		.andExpect(status().isOk())//
-		.andExpect(forwardedUrl("/WEB-INF/layouts/standardLayout.jsp"));
+		this.mockMvc.perform(get("/"))
+			.andExpect(status().isOk())
+			.andExpect(forwardedUrl("/WEB-INF/layouts/standardLayout.jsp"));
+	}
+
+	/**
+	 * Verify that the breaking change introduced in <a
+	 * href="https://jira.spring.io/browse/SPR-12553">SPR-12553</a> has been reverted.
+	 *
+	 * <p>This code has been copied from
+	 * {@link org.springframework.test.context.hierarchies.web.ControllerIntegrationTests}.
+	 *
+	 * @see org.springframework.test.context.hierarchies.web.ControllerIntegrationTests#verifyRootWacSupport()
+	 */
+	private void verifyRootWacSupport() {
+		assertNotNull(personDao);
+		assertNotNull(personController);
+
+		ApplicationContext parent = wac.getParent();
+		assertNotNull(parent);
+		assertTrue(parent instanceof WebApplicationContext);
+		WebApplicationContext root = (WebApplicationContext) parent;
+		assertFalse(root.getBeansOfType(String.class).containsKey("bar"));
+
+		ServletContext childServletContext = wac.getServletContext();
+		assertNotNull(childServletContext);
+		ServletContext rootServletContext = root.getServletContext();
+		assertNotNull(rootServletContext);
+		assertSame(childServletContext, rootServletContext);
+
+		assertSame(root, rootServletContext.getAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE));
+		assertSame(root, childServletContext.getAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE));
 	}
 
 
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/setup/DefaultMockMvcBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/setup/DefaultMockMvcBuilderTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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. You may obtain a copy of the License at
@@ -12,28 +12,112 @@
  */
 package org.springframework.test.web.servlet.setup;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
+import org.springframework.context.support.StaticApplicationContext;
 import org.springframework.mock.web.MockServletContext;
+import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.context.support.StaticWebApplicationContext;
 import org.springframework.web.context.support.WebApplicationContextUtils;
 
+import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
 
 /**
  * Tests for {@link DefaultMockMvcBuilder}.
  *
  * @author Rob Winch
  * @author Sebastien Deleuze
+ * @author Sam Brannen
  */
 public class DefaultMockMvcBuilderTests {
 
-	@Test // SPR-12553
-	public void applicationContextAttribute() {
-		MockServletContext servletContext = new MockServletContext();
-		StubWebApplicationContext wac = new StubWebApplicationContext(servletContext);
-		DefaultMockMvcBuilder builder = MockMvcBuilders.webAppContextSetup(wac);
-		assertEquals(builder.initWebAppContext(), WebApplicationContextUtils
-				.getRequiredWebApplicationContext(servletContext));
+	private final MockServletContext servletContext = new MockServletContext();
+
+	@Rule
+	public final ExpectedException exception = ExpectedException.none();
+
+
+	@Test
+	public void webAppContextSetupWithNullWac() {
+		exception.expect(IllegalArgumentException.class);
+		exception.expectMessage(equalTo("WebApplicationContext is required"));
+		webAppContextSetup(null);
+	}
+
+	@Test
+	public void webAppContextSetupWithNullServletContext() {
+		exception.expect(IllegalArgumentException.class);
+		exception.expectMessage(equalTo("WebApplicationContext must have a ServletContext"));
+		webAppContextSetup(new StubWebApplicationContext(null));
+	}
+
+	/**
+	 * See SPR-12553 and SPR-13075.
+	 */
+	@Test
+	public void rootWacServletContainerAttributePreviouslySet() {
+		StubWebApplicationContext child = new StubWebApplicationContext(this.servletContext);
+		this.servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, child);
+
+		DefaultMockMvcBuilder builder = webAppContextSetup(child);
+		assertSame(builder.initWebAppContext(),
+			WebApplicationContextUtils.getRequiredWebApplicationContext(this.servletContext));
+	}
+
+	/**
+	 * See SPR-12553 and SPR-13075.
+	 */
+	@Test
+	public void rootWacServletContainerAttributePreviouslySetWithContextHierarchy() {
+		StubWebApplicationContext root = new StubWebApplicationContext(this.servletContext);
+
+		this.servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, root);
+
+		StaticWebApplicationContext child = new StaticWebApplicationContext();
+		child.setParent(root);
+		child.setServletContext(this.servletContext);
+
+		DefaultMockMvcBuilder builder = webAppContextSetup(child);
+		assertSame(builder.initWebAppContext().getParent(),
+			WebApplicationContextUtils.getRequiredWebApplicationContext(this.servletContext));
+	}
+
+	/**
+	 * See SPR-12553 and SPR-13075.
+	 */
+	@Test
+	public void rootWacServletContainerAttributeNotPreviouslySet() {
+		StubWebApplicationContext root = new StubWebApplicationContext(this.servletContext);
+		DefaultMockMvcBuilder builder = webAppContextSetup(root);
+		WebApplicationContext wac = builder.initWebAppContext();
+		assertSame(root, wac);
+		assertSame(root, WebApplicationContextUtils.getRequiredWebApplicationContext(this.servletContext));
+	}
+
+	/**
+	 * See SPR-12553 and SPR-13075.
+	 */
+	@Test
+	public void rootWacServletContainerAttributeNotPreviouslySetWithContextHierarchy() {
+		StaticApplicationContext ear = new StaticApplicationContext();
+		StaticWebApplicationContext root = new StaticWebApplicationContext();
+		root.setParent(ear);
+		root.setServletContext(this.servletContext);
+		StaticWebApplicationContext dispatcher = new StaticWebApplicationContext();
+		dispatcher.setParent(root);
+		dispatcher.setServletContext(this.servletContext);
+
+		DefaultMockMvcBuilder builder = webAppContextSetup(dispatcher);
+		WebApplicationContext wac = builder.initWebAppContext();
+
+		assertSame(dispatcher, wac);
+		assertSame(root, wac.getParent());
+		assertSame(ear, wac.getParent().getParent());
+		assertSame(root, WebApplicationContextUtils.getRequiredWebApplicationContext(this.servletContext));
 	}
 
 }
