From 1eaa9cd2f442621d248abe5f83b7d60e0c0cd145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 18 Jun 2024 15:44:23 +0200 Subject: [PATCH] Polish --- .../web/servlet/assertj/MockMvcTester.java | 93 ++++++++++++------- .../MockMvcTesterIntegrationTests.java | 31 +++++-- .../servlet/assertj/MockMvcTesterTests.java | 69 ++++++++++++++ 3 files changed, 149 insertions(+), 44 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java index 5c93c783e20..4cbb61e92e4 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java @@ -61,8 +61,8 @@ import org.springframework.web.context.WebApplicationContext; * builder -> builder.addFilters(filter).build()); * * - *

A tester can be created in standalone mode by providing the controller(s) - * to include:


+ * 
A tester can be created in standalone mode by providing the controller
+ * instances to include:

  * // Create an instance for PersonController
  * MockMvcTester mvc = MockMvcTester.of(new PersonController());
  * 

@@ -74,7 +74,7 @@ import org.springframework.web.context.WebApplicationContext;
  * assertThat(mvc.get().uri("/hi")).hasStatusOk().hasBodyTextEqualTo("Hello");
  *
* - *

For more complex scenarios the {@linkplain MvcTestResult result} of the + *

For more complex scenarios the {@linkplain MvcTestResult result} of the * exchange can be assigned in a variable to run multiple assertions: * 


  * // perform a POST on /save and assert the response body is empty
@@ -83,6 +83,16 @@ import org.springframework.web.context.WebApplicationContext;
  * assertThat(result).body().isEmpty();
  *
* + *

If the request is processing asynchronously, {@code exchange} waits for + * its completion, either using the + * {@linkplain org.springframework.mock.web.MockAsyncContext#setTimeout default + * timeout} or a given one. If you prefer to get the result of an + * asynchronous request immediately, use {@code asyncExchange}: + * 


+ * // perform a POST on /save and assert an asynchronous request has started
+ * assertThat(mvc.post().uri("/save").asyncExchange()).request().hasAsyncStarted();
+ *
+ * *

You can also perform requests using the static builders approach that * {@link MockMvc} uses. For instance:


  * // perform a GET on /hi and assert the response body is equal to Hello
@@ -90,6 +100,10 @@ import org.springframework.web.context.WebApplicationContext;
  *         .hasStatusOk().hasBodyTextEqualTo("Hello");
  *
* + *

Use this approach if you have a custom {@link RequestBuilder} implementation + * that you'd like to integrate here. This approach is also invoking {@link MockMvc} + * without any additional processing of asynchronous requests. + * *

One main difference between {@link MockMvc} and {@code MockMvcTester} is * that an unresolved exception is not thrown directly when using * {@code MockMvcTester}. Rather an {@link MvcTestResult} is available with an @@ -231,8 +245,10 @@ public final class MockMvcTester { * Prepare an HTTP GET request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder get() { @@ -243,8 +259,10 @@ public final class MockMvcTester { * Prepare an HTTP HEAD request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder head() { @@ -255,8 +273,10 @@ public final class MockMvcTester { * Prepare an HTTP POST request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder post() { @@ -267,8 +287,10 @@ public final class MockMvcTester { * Prepare an HTTP PUT request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder put() { @@ -279,8 +301,10 @@ public final class MockMvcTester { * Prepare an HTTP PATCH request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder patch() { @@ -291,8 +315,10 @@ public final class MockMvcTester { * Prepare an HTTP DELETE request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder delete() { @@ -303,8 +329,10 @@ public final class MockMvcTester { * Prepare an HTTP OPTIONS request. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder options() { @@ -315,8 +343,10 @@ public final class MockMvcTester { * Prepare a request for the specified {@code HttpMethod}. *

The returned builder can be wrapped in {@code assertThat} to enable * assertions on the result. For multi-statements assertions, use - * {@linkplain MockMvcRequestBuilder#exchange() exchange} to assign the - * result. + * {@link MockMvcRequestBuilder#exchange() exchange()} to assign the + * result. To control the time to wait for asynchronous request to complete + * on a per-request basis, use + * {@link MockMvcRequestBuilder#exchange(Duration) exchange(Duration)}. * @return a request builder for specifying the target URI */ public MockMvcRequestBuilder method(HttpMethod method) { @@ -324,17 +354,13 @@ public final class MockMvcTester { } /** - * Perform a request using {@link MockMvcRequestBuilders} and return a + * Perform a request using the given {@link RequestBuilder} and return a * {@link MvcTestResult result} that can be used with standard * {@link org.assertj.core.api.Assertions AssertJ} assertions. - *

Use static methods of {@link MockMvcRequestBuilders} to prepare the - * request, wrapping the invocation in {@code assertThat}. The following - * asserts that a {@linkplain MockMvcRequestBuilders#get(URI) GET} request - * against "/greet" has an HTTP status code 200 (OK) and a simple body: - * 

assertThat(mvc.perform(get("/greet")))
-	 *       .hasStatusOk()
-	 *       .body().asString().isEqualTo("Hello");
-	 *
+ *

Use only this method if you need to provide a custom + * {@link RequestBuilder}. For regular cases, users should initiate the + * configuration of the request using one of the methods available on + * this instance, e.g. {@link #get()} for HTTP GET. *

Contrary to {@link MockMvc#perform(RequestBuilder)}, this does not * throw an exception if the request fails with an unresolved exception. * Rather, the result provides the exception, if any. Assuming that a @@ -342,17 +368,14 @@ public final class MockMvcTester { * {@code /boom} throws an {@code IllegalStateException}, the following * asserts that the invocation has indeed failed with the expected error * message: - * 

assertThat(mvc.perform(post("/boom")))
-	 *       .unresolvedException().isInstanceOf(IllegalStateException.class)
+	 * 
assertThat(mvc.post().uri("/boom")))
+	 *       .failure().isInstanceOf(IllegalStateException.class)
 	 *       .hasMessage("Expected");
 	 * 

-	 * @param requestBuilder used to prepare the request to execute;
-	 * see static factory methods in
-	 * {@link org.springframework.test.web.servlet.request.MockMvcRequestBuilders}
+	 * @param requestBuilder used to prepare the request to execute
 	 * @return an {@link MvcTestResult} to be wrapped in {@code assertThat}
 	 * @see MockMvc#perform(RequestBuilder)
-	 * @see #get()
-	 * @see #post()
+	 * @see #method(HttpMethod)
 	 */
 	public MvcTestResult perform(RequestBuilder requestBuilder) {
 		Object result = getMvcResultOrFailure(requestBuilder);
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
index 90ceeaa1b8b..7175bc25c64 100644
--- a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
+++ b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
@@ -97,6 +97,10 @@ import static org.mockito.Mockito.verifyNoInteractions;
 @SpringJUnitWebConfig
 public class MockMvcTesterIntegrationTests {
 
+	private static final MockMultipartFile file = new MockMultipartFile("file", "content.txt", null,
+			"value".getBytes(StandardCharsets.UTF_8));
+
+
 	private final MockMvcTester mvc;
 
 	MockMvcTesterIntegrationTests(WebApplicationContext wac) {
@@ -106,9 +110,6 @@ public class MockMvcTesterIntegrationTests {
 	@Nested
 	class PerformTests {
 
-		private final MockMultipartFile file = new MockMultipartFile("file", "content.txt", null,
-				"value".getBytes(StandardCharsets.UTF_8));
-
 		@Test
 		void syncRequestWithDefaultExchange() {
 			assertThat(mvc.get().uri("/greet")).hasStatusOk();
@@ -123,7 +124,7 @@ public class MockMvcTesterIntegrationTests {
 		@Test
 		void asyncMultipartRequestWithDefaultExchange() {
 			assertThat(mvc.post().uri("/multipart-streaming").multipart()
-					.file(this.file).param("timeToWait", "100"))
+					.file(file).param("timeToWait", "100"))
 					.hasStatusOk().hasBodyTextEqualTo("name=Joe&file=content.txt");
 		}
 
@@ -141,7 +142,7 @@ public class MockMvcTesterIntegrationTests {
 		@Test
 		void asyncMultipartRequestWitExplicitExchange() {
 			assertThat(mvc.post().uri("/multipart-streaming").multipart()
-					.file(this.file).param("timeToWait", "100").exchange())
+					.file(file).param("timeToWait", "100").exchange())
 					.hasStatusOk().hasBodyTextEqualTo("name=Joe&file=content.txt");
 		}
 
@@ -161,7 +162,7 @@ public class MockMvcTesterIntegrationTests {
 		@Test
 		void asyncMultipartRequestWithExplicitExchangeAndEnoughTimeToWait() {
 			assertThat(mvc.post().uri("/multipart-streaming").multipart()
-					.file(this.file).param("timeToWait", "100").exchange(Duration.ofMillis(200)))
+					.file(file).param("timeToWait", "100").exchange(Duration.ofMillis(200)))
 					.hasStatusOk().hasBodyTextEqualTo("name=Joe&file=content.txt");
 		}
 
@@ -176,7 +177,7 @@ public class MockMvcTesterIntegrationTests {
 		@Test
 		void asyncMultipartRequestWithExplicitExchangeAndNotEnoughTimeToWait() {
 			MockMultipartMvcRequestBuilder builder = mvc.post().uri("/multipart-streaming").multipart()
-					.file(this.file).param("timeToWait", "500");
+					.file(file).param("timeToWait", "500");
 			assertThatIllegalStateException()
 					.isThrownBy(() -> builder.exchange(Duration.ofMillis(100)))
 					.withMessageContaining("was not set during the specified timeToWait=100");
@@ -192,11 +193,24 @@ public class MockMvcTesterIntegrationTests {
 					.request().hasAsyncStarted(true);
 		}
 
+		@Test
+		void hasAsyncStartedForMultipartTrue() {
+			assertThat(mvc.post().uri("/multipart-streaming").multipart()
+					.file(file).param("timeToWait", "100").asyncExchange())
+					.request().hasAsyncStarted(true);
+		}
+
 		@Test
 		void hasAsyncStartedFalse() {
 			assertThat(mvc.get().uri("/greet").asyncExchange()).request().hasAsyncStarted(false);
 		}
 
+		@Test
+		void hasAsyncStartedForMultipartFalse() {
+			assertThat(mvc.put().uri("/multipart-put").multipart().file(file).asyncExchange())
+					.request().hasAsyncStarted(false);
+		}
+
 		@Test
 		void attributes() {
 			assertThat(mvc.get().uri("/greet")).request().attributes()
@@ -220,8 +234,7 @@ public class MockMvcTesterIntegrationTests {
 
 		@Test
 		void multipartWithPut() {
-			MockMultipartFile part = new MockMultipartFile("file", "content.txt", null, "value".getBytes(StandardCharsets.UTF_8));
-			assertThat(mvc.put().uri("/multipart-put").multipart().file(part).file(JSON_PART_FILE))
+			assertThat(mvc.put().uri("/multipart-put").multipart().file(file).file(JSON_PART_FILE))
 					.hasStatusOk()
 					.hasViewName("index")
 					.model().contains(entry("name", "file"));
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterTests.java
index a8f17e6cf62..fef8bd7caa8 100644
--- a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterTests.java
+++ b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterTests.java
@@ -19,19 +19,25 @@ package org.springframework.test.web.servlet.assertj;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletException;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.cglib.core.internal.Function;
 import org.springframework.context.annotation.AnnotationConfigUtils;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockServletContext;
 import org.springframework.test.json.AbstractJsonContentAssert;
+import org.springframework.test.web.servlet.assertj.MockMvcTester.MockMvcRequestBuilder;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RestController;
@@ -57,6 +63,8 @@ class MockMvcTesterTests {
 	private static final MappingJackson2HttpMessageConverter jsonHttpMessageConverter =
 			new MappingJackson2HttpMessageConverter(new ObjectMapper());
 
+	private final ServletContext servletContext = new MockServletContext();
+
 
 	@Test
 	void createShouldRejectNullMockMvc() {
@@ -139,6 +147,67 @@ class MockMvcTesterTests {
 		assertThat(result).hasFieldOrPropertyWithValue("mvcResult", null);
 	}
 
+	@Test
+	void getConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.get().uri("/hello")))
+				.satisfies(hasSettings(HttpMethod.GET, "/hello"));
+	}
+
+	@Test
+	void headConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.head().uri("/download")))
+				.satisfies(hasSettings(HttpMethod.HEAD, "/download"));
+	}
+
+	@Test
+	void postConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.post().uri("/save")))
+				.satisfies(hasSettings(HttpMethod.POST, "/save"));
+	}
+
+	@Test
+	void putConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.put().uri("/save")))
+				.satisfies(hasSettings(HttpMethod.PUT, "/save"));
+	}
+
+	@Test
+	void patchConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.patch().uri("/update")))
+				.satisfies(hasSettings(HttpMethod.PATCH, "/update"));
+	}
+
+	@Test
+	void deleteConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.delete().uri("/users/42")))
+				.satisfies(hasSettings(HttpMethod.DELETE, "/users/42"));
+	}
+
+	@Test
+	void optionsConfiguresBuilder() {
+		assertThat(createMockHttpServletRequest(tester -> tester.options().uri("/users")))
+				.satisfies(hasSettings(HttpMethod.OPTIONS, "/users"));
+	}
+
+	@Test
+	void methodConfiguresBuilderWithCustomMethod() {
+		HttpMethod customMethod = HttpMethod.valueOf("CUSTOM");
+		assertThat(createMockHttpServletRequest(tester -> tester.method(customMethod).uri("/hello")))
+				.satisfies(hasSettings(customMethod, "/hello"));
+	}
+
+	private MockHttpServletRequest createMockHttpServletRequest(Function builder) {
+		MockMvcTester mockMvcTester = MockMvcTester.of(HelloController.class);
+		return builder.apply(mockMvcTester).buildRequest(this.servletContext);
+	}
+
+	private Consumer hasSettings(HttpMethod method, String uri) {
+		return request -> {
+			assertThat(request.getMethod()).isEqualTo(method.name());
+			assertThat(request.getRequestURI()).isEqualTo(uri);
+		};
+	}
+
 	private GenericWebApplicationContext create(Class... classes) {
 		GenericWebApplicationContext applicationContext = new GenericWebApplicationContext(new MockServletContext());
 		AnnotationConfigUtils.registerAnnotationConfigProcessors(applicationContext);