From a2aef374c83b26a7dde29c64fc7c6adb414d3cc9 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 18 Mar 2016 09:09:17 -0500 Subject: [PATCH] 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 --- .../htmlunit/MockMvcWebClientBuilder.java | 2 +- .../htmlunit/MockMvcWebConnection.java | 40 ++++++++-- .../MockMvcWebConnectionBuilderSupport.java | 40 +++++++++- .../MockMvcHtmlUnitDriverBuilder.java | 2 +- .../WebConnectionHtmlUnitDriver.java | 8 +- .../MockMvcConnectionBuilderSupportTests.java | 77 +++++++++++++++++-- .../MockMvcWebClientBuilderTests.java | 20 +++++ .../htmlunit/MockMvcWebConnectionTests.java | 3 +- .../MockMvcHtmlUnitDriverBuilderTests.java | 24 ++++++ 9 files changed, 197 insertions(+), 19 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilder.java index 5dc12a21182b..94cc1ea86539 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilder.java @@ -106,7 +106,7 @@ public static MockMvcWebClientBuilder webAppContextSetup(WebApplicationContext c */ 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; } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java index d7a6fe60872e..184d0b7ec7d1 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java +++ b/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). *

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 MockMvcWebConnection(MockMvc mockMvc) { * 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). + *

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. + *

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"); diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionBuilderSupport.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionBuilderSupport.java index 6575e6a26eab..d60d2d4d1b25 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionBuilderSupport.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionBuilderSupport.java @@ -19,6 +19,7 @@ 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 T useMockMvcForHosts(String... hosts) { * @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 @@ protected final WebConnection createConnection(WebConnection defaultConnection) return new DelegatingWebConnection(defaultConnection, delegates); } - } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilder.java index 3b8ded716871..f8b88512bfe4 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilder.java @@ -129,7 +129,7 @@ public MockMvcHtmlUnitDriverBuilder javascriptEnabled(boolean javascriptEnabled) 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; } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/WebConnectionHtmlUnitDriver.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/WebConnectionHtmlUnitDriver.java index ab04baf25314..ba7a91752eae 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/webdriver/WebConnectionHtmlUnitDriver.java +++ b/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 void setWebConnection(WebConnection webConnection) { this.webClient.setWebConnection(webConnection); } + /** + * Gets the current {@link WebClient} + * @return the current {@link WebClient} + */ + public WebClient getWebClient() { + return this.webClient; + } } diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java index c89f4f40f9bc..9f74285074fa 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java @@ -36,6 +36,7 @@ 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.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 @@ @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 void constructorContextNull() { } @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 void mockMvc() throws Exception { 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 void mockMvcExampleDotCom() throws Exception { public void mockMvcAlwaysUseMockMvc() throws Exception { connection = new MockMvcWebConnectionBuilderSupport(wac) {} .alwaysUseMockMvc() - .createConnection(delegateConnection); + .createConnection(webClient); assertMvcProcessed("http://other.com/"); } @@ -123,7 +184,7 @@ public void mockMvcAlwaysUseMockMvc() throws Exception { @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 void defaultContextPathEmpty() throws Exception { @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")); } diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java index 0f68d9075f52..d07c86faffe9 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java @@ -34,6 +34,7 @@ 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 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 void mockMvcSetupWithCustomWebClientDelegate() throws Exception { 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 String contextPath(HttpServletRequest request) { } } + @RestController + static class CookieController { + @RequestMapping("/") + public String cookie(@CookieValue("cookie") String cookie) { + return cookie; + } + } } diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java index 726c4f0bb663..78e207b2e15b 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java @@ -36,6 +36,7 @@ * @author Rob Winch * @since 4.2 */ +@SuppressWarnings("deprecation") public class MockMvcWebConnectionTests { private final WebClient webClient = new WebClient(); @@ -50,7 +51,7 @@ public void setup() { @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"); diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java index 9fe37224bc2e..3a1bf27a373c 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java @@ -35,11 +35,14 @@ 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 void javaScriptDisabled() { 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 String contextPath(HttpServletRequest request) { } } + @RestController + static class CookieController { + @RequestMapping("/") + public String cookie(@CookieValue("cookie") String cookie) { + return cookie; + } + } } \ No newline at end of file