Polish HttpHiddenMethodFilter

spring-web/src/main/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilter.java

@@ -37,29 +37,33 @@ import org.springframework.web.server.WebFilterChain;
  * return value using {@link ServerWebExchange#mutate()}.
  *
  * <p>The name of the request parameter defaults to {@code _method}, but can be
- * adapted via the {@link #setMethodParam(String) methodParam} property.
+ * adapted via the {@link #setMethodParamName(String) methodParamName} property.
  *
  * @author Greg Turnquist
+ * @author Rossen Stoyanchev
  * @since 5.0
  */
 public class HiddenHttpMethodFilter implements WebFilter {
 
-	/** Default method parameter: {@code _method} */
-	public static final String DEFAULT_METHOD_PARAM = "_method";
+	/** Default name of the form parameter with the HTTP method to use */
+	public static final String DEFAULT_METHOD_PARAMETER_NAME = "_method";
+
+
+	private String methodParamName = DEFAULT_METHOD_PARAMETER_NAME;
 
-	private String methodParam = DEFAULT_METHOD_PARAM;
 
 	/**
-	 * Set the parameter name to look for HTTP methods.
-	 * @see #DEFAULT_METHOD_PARAM
+	 * Set the name of the form parameter with the HTTP method to use.
+	 * <p>By default this is set to {@code "_method"}.
 	 */
-	public void setMethodParam(String methodParam) {
-		Assert.hasText(methodParam, "'methodParam' must not be empty");
-		this.methodParam = methodParam;
+	public void setMethodParamName(String methodParamName) {
+		Assert.hasText(methodParamName, "'methodParamName' must not be empty");
+		this.methodParamName = methodParamName;
 	}
 
+
 	/**
-	 * Transform an HTTP POST into another method based on {@code methodParam}
+	 * Transform an HTTP POST into another method based on {@code methodParamName}
 	 *
 	 * @param exchange the current server exchange
 	 * @param chain provides a way to delegate to the next filter
@@ -68,36 +72,22 @@ public class HiddenHttpMethodFilter implements WebFilter {
 	@Override
 	public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
 
-		if (exchange.getRequest().getMethod() == HttpMethod.POST) {
-			return exchange.getFormData()
-					.map(formData -> {
-						String method = formData.getFirst(methodParam);
-						if (StringUtils.hasLength(method)) {
-							return convertedRequest(exchange, method);
-						}
-						else {
-							return exchange;
-						}
-					})
-					.then(convertedExchange -> chain.filter(convertedExchange));
-		}
-		else {
+		if (exchange.getRequest().getMethod() != HttpMethod.POST) {
 			return chain.filter(exchange);
 		}
+
+		return exchange.getFormData()
+				.map(formData -> {
+					String method = formData.getFirst(this.methodParamName);
+					return StringUtils.hasLength(method) ? mapExchange(exchange, method) : exchange;
+				})
+				.then((exchange1) -> chain.filter(exchange1));
 	}
 
-	/**
-	 * Mutate exchange into a new HTTP request method.
-	 *
-	 * @param exchange original {@link ServerWebExchange}
-	 * @param method request HTTP method based on form data
-	 * @return a mutated {@link ServerWebExchange}
-	 */
-	private ServerWebExchange convertedRequest(ServerWebExchange exchange, String method) {
-		HttpMethod resolved = HttpMethod.resolve(method.toUpperCase(Locale.ENGLISH));
-		Assert.notNull(resolved, () -> "HttpMethod '" + method + "' is not supported");
-		return exchange.mutate()
-				.request(builder -> builder.method(resolved))
-				.build();
+	private ServerWebExchange mapExchange(ServerWebExchange exchange, String methodParamValue) {
+		HttpMethod httpMethod = HttpMethod.resolve(methodParamValue.toUpperCase(Locale.ENGLISH));
+		Assert.notNull(httpMethod, () -> "HttpMethod '" + methodParamValue + "' not supported");
+		return exchange.mutate().request(builder -> builder.method(httpMethod)).build();
 	}
+
 }

spring-web/src/main/java/org/springframework/web/filter/reactive/package-info.java

@@ -0,0 +1,5 @@
+/**
+ * {@link org.springframework.web.server.WebFilter} implementations for use in
+ * reactive web applications.
+ */
+package org.springframework.web.filter.reactive;

spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java

@@ -104,8 +104,10 @@ public class DefaultServerWebExchange implements ServerWebExchange {
 		try {
 			contentType = request.getHeaders().getContentType();
 			if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
-				Map<String, Object> hints = Collections.emptyMap();
-				return FORM_READER.readMono(FORM_DATA_VALUE_TYPE, request, hints).cache();
+				return FORM_READER
+						.readMono(FORM_DATA_VALUE_TYPE, request, Collections.emptyMap())
+						.otherwiseIfEmpty(EMPTY_FORM_DATA)
+						.cache();
 			}
 		}
 		catch (InvalidMediaTypeException ex) {

spring-web/src/test/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilterTests.java

@@ -16,7 +16,7 @@
 
 package org.springframework.web.filter.reactive;
 
-import java.util.Optional;
+import java.time.Duration;
 
 import org.hamcrest.Matchers;
 import org.junit.Test;
@@ -27,6 +27,7 @@ import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
+import org.springframework.mock.http.server.reactive.test.MockServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebFilterChain;
 
@@ -34,116 +35,90 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 
 /**
- * Tests for {@link HiddenHttpMethodFilter}
- *
+ * Tests for {@link HiddenHttpMethodFilter}.
  * @author Greg Turnquist
+ * @author Rossen Stoyanchev
  */
 public class HiddenHttpMethodFilterTests {
 
 	private final HiddenHttpMethodFilter filter = new HiddenHttpMethodFilter();
 
+	private final TestWebFilterChain filterChain = new TestWebFilterChain();
+
+
 	@Test
 	public void filterWithParameter() {
-		ServerWebExchange mockExchange = createExchange(Optional.of("DELETE"));
-
-		WebFilterChain filterChain = exchange -> {
-			assertEquals("Invalid method", HttpMethod.DELETE, exchange.getRequest().getMethod());
-			return Mono.empty();
-		};
+		postForm("_method=DELETE").block(Duration.ZERO);
+		assertEquals(HttpMethod.DELETE, this.filterChain.getHttpMethod());
+	}
 
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
-				.expectComplete()
-				.verify();
+	@Test
+	public void filterWithNoParameter() {
+		postForm("").block(Duration.ZERO);
+		assertEquals(HttpMethod.POST, this.filterChain.getHttpMethod());
 	}
 
 	@Test
-	public void filterWithInvalidParameter() {
-		ServerWebExchange mockExchange = createExchange(Optional.of("INVALID"));
+	public void filterWithEmptyStringParameter() {
+		postForm("_method=").block(Duration.ZERO);
+		assertEquals(HttpMethod.POST, this.filterChain.getHttpMethod());
+	}
 
-		WebFilterChain filterChain = exchange -> Mono.empty();
+	@Test
+	public void filterWithDifferentMethodParam() {
+		this.filter.setMethodParamName("_foo");
+		postForm("_foo=DELETE").block(Duration.ZERO);
+		assertEquals(HttpMethod.DELETE, this.filterChain.getHttpMethod());
+	}
 
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
+	@Test
+	public void filterWithInvalidMethodValue() {
+		StepVerifier.create(postForm("_method=INVALID"))
 				.consumeErrorWith(error -> {
 					assertThat(error, Matchers.instanceOf(IllegalArgumentException.class));
-					assertEquals(error.getMessage(), "HttpMethod 'INVALID' is not supported");
+					assertEquals(error.getMessage(), "HttpMethod 'INVALID' not supported");
 				})
 				.verify();
 	}
 
 	@Test
-	public void filterWithNoParameter() {
-		ServerWebExchange mockExchange = createExchange(Optional.empty());
+	public void filterWithHttpPut() {
 
-		WebFilterChain filterChain = exchange -> {
-			assertEquals("Invalid method", HttpMethod.POST, exchange.getRequest().getMethod());
-			return Mono.empty();
-		};
+		ServerWebExchange exchange = MockServerHttpRequest.put("/")
+				.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
+				.body("_method=DELETE")
+				.toExchange();
 
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
-				.expectComplete()
-				.verify();
+		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
+		assertEquals(HttpMethod.PUT, this.filterChain.getHttpMethod());
 	}
 
-	@Test
-	public void filterWithEmptyStringParameter() {
-		ServerWebExchange mockExchange = createExchange(Optional.of(""));
-
-		WebFilterChain filterChain = exchange -> {
-			assertEquals("Invalid method", HttpMethod.POST, exchange.getRequest().getMethod());
-			return Mono.empty();
-		};
-
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
-				.expectComplete()
-				.verify();
-	}
-
-	@Test
-	public void filterWithDifferentMethodParam() {
-		ServerWebExchange mockExchange = createExchange("_foo", Optional.of("DELETE"));
 
-		WebFilterChain filterChain = exchange -> {
-			assertEquals("Invalid method", HttpMethod.DELETE, exchange.getRequest().getMethod());
-			return Mono.empty();
-		};
+	private Mono<Void> postForm(String body) {
 
-		filter.setMethodParam("_foo");
+		MockServerWebExchange exchange = MockServerHttpRequest.post("/")
+				.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
+				.body(body)
+				.toExchange();
 
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
-				.expectComplete()
-				.verify();
+		return this.filter.filter(exchange, this.filterChain);
 	}
 
-	@Test
-	public void filterWithoutPost() {
-		ServerWebExchange mockExchange = createExchange(Optional.of("DELETE")).mutate()
-				.request(builder -> builder.method(HttpMethod.PUT))
-				.build();
 
-		WebFilterChain filterChain = exchange -> {
-			assertEquals("Invalid method", HttpMethod.PUT, exchange.getRequest().getMethod());
-			return Mono.empty();
-		};
+	private static class TestWebFilterChain implements WebFilterChain {
 
-		StepVerifier.create(filter.filter(mockExchange, filterChain))
-				.expectComplete()
-				.verify();
-	}
-
-	private ServerWebExchange createExchange(Optional<String> optionalMethod) {
-		return createExchange("_method", optionalMethod);
-	}
+		private HttpMethod httpMethod;
 
-	private ServerWebExchange createExchange(String methodName, Optional<String> optionalBody) {
-		MockServerHttpRequest.BodyBuilder builder = MockServerHttpRequest
-				.post("/hotels")
-				.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE);
 
-		MockServerHttpRequest request = optionalBody
-				.map(method -> builder.body(methodName + "=" + method))
-				.orElse(builder.build());
+		public HttpMethod getHttpMethod() {
+			return this.httpMethod;
+		}
 
-		return request.toExchange();
+		@Override
+		public Mono<Void> filter(ServerWebExchange exchange) {
+			this.httpMethod = exchange.getRequest().getMethod();
+			return Mono.empty();
+		}
 	}
 
 }