MockMvc HtmlUnit support shares CookieManager

Previously MockMvc builders failed to share the WebConnection used for
managing cookies in the MockMvcWebConnection. This meant that the various
CookieManagers would have different states.

This commit ensures that the WebConnection is set on the
MockMvcWebConnection.

Fixes SPR-14066

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilder.java

@@ -106,7 +106,7 @@ public class MockMvcWebClientBuilder extends MockMvcWebConnectionBuilderSupport<
 	 */
 	public MockMvcWebClientBuilder withDelegate(WebClient webClient) {
 		Assert.notNull(webClient, "WebClient must not be null");
-		webClient.setWebConnection(createConnection(webClient.getWebConnection()));
+		webClient.setWebConnection(createConnection(webClient));
 		this.webClient = webClient;
 		return this;
 	}

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java

@@ -63,16 +63,16 @@ public final class MockMvcWebConnection implements WebConnection {
 
 	private WebClient webClient;
 
-
 	/**
 	 * Create a new instance that assumes the context path of the application
 	 * is {@code ""} (i.e., the root context).
 	 * <p>For example, the URL {@code http://localhost/test/this} would use
 	 * {@code ""} as the context path.
 	 * @param mockMvc the {@code MockMvc} instance to use; never {@code null}
+	 * @param webClient the {@link WebClient} to use. never {@code null}
 	 */
-	public MockMvcWebConnection(MockMvc mockMvc) {
-		this(mockMvc, "");
+	public MockMvcWebConnection(MockMvc mockMvc, WebClient webClient) {
+		this(mockMvc, webClient, "");
 	}
 
 	/**
@@ -83,17 +83,47 @@ public final class MockMvcWebConnection implements WebConnection {
 	 * which states that it can be an empty string and otherwise must start
 	 * with a "/" character and not end with a "/" character.
 	 * @param mockMvc the {@code MockMvc} instance to use; never {@code null}
+	 * @param webClient  the {@link WebClient} to use. never {@code null}
 	 * @param contextPath the contextPath to use
 	 */
-	public MockMvcWebConnection(MockMvc mockMvc, String contextPath) {
+	public MockMvcWebConnection(MockMvc mockMvc, WebClient webClient, String contextPath) {
 		Assert.notNull(mockMvc, "MockMvc must not be null");
+		Assert.notNull(webClient, "WebClient must not be null");
 		validateContextPath(contextPath);
 
-		this.webClient = new WebClient();
+		this.webClient = webClient;
 		this.mockMvc = mockMvc;
 		this.contextPath = contextPath;
 	}
 
+	/**
+	 * Create a new instance that assumes the context path of the application
+	 * is {@code ""} (i.e., the root context).
+	 * <p>For example, the URL {@code http://localhost/test/this} would use
+	 * {@code ""} as the context path.
+	 * @param mockMvc the {@code MockMvc} instance to use; never {@code null}
+	 * @deprecated Use {@link #MockMvcWebConnection(MockMvc, WebClient)}
+	 */
+	@Deprecated
+	public MockMvcWebConnection(MockMvc mockMvc) {
+		this(mockMvc, "");
+	}
+
+	/**
+	 * Create a new instance with the specified context path.
+	 * <p>The path may be {@code null} in which case the first path segment
+	 * of the URL is turned into the contextPath. Otherwise it must conform
+	 * to {@link javax.servlet.http.HttpServletRequest#getContextPath()}
+	 * which states that it can be an empty string and otherwise must start
+	 * with a "/" character and not end with a "/" character.
+	 * @param mockMvc the {@code MockMvc} instance to use; never {@code null}
+	 * @param contextPath the contextPath to use
+	 * @deprecated use {@link #MockMvcWebConnection(MockMvc, WebClient, String)}
+	 */
+	@Deprecated
+	public MockMvcWebConnection(MockMvc mockMvc, String contextPath) {
+		this(mockMvc, new WebClient(), contextPath);
+	}
 
 	public void setWebClient(WebClient webClient) {
 		Assert.notNull(webClient, "WebClient must not be null");

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionBuilderSupport.java

@@ -19,6 +19,7 @@ package org.springframework.test.web.servlet.htmlunit;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.gargoylesoftware.htmlunit.WebClient;
 import com.gargoylesoftware.htmlunit.WebConnection;
 
 import org.springframework.test.web.servlet.MockMvc;
@@ -145,10 +146,46 @@ public abstract class MockMvcWebConnectionBuilderSupport<T extends MockMvcWebCon
 	 * @see #alwaysUseMockMvc()
 	 * @see #useMockMvc(WebRequestMatcher...)
 	 * @see #useMockMvcForHosts(String...)
+	 * @deprecated Use {@link #createConnection(WebClient)} instead
 	 */
+	@Deprecated
 	protected final WebConnection createConnection(WebConnection defaultConnection) {
 		Assert.notNull(defaultConnection, "Default WebConnection must not be null");
-		MockMvcWebConnection mockMvcWebConnection = new MockMvcWebConnection(this.mockMvc, this.contextPath);
+		return createConnection(new WebClient(), defaultConnection);
+	}
+
+	/**
+	 * Create a new {@link WebConnection} that will use a {@link MockMvc}
+	 * instance if one of the specified {@link WebRequestMatcher} instances
+	 * matches.
+	 * @param webClient the WebClient to use if none of
+	 * the specified {@code WebRequestMatcher} instances matches; never {@code null}
+	 * @return a new {@code WebConnection} that will use a {@code MockMvc}
+	 * instance if one of the specified {@code WebRequestMatcher} matches
+	 * @see #alwaysUseMockMvc()
+	 * @see #useMockMvc(WebRequestMatcher...)
+	 * @see #useMockMvcForHosts(String...)
+	 */
+	protected final WebConnection createConnection(WebClient webClient) {
+		Assert.notNull(webClient, "WebClient must not be null");
+		return createConnection(webClient, webClient.getWebConnection());
+	}
+
+	/**
+	 * Create a new {@link WebConnection} that will use a {@link MockMvc}
+	 * instance if one of the specified {@link WebRequestMatcher} instances
+	 * matches.
+	 * @param webClient the WebClient to use if none of
+	 * the specified {@code WebRequestMatcher} instances matches; never {@code null}
+	 * @param defaultConnection the WebConnection to use
+	 * @return a new {@code WebConnection} that will use a {@code MockMvc}
+	 * instance if one of the specified {@code WebRequestMatcher} matches
+	 * @see #alwaysUseMockMvc()
+	 * @see #useMockMvc(WebRequestMatcher...)
+	 * @see #useMockMvcForHosts(String...)
+	 */
+	private WebConnection createConnection(WebClient webClient, WebConnection defaultConnection) {
+		MockMvcWebConnection mockMvcWebConnection = new MockMvcWebConnection(this.mockMvc, webClient, this.contextPath);
 
 		if (this.alwaysUseMockMvc) {
 			return mockMvcWebConnection;
@@ -162,5 +199,4 @@ public abstract class MockMvcWebConnectionBuilderSupport<T extends MockMvcWebCon
 
 		return new DelegatingWebConnection(defaultConnection, delegates);
 	}
-
 }

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilder.java

@@ -129,7 +129,7 @@ public class MockMvcHtmlUnitDriverBuilder extends MockMvcWebConnectionBuilderSup
 	public MockMvcHtmlUnitDriverBuilder withDelegate(WebConnectionHtmlUnitDriver driver) {
 		Assert.notNull(driver, "HtmlUnitDriver must not be null");
 		driver.setJavascriptEnabled(this.javascriptEnabled);
-		driver.setWebConnection(createConnection(driver.getWebConnection()));
+		driver.setWebConnection(createConnection(driver.getWebClient()));
 		this.driver = driver;
 		return this;
 	}

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/WebConnectionHtmlUnitDriver.java

@@ -41,7 +41,6 @@ public class WebConnectionHtmlUnitDriver extends HtmlUnitDriver {
 
 	private WebClient webClient;
 
-
 	public WebConnectionHtmlUnitDriver(BrowserVersion browserVersion) {
 		super(browserVersion);
 	}
@@ -107,4 +106,11 @@ public class WebConnectionHtmlUnitDriver extends HtmlUnitDriver {
 		this.webClient.setWebConnection(webConnection);
 	}
 
+	/**
+	 * Gets the current {@link WebClient}
+	 * @return the current {@link WebClient}
+	 */
+	public WebClient getWebClient() {
+		return this.webClient;
+	}
 }

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java

@@ -36,6 +36,7 @@ import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 
+import com.gargoylesoftware.htmlunit.WebClient;
 import com.gargoylesoftware.htmlunit.WebConnection;
 import com.gargoylesoftware.htmlunit.WebRequest;
 import com.gargoylesoftware.htmlunit.WebResponse;
@@ -45,6 +46,7 @@ import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 /**
  * Integration tests for {@link MockMvcWebConnectionBuilderSupport}.
@@ -55,10 +57,12 @@ import static org.mockito.Mockito.mock;
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration
 @WebAppConfiguration
-@SuppressWarnings("rawtypes")
+@SuppressWarnings({"rawtypes","deprecation"})
 public class MockMvcConnectionBuilderSupportTests {
 
-	private final WebConnection delegateConnection = mock(WebConnection.class);
+	private WebConnection delegateConnection;
+
+	private WebClient webClient;
 
 	@Autowired
 	private WebApplicationContext wac;
@@ -69,10 +73,16 @@ public class MockMvcConnectionBuilderSupportTests {
 
 	@Before
 	public void setup() {
+		delegateConnection = mock(WebConnection.class);
+
+		webClient = mock(WebClient.class);
+		when(webClient.getWebConnection()).thenReturn(delegateConnection);
+
+
 		mockMvc = MockMvcBuilders.webAppContextSetup(wac).build();
 
 		connection = new MockMvcWebConnectionBuilderSupport(mockMvc){}
-				.createConnection(delegateConnection);
+				.createConnection(webClient);
 	}
 
 	@Test(expected = IllegalArgumentException.class)
@@ -86,10 +96,61 @@ public class MockMvcConnectionBuilderSupportTests {
 	}
 
 	@Test
-	public void context() throws Exception {
+	public void contextDeprecated() throws Exception {
+		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
+				.createConnection(webClient);
+
+		assertMvcProcessed("http://localhost/");
+		assertDelegateProcessed("http://example.com/");
+	}
+
+	@Test
+	public void mockMvcDeprecated() throws Exception {
+		assertMvcProcessed("http://localhost/");
+		assertDelegateProcessed("http://example.com/");
+	}
+
+	@Test
+	public void mockMvcExampleDotComDeprecated() throws Exception {
+		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
+				.useMockMvcForHosts("example.com")
+				.createConnection(delegateConnection);
+
+		assertMvcProcessed("http://localhost/");
+		assertMvcProcessed("http://example.com/");
+		assertDelegateProcessed("http://other.com/");
+	}
+
+	@Test
+	public void mockMvcAlwaysUseMockMvcDeprecated() throws Exception {
+		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
+				.alwaysUseMockMvc()
+				.createConnection(delegateConnection);
+
+		assertMvcProcessed("http://other.com/");
+	}
+
+	@Test
+	public void defaultContextPathEmptyDeprecated() throws Exception {
 		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
 				.createConnection(delegateConnection);
 
+		assertThat(getWebResponse("http://localhost/abc").getContentAsString(), equalTo(""));
+	}
+
+	@Test
+	public void defaultContextPathCustomDeprecated() throws Exception {
+		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
+				.contextPath("/abc").createConnection(delegateConnection);
+
+		assertThat(getWebResponse("http://localhost/abc/def").getContentAsString(), equalTo("/abc"));
+	}
+
+	@Test
+	public void context() throws Exception {
+		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
+				.createConnection(webClient);
+
 		assertMvcProcessed("http://localhost/");
 		assertDelegateProcessed("http://example.com/");
 	}
@@ -104,7 +165,7 @@ public class MockMvcConnectionBuilderSupportTests {
 	public void mockMvcExampleDotCom() throws Exception {
 		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
 				.useMockMvcForHosts("example.com")
-				.createConnection(delegateConnection);
+				.createConnection(webClient);
 
 		assertMvcProcessed("http://localhost/");
 		assertMvcProcessed("http://example.com/");
@@ -115,7 +176,7 @@ public class MockMvcConnectionBuilderSupportTests {
 	public void mockMvcAlwaysUseMockMvc() throws Exception {
 		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
 				.alwaysUseMockMvc()
-				.createConnection(delegateConnection);
+				.createConnection(webClient);
 
 		assertMvcProcessed("http://other.com/");
 	}
@@ -123,7 +184,7 @@ public class MockMvcConnectionBuilderSupportTests {
 	@Test
 	public void defaultContextPathEmpty() throws Exception {
 		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
-				.createConnection(delegateConnection);
+				.createConnection(webClient);
 
 		assertThat(getWebResponse("http://localhost/abc").getContentAsString(), equalTo(""));
 	}
@@ -131,7 +192,7 @@ public class MockMvcConnectionBuilderSupportTests {
 	@Test
 	public void defaultContextPathCustom() throws Exception {
 		connection = new MockMvcWebConnectionBuilderSupport(wac) {}
-				.contextPath("/abc").createConnection(delegateConnection);
+				.contextPath("/abc").createConnection(webClient);
 
 		assertThat(getWebResponse("http://localhost/abc/def").getContentAsString(), equalTo("/abc"));
 	}

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java

@@ -34,6 +34,7 @@ import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.tests.Assume;
 import org.springframework.tests.TestGroup;
+import org.springframework.web.bind.annotation.CookieValue;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.context.WebApplicationContext;
@@ -42,6 +43,7 @@ import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 import com.gargoylesoftware.htmlunit.WebClient;
 import com.gargoylesoftware.htmlunit.WebRequest;
 import com.gargoylesoftware.htmlunit.WebResponse;
+import com.gargoylesoftware.htmlunit.util.Cookie;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
@@ -99,6 +101,17 @@ public class MockMvcWebClientBuilderTests {
 		Assume.group(TestGroup.PERFORMANCE, () -> assertDelegateProcessed("http://example.com/"));
 	}
 
+	// SPR-14066
+	@Test
+	public void cookieManagerShared() throws Exception {
+		this.mockMvc = MockMvcBuilders.standaloneSetup(new CookieController()).build();
+		this.webClient = mockMvcSetup(this.mockMvc).build();
+
+		assertThat(getWebResponse("http://localhost/").getContentAsString(), equalTo(""));
+		this.webClient.getCookieManager().addCookie(new Cookie("localhost", "cookie", "cookieManagerShared"));
+		assertThat(getWebResponse("http://localhost/").getContentAsString(), equalTo("cookieManagerShared"));
+	}
+
 	private void assertMvcProcessed(String url) throws Exception {
 		assertThat(getWebResponse(url).getContentAsString(), equalTo("mvc"));
 	}
@@ -126,4 +139,11 @@ public class MockMvcWebClientBuilderTests {
 		}
 	}
 
+	@RestController
+	static class CookieController {
+		@RequestMapping("/")
+		public String cookie(@CookieValue("cookie") String cookie) {
+			return cookie;
+		}
+	}
 }

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java

@@ -36,6 +36,7 @@ import static org.junit.Assert.assertThat;
  * @author Rob Winch
  * @since 4.2
  */
+@SuppressWarnings("deprecation")
 public class MockMvcWebConnectionTests {
 
 	private final WebClient webClient = new WebClient();
@@ -50,7 +51,7 @@ public class MockMvcWebConnectionTests {
 
 	@Test
 	public void contextPathNull() throws IOException {
-		this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, null));
+		this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, (String) null));
 
 		Page page = this.webClient.getPage("http://localhost/context/a");
 

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java

@@ -35,11 +35,14 @@ import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.tests.Assume;
 import org.springframework.tests.TestGroup;
+import org.springframework.web.bind.annotation.CookieValue;
 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 com.gargoylesoftware.htmlunit.util.Cookie;
+
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 import static org.springframework.test.web.servlet.htmlunit.webdriver.MockMvcHtmlUnitDriverBuilder.*;
@@ -111,6 +114,20 @@ public class MockMvcHtmlUnitDriverBuilderTests {
 		assertFalse(this.driver.isJavascriptEnabled());
 	}
 
+	// SPR-14066
+	@Test
+	public void cookieManagerShared() throws Exception {
+		WebConnectionHtmlUnitDriver delegateDriver = new WebConnectionHtmlUnitDriver();
+		this.mockMvc = MockMvcBuilders.standaloneSetup(new CookieController()).build();
+		this.driver = mockMvcSetup(this.mockMvc)
+				.withDelegate(delegateDriver)
+				.build();
+
+		assertThat(get("http://localhost/"), equalTo(""));
+		delegateDriver.getWebClient().getCookieManager().addCookie(new Cookie("localhost", "cookie", "cookieManagerShared"));
+		assertThat(get("http://localhost/"), equalTo("cookieManagerShared"));
+	}
+
 	private void assertMvcProcessed(String url) throws Exception {
 		assertThat(get(url), containsString(EXPECTED_BODY));
 	}
@@ -139,4 +156,11 @@ public class MockMvcHtmlUnitDriverBuilderTests {
 		}
 	}
 
+	@RestController
+	static class CookieController {
+		@RequestMapping("/")
+		public String cookie(@CookieValue("cookie") String cookie) {
+			return cookie;
+		}
+	}
 }
\ No newline at end of file