Remove ServerWebExchange#getRequestParams

Issue: SPR-15508

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java

@@ -16,18 +16,17 @@
 
 package org.springframework.web.bind.support;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
 import reactor.core.publisher.Mono;
 
 import org.springframework.beans.MutablePropertyValues;
+import org.springframework.http.codec.multipart.Part;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.bind.WebDataBinder;
-import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
@@ -39,6 +38,7 @@
  */
 public class WebExchangeDataBinder extends WebDataBinder {
 
+
 	/**
 	 * Create a new instance, with default object name.
 	 * @param target the target object to bind onto (or {@code null} if the
@@ -61,62 +61,52 @@ public WebExchangeDataBinder(Object target, String objectName) {
 
 
 	/**
-	 * Bind the URL query parameters or form data of the body of the given request
-	 * to this binder's target. The request body is parsed if the Content-Type
-	 * is {@code "application/x-www-form-urlencoded"}.
+	 * Bind query params, form data, and or multipart form data to the binder target.
 	 * @param exchange the current exchange.
 	 * @return a {@code Mono<Void>} when binding is complete
 	 */
 	public Mono<Void> bind(ServerWebExchange exchange) {
-		return exchange.getRequestParams()
-				.map(this::getParamsToBind)
-				.doOnNext(values -> values.putAll(getMultipartFiles(exchange)))
-				.doOnNext(values -> values.putAll(getExtraValuesToBind(exchange)))
-				.flatMap(values -> {
-					doBind(new MutablePropertyValues(values));
-					return Mono.empty();
-				});
-	}
-
-	private Map<String, Object> getParamsToBind(MultiValueMap<String, String> params) {
-		Map<String, Object> result = new TreeMap<>();
-		for (Map.Entry<String, List<String>> entry : params.entrySet()) {
-			String name = entry.getKey();
-			List<String> values = entry.getValue();
-			if (CollectionUtils.isEmpty(values)) {
-				// Do nothing, no values found at all.
-			}
-			else if (values.size() == 1) {
-				result.put(name, values.get(0));
-			}
-			else {
-				result.put(name, values);
-			}
-		}
-		return result;
+		return getValuesToBind(exchange)
+				.doOnNext(values -> doBind(new MutablePropertyValues(values)))
+				.then();
 	}
 
 	/**
-	 * Bind all multipart files contained in the given request, if any (in case
-	 * of a multipart request).
-	 * <p>Multipart files will only be added to the property values if they
-	 * are not empty or if we're configured to bind empty multipart files too.
-	 * @param exchange the current exchange
-	 * @return Map of field name String to MultipartFile object
+	 * Protected method to obtain the values for data binding. By default this
+	 * method delegates to {@link #extractValuesToBind(ServerWebExchange)}.
 	 */
-	protected Map<String, List<MultipartFile>> getMultipartFiles(ServerWebExchange exchange) {
-		// TODO
-		return Collections.emptyMap();
+	protected Mono<Map<String, Object>> getValuesToBind(ServerWebExchange exchange) {
+		return extractValuesToBind(exchange);
 	}
 
 	/**
-	 * Extension point that subclasses can use to add extra bind values for a
-	 * request. Invoked before {@link #doBind(MutablePropertyValues)}.
-	 * <p>The default implementation is empty.
+	 * Combine query params and form data for multipart form data from the body
+	 * of the request into a {@code Map<String, Object>} of values to use for
+	 * data binding purposes.
+	 *
 	 * @param exchange the current exchange
+	 * @return a {@code Mono} with the values to bind
 	 */
-	protected Map<String, ?> getExtraValuesToBind(ServerWebExchange exchange) {
-		return Collections.emptyMap();
+	public static Mono<Map<String, Object>> extractValuesToBind(ServerWebExchange exchange) {
+
+		MultiValueMap<String, String> queryParams = exchange.getRequest().getQueryParams();
+		Mono<MultiValueMap<String, String>> formData = exchange.getFormData();
+		Mono<MultiValueMap<String, Part>> multipartData = exchange.getMultipartData();
+
+		return Mono.when(Mono.just(queryParams), formData, multipartData)
+				.map(tuple -> {
+					Map<String, Object> result = new TreeMap<>();
+					tuple.getT1().forEach((key, values) -> addBindValue(result, key, values));
+					tuple.getT2().forEach((key, values) -> addBindValue(result, key, values));
+					tuple.getT3().forEach((key, values) -> addBindValue(result, key, values));
+					return result;
+				});
+	}
+
+	private static void addBindValue(Map<String, Object> params, String key, List<?> values) {
+		if (!CollectionUtils.isEmpty(values)) {
+			params.put(key, values.size() == 1 ? values.get(0) : values);
+		}
 	}
 
 }

spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -80,24 +80,23 @@ public interface ServerWebExchange {
 	/**
 	 * Return the form data from the body of the request if the Content-Type is
 	 * {@code "application/x-www-form-urlencoded"} or an empty map otherwise.
-	 * This method may be called multiple times.
+	 *
+	 * <p><strong>Note:</strong> calling this method causes the request body to
+	 * be read and parsed in full and the resulting {@code MultiValueMap} is
+	 * cached so that this method is safe to call more than once.
 	 */
 	Mono<MultiValueMap<String, String>> getFormData();
 
 	/**
 	 * Return the parts of a multipart request if the Content-Type is
 	 * {@code "multipart/form-data"} or an empty map otherwise.
-	 * This method may be called multiple times.
+	 *
+	 * <p><strong>Note:</strong> calling this method causes the request body to
+	 * be read and parsed in full and the resulting {@code MultiValueMap} is
+	 * cached so that this method is safe to call more than once.
 	 */
 	Mono<MultiValueMap<String, Part>> getMultipartData();
 
-	/**
-	 * Return a combined map that represents both
-	 * {@link ServerHttpRequest#getQueryParams()} and {@link #getFormData()}
-	 * or an empty map.
-	 */
-	Mono<MultiValueMap<String, String>> getRequestParams();
-
 	/**
 	 * Returns {@code true} if the one of the {@code checkNotModified} methods
 	 * in this contract were used and they returned true.

spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java

@@ -99,11 +99,6 @@ public Mono<MultiValueMap<String, Part>> getMultipartData() {
 		return getDelegate().getMultipartData();
 	}
 
-	@Override
-	public Mono<MultiValueMap<String, String>> getRequestParams() {
-		return getDelegate().getRequestParams();
-	}
-
 	@Override
 	public boolean isNotModified() {
 		return getDelegate().isNotModified();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -88,8 +88,6 @@ public class DefaultServerWebExchange implements ServerWebExchange {
 
 	private final Mono<MultiValueMap<String, Part>> multipartDataMono;
 
-	private final Mono<MultiValueMap<String, String>> requestParamsMono;
-
 	private volatile boolean notModified;
 
 
@@ -109,8 +107,6 @@ public DefaultServerWebExchange(ServerHttpRequest request, ServerHttpResponse re
 		this.sessionMono = sessionManager.getSession(this).cache();
 		this.formDataMono = initFormData(request, codecConfigurer);
 		this.multipartDataMono = initMultipartData(request, codecConfigurer);
-		this.requestParamsMono = initRequestParams(request, this.formDataMono);
-
 	}
 
 	@SuppressWarnings("unchecked")
@@ -165,20 +161,6 @@ private static Mono<MultiValueMap<String, Part>> initMultipartData(
 		return EMPTY_MULTIPART_DATA;
 	}
 
-	private static Mono<MultiValueMap<String, String>> initRequestParams(
-			ServerHttpRequest request, Mono<MultiValueMap<String, String>> formDataMono) {
-
-		return formDataMono
-				.map(formData -> {
-					MultiValueMap<String, String> result = new LinkedMultiValueMap<>();
-					result.putAll(request.getQueryParams());
-					result.putAll(formData);
-					return CollectionUtils.unmodifiableMultiValueMap(result);
-				})
-				.defaultIfEmpty(request.getQueryParams())
-				.cache();
-	}
-
 
 	@Override
 	public ServerHttpRequest getRequest() {
@@ -228,11 +210,6 @@ public Mono<MultiValueMap<String, Part>> getMultipartData() {
 		return this.multipartDataMono;
 	}
 
-	@Override
-	public Mono<MultiValueMap<String, String>> getRequestParams() {
-		return this.requestParamsMono;
-	}
-
 	@Override
 	public boolean isNotModified() {
 		return this.notModified;

spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java

@@ -21,8 +21,6 @@
 import java.util.LinkedHashSet;
 import java.util.Set;
 
-import org.springframework.util.Assert;
-import org.springframework.util.MultiValueMap;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -143,18 +141,12 @@ protected String parseValue(String valueExpression) {
 
 		@Override
 		protected boolean matchName(ServerWebExchange exchange) {
-			return getRequestParams(exchange).containsKey(this.name);
+			return exchange.getRequest().getQueryParams().containsKey(this.name);
 		}
 
 		@Override
 		protected boolean matchValue(ServerWebExchange exchange) {
-			return this.value.equals(getRequestParams(exchange).getFirst(this.name));
-		}
-
-		private MultiValueMap<String, String> getRequestParams(ServerWebExchange exchange) {
-			MultiValueMap<String, String> params = exchange.getRequestParams().subscribe().peek();
-			Assert.notNull(params, "Expected form data (if any) to be parsed.");
-			return params;
+			return this.value.equals(exchange.getRequest().getQueryParams().getFirst(this.name));
 		}
 	}
 

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java

@@ -258,32 +258,26 @@ public Mono<HandlerMethod> getHandlerInternal(ServerWebExchange exchange) {
 			logger.debug("Looking up handler method for path " + lookupPath);
 		}
 		this.mappingRegistry.acquireReadLock();
-
 		try {
-			// Ensure form data is parsed for "params" conditions...
-			return exchange.getRequestParams()
-					.then(exchange.getMultipartData())
-					.then(Mono.defer(() -> {
-						HandlerMethod handlerMethod = null;
-						try {
-							handlerMethod = lookupHandlerMethod(lookupPath, exchange);
-						}
-						catch (Exception ex) {
-							return Mono.error(ex);
-						}
-						if (logger.isDebugEnabled()) {
-							if (handlerMethod != null) {
-								logger.debug("Returning handler method [" + handlerMethod + "]");
-							}
-							else {
-								logger.debug("Did not find handler method for [" + lookupPath + "]");
-							}
-						}
-						if (handlerMethod != null) {
-							handlerMethod = handlerMethod.createWithResolvedBean();
-						}
-						return Mono.justOrEmpty(handlerMethod);
-					}));
+			HandlerMethod handlerMethod;
+			try {
+				handlerMethod = lookupHandlerMethod(lookupPath, exchange);
+			}
+			catch (Exception ex) {
+				return Mono.error(ex);
+			}
+			if (logger.isDebugEnabled()) {
+				if (handlerMethod != null) {
+					logger.debug("Returning handler method [" + handlerMethod + "]");
+				}
+				else {
+					logger.debug("Did not find handler method for [" + lookupPath + "]");
+				}
+			}
+			if (handlerMethod != null) {
+				handlerMethod = handlerMethod.createWithResolvedBean();
+			}
+			return Mono.justOrEmpty(handlerMethod);
 		}
 		finally {
 			this.mappingRegistry.releaseReadLock();

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java

@@ -133,8 +133,8 @@ private void addResolversTo(ArgumentResolverRegistrar registrar,
 
 		// Annotation-based...
 		registrar.add(new RequestParamMethodArgumentResolver(beanFactory, reactiveRegistry, false));
-		registrar.add(new RequestPartMethodArgumentResolver(beanFactory, reactiveRegistry, false));
 		registrar.add(new RequestParamMapMethodArgumentResolver(reactiveRegistry));
+		registrar.add(new RequestPartMethodArgumentResolver(reactiveRegistry));
 		registrar.add(new PathVariableMethodArgumentResolver(beanFactory, reactiveRegistry));
 		registrar.add(new PathVariableMapMethodArgumentResolver(reactiveRegistry));
 		registrar.addIfRequestBody(readers -> new RequestBodyArgumentResolver(readers, reactiveRegistry));

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java

@@ -216,7 +216,7 @@ private Mono<?> createAttribute(
 		}
 
 		// A single data class constructor -> resolve constructor arguments from request parameters.
-		return exchange.getRequestParams().map(requestParams -> {
+		return WebExchangeDataBinder.extractValuesToBind(exchange).map(bindValues -> {
 			ConstructorProperties cp = ctor.getAnnotation(ConstructorProperties.class);
 			String[] paramNames = (cp != null ? cp.value() : parameterNameDiscoverer.getParameterNames(ctor));
 			Assert.state(paramNames != null, () -> "Cannot resolve parameter names for constructor " + ctor);
@@ -226,13 +226,9 @@ private Mono<?> createAttribute(
 			Object[] args = new Object[paramTypes.length];
 			WebDataBinder binder = context.createDataBinder(exchange, null, attributeName);
 			for (int i = 0; i < paramNames.length; i++) {
-				List<String> paramValues = requestParams.get(paramNames[i]);
-				Object paramValue = null;
-				if (paramValues != null) {
-					paramValue = (paramValues.size() == 1 ? paramValues.get(0) :
-							paramValues.toArray(new String[paramValues.size()]));
-				}
-				args[i] = binder.convertIfNecessary(paramValue, paramTypes[i], new MethodParameter(ctor, i));
+				Object value = bindValues.get(paramNames[i]);
+				value = (value != null && value instanceof List ? ((List<?>) value).toArray() : value);
+				args[i] = binder.convertIfNecessary(value, paramTypes[i], new MethodParameter(ctor, i));
 			}
 			return BeanUtils.instantiateClass(ctor, args);
 		});

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestParamMapMethodArgumentResolver.java

@@ -21,9 +21,6 @@
 
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
-import org.springframework.core.ResolvableType;
-import org.springframework.http.codec.multipart.Part;
-import org.springframework.util.Assert;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -51,6 +48,7 @@
 public class RequestParamMapMethodArgumentResolver extends HandlerMethodArgumentResolverSupport
 		implements SyncHandlerMethodArgumentResolver {
 
+
 	public RequestParamMapMethodArgumentResolver(ReactiveAdapterRegistry adapterRegistry) {
 		super(adapterRegistry);
 	}
@@ -70,18 +68,10 @@ private boolean allParams(RequestParam requestParam, Class<?> type) {
 	public Optional<Object> resolveArgumentValue(MethodParameter methodParameter,
 			BindingContext context, ServerWebExchange exchange) {
 
-		ResolvableType paramType = ResolvableType.forType(methodParameter.getGenericParameterType());
-		boolean isMultiValueMap = MultiValueMap.class.isAssignableFrom(paramType.getRawClass());
-
-
-		if (paramType.getGeneric(1).getRawClass() == Part.class) {
-			MultiValueMap<String, Part> requestParts = exchange.getMultipartData().subscribe().peek();
-			Assert.notNull(requestParts, "Expected multipart data (if any) to be parsed.");
-			return Optional.of(isMultiValueMap ? requestParts : requestParts.toSingleValueMap());
-		}
-		MultiValueMap<String, String> requestParams = exchange.getRequestParams().subscribe().peek();
-		Assert.notNull(requestParams, "Expected form data (if any) to be parsed.");
-		return Optional.of(isMultiValueMap ? requestParams : requestParams.toSingleValueMap());
+		boolean isMultiValueMap = MultiValueMap.class.isAssignableFrom(methodParameter.getParameterType());
+		MultiValueMap<String, String> queryParams = exchange.getRequest().getQueryParams();
+		Object value = isMultiValueMap ? queryParams : queryParams.toSingleValueMap();
+		return Optional.of(value);
 	}
 
 }