BindingResult for resolved async attributes

ViewResolutionResultHandler no longer adds BindingResult objects for
async model attributes. Instead AbstractView adds them later when
those attributes are resolved to concrete values.

Closes gh-22933

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java

@@ -19,6 +19,7 @@ package org.springframework.web.reactive.result.view;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -29,6 +30,7 @@ import org.apache.commons.logging.LogFactory;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
+import org.springframework.beans.BeanUtils;
 import org.springframework.beans.factory.BeanNameAware;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -37,6 +39,8 @@ import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.http.MediaType;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.validation.BindingResult;
+import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
@@ -54,9 +58,6 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo
 	/** Logger that is available to subclasses. */
 	protected final Log logger = LogFactory.getLog(getClass());
 
-	private static final Object NO_VALUE = new Object();
-
-
 	private final ReactiveAdapterRegistry adapterRegistry;
 
 	private final List<MediaType> mediaTypes = new ArrayList<>(4);
@@ -219,18 +220,23 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo
 		if (model != null) {
 			attributes.putAll(model);
 		}
-		return resolveAsyncAttributes(attributes).thenReturn(attributes);
+		//noinspection deprecation
+		return resolveAsyncAttributes(attributes)
+				.then(resolveAsyncAttributes(attributes, exchange))
+				.doOnTerminate(() -> exchange.getAttributes().remove(BINDING_CONTEXT_ATTRIBUTE))
+				.thenReturn(attributes);
 	}
 
 	/**
 	 * Use the configured {@link ReactiveAdapterRegistry} to adapt asynchronous
-	 * model attributes to {@code Mono<T>} or {@code Mono<List<T>>} and resolve
-	 * them to actual values via {@link Mono#zip(Mono, Mono)}, so that when
-	 * the returned result {@code Mono} completes, the model has its asynchronous
-	 * attributes replaced with synchronous values.
+	 * attributes to {@code Mono<T>} or {@code Mono<List<T>>} and then wait to
+	 * resolve them into actual values. When the returned {@code Mono<Void>}
+	 * completes, the asynchronous attributes in the model would have been
+	 * replaced with their corresponding resolved values.
 	 * @return result {@code Mono} that completes when the model is ready
+	 * @since 5.1.8
 	 */
-	protected Mono<Void> resolveAsyncAttributes(Map<String, Object> model) {
+	protected Mono<Void> resolveAsyncAttributes(Map<String, Object> model, ServerWebExchange exchange) {
 		List<Mono<?>> asyncAttributes = null;
 		for (Map.Entry<String, ?> entry : model.entrySet()) {
 			Object value =  entry.getValue();
@@ -247,10 +253,7 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo
 					asyncAttributes.add(
 							Flux.from(adapter.toPublisher(value))
 									.collectList()
-									.doOnSuccess(result -> {
-										result = result != null ? result : Collections.emptyList();
-										model.put(name, result);
-									}));
+									.doOnSuccess(result -> model.put(name, result)));
 				}
 				else {
 					asyncAttributes.add(
@@ -258,6 +261,7 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo
 									.doOnSuccess(result -> {
 										if (result != null) {
 											model.put(name, result);
+											addBindingResult(name, result, model, exchange);
 										}
 										else {
 											model.remove(name);
@@ -269,6 +273,32 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo
 		return asyncAttributes != null ? Mono.when(asyncAttributes) : Mono.empty();
 	}
 
+	private void addBindingResult(String name, Object value, Map<String, Object> model, ServerWebExchange exchange) {
+		BindingContext context = exchange.getAttribute(BINDING_CONTEXT_ATTRIBUTE);
+		if (context == null || value.getClass().isArray() || value instanceof Collection ||
+				value instanceof Map || BeanUtils.isSimpleValueType(value.getClass())) {
+			return;
+		}
+		BindingResult result = context.createDataBinder(exchange, value, name).getBindingResult();
+		model.put(BindingResult.MODEL_KEY_PREFIX + name, result);
+	}
+
+	/**
+	 * Use the configured {@link ReactiveAdapterRegistry} to adapt asynchronous
+	 * attributes to {@code Mono<T>} or {@code Mono<List<T>>} and then wait to
+	 * resolve them into actual values. When the returned {@code Mono<Void>}
+	 * completes, the asynchronous attributes in the model would have been
+	 * replaced with their corresponding resolved values.
+	 * @return result {@code Mono} that completes when the model is ready
+	 * @deprecated as of 5.1.8 this method is still invoked but it is a no-op.
+	 * Please, use {@link #resolveAsyncAttributes(Map, ServerWebExchange)}
+	 * instead. It is invoked after this one and does the actual work.
+	 */
+	@Deprecated
+	protected Mono<Void> resolveAsyncAttributes(Map<String, Object> model) {
+		return Mono.empty();
+	}
+
 	/**
 	 * Create a RequestContext to expose under the specified attribute name.
 	 * <p>The default implementation creates a standard RequestContext instance

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/View.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -44,6 +44,18 @@ import org.springframework.web.server.ServerWebExchange;
  */
 public interface View {
 
+	/**
+	 * The name of the exchange attribute that contains the
+	 * {@link org.springframework.web.reactive.BindingContext BindingContext}
+	 * for the request which can be used to create
+	 * {@link org.springframework.validation.BindingResult BindingResult}
+	 * instances for objects in to the model.
+	 * <p>Note: This attribute is not required and may not be present.
+	 * @since 5.1.8
+	 */
+	String BINDING_CONTEXT_ATTRIBUTE = View.class.getName() + ".bindingContext";
+
+
 	/**
 	 * Return the list of media types this View supports, or an empty list.
 	 */

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -245,10 +245,9 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport
 						model.addAttribute(name, returnValue);
 						viewsMono = resolveViews(getDefaultViewName(exchange), locale);
 					}
-
-					updateBindingContext(result.getBindingContext(), exchange);
-
-					return viewsMono.flatMap(views -> render(views, model.asMap(), exchange));
+					BindingContext bindingContext = result.getBindingContext();
+					updateBindingResult(bindingContext, exchange);
+					return viewsMono.flatMap(views -> render(views, model.asMap(), bindingContext, exchange));
 				});
 	}
 
@@ -288,39 +287,42 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport
 				.orElseGet(() -> Conventions.getVariableNameForParameter(returnType));
 	}
 
-	private void updateBindingContext(BindingContext context, ServerWebExchange exchange) {
+	private void updateBindingResult(BindingContext context, ServerWebExchange exchange) {
 		Map<String, Object> model = context.getModel().asMap();
-		model.keySet().stream()
-				.filter(name -> isBindingCandidate(name, model.get(name)))
-				.filter(name -> !model.containsKey(BindingResult.MODEL_KEY_PREFIX + name))
-				.forEach(name -> {
-					WebExchangeDataBinder binder = context.createDataBinder(exchange, model.get(name), name);
+		for (Map.Entry<String, Object> entry : model.entrySet()) {
+			String name = entry.getKey();
+			Object value = entry.getValue();
+			if (isBindingCandidate(name, value)) {
+				if (!model.containsKey(BindingResult.MODEL_KEY_PREFIX + name)) {
+					WebExchangeDataBinder binder = context.createDataBinder(exchange, value, name);
 					model.put(BindingResult.MODEL_KEY_PREFIX + name, binder.getBindingResult());
-				});
+				}
+			}
+		}
 	}
 
 	private boolean isBindingCandidate(String name, @Nullable Object value) {
 		return (!name.startsWith(BindingResult.MODEL_KEY_PREFIX) && value != null &&
-				!value.getClass().isArray() && !(value instanceof Collection) &&
-				!(value instanceof Map) && !BeanUtils.isSimpleValueType(value.getClass()));
+				!value.getClass().isArray() && !(value instanceof Collection) && !(value instanceof Map) &&
+				getAdapterRegistry().getAdapter(null, value) == null &&
+				!BeanUtils.isSimpleValueType(value.getClass()));
 	}
 
 	private Mono<? extends Void> render(List<View> views, Map<String, Object> model,
-			ServerWebExchange exchange) {
+			BindingContext bindingContext, ServerWebExchange exchange) {
 
 		for (View view : views) {
 			if (view.isRedirectView()) {
-				return view.render(model, null, exchange);
+				return renderWith(view, model, null, exchange, bindingContext);
 			}
 		}
-
 		List<MediaType> mediaTypes = getMediaTypes(views);
 		MediaType bestMediaType = selectMediaType(exchange, () -> mediaTypes);
 		if (bestMediaType != null) {
 			for (View view : views) {
 				for (MediaType mediaType : view.getSupportedMediaTypes()) {
 					if (mediaType.isCompatibleWith(bestMediaType)) {
-						return view.render(model, mediaType, exchange);
+						return renderWith(view, model, mediaType, exchange, bindingContext);
 					}
 				}
 			}
@@ -328,6 +330,14 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport
 		throw new NotAcceptableStatusException(mediaTypes);
 	}
 
+	private Mono<? extends Void> renderWith(View view, Map<String, Object> model,
+			@Nullable MediaType mediaType, ServerWebExchange exchange, BindingContext bindingContext) {
+
+		exchange.getAttributes().put(View.BINDING_CONTEXT_ATTRIBUTE, bindingContext);
+		return view.render(model, mediaType, exchange)
+				.doOnTerminate(() -> exchange.getAttributes().remove(View.BINDING_CONTEXT_ATTRIBUTE));
+	}
+
 	private List<MediaType> getMediaTypes(List<View> views) {
 		return views.stream()
 				.flatMap(view -> view.getSupportedMediaTypes().stream())

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/AbstractViewTests.java

@@ -17,8 +17,8 @@
 package org.springframework.web.reactive.result.view;
 
 import java.time.Duration;
+import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 import io.reactivex.Observable;
@@ -29,14 +29,15 @@ import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 import org.springframework.http.MediaType;
+import org.springframework.lang.Nullable;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.tests.sample.beans.TestBean;
+import org.springframework.validation.BindingResult;
+import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.server.ServerWebExchange;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;
 
 /**
  * Unit tests for {@link AbstractView}.
@@ -54,25 +55,31 @@ public class AbstractViewTests {
 
 		TestBean testBean1 = new TestBean("Bean1");
 		TestBean testBean2 = new TestBean("Bean2");
-		Map<String, Object> attributes = new HashMap<>();
-		attributes.put("attr1", Mono.just(testBean1).delayElement(Duration.ofMillis(10)));
-		attributes.put("attr2", Flux.just(testBean1, testBean2).delayElements(Duration.ofMillis(10)));
-		attributes.put("attr3", Single.just(testBean2));
-		attributes.put("attr4", Observable.just(testBean1, testBean2));
-		attributes.put("attr5", Mono.empty());
+
+		Map<String, Object> inMap = new HashMap<>();
+		inMap.put("attr1", Mono.just(testBean1).delayElement(Duration.ofMillis(10)));
+		inMap.put("attr2", Flux.just(testBean1, testBean2).delayElements(Duration.ofMillis(10)));
+		inMap.put("attr3", Single.just(testBean2));
+		inMap.put("attr4", Observable.just(testBean1, testBean2));
+		inMap.put("attr5", Mono.empty());
+
+		this.exchange.getAttributes().put(View.BINDING_CONTEXT_ATTRIBUTE, new BindingContext());
 
 		TestView view = new TestView();
-		StepVerifier.create(
-				view.render(attributes, null, this.exchange)).verifyComplete();
-
-		Map<String, Object> actual = view.attributes;
-		assertEquals(testBean1, actual.get("attr1"));
-		assertArrayEquals(new TestBean[] { testBean1, testBean2 },
-				((List<TestBean>) actual.get("attr2")).toArray());
-		assertEquals(testBean2, actual.get("attr3"));
-		assertArrayEquals(new TestBean[] { testBean1, testBean2 },
-				((List<TestBean>) actual.get("attr4")).toArray());
-		assertNull(actual.get("attr5"));
+		StepVerifier.create(view.render(inMap, null, this.exchange)).verifyComplete();
+
+		Map<String, Object> outMap = view.attributes;
+		assertEquals(testBean1, outMap.get("attr1"));
+		assertEquals(Arrays.asList(testBean1, testBean2), outMap.get("attr2"));
+		assertEquals(testBean2, outMap.get("attr3"));
+		assertEquals(Arrays.asList(testBean1, testBean2), outMap.get("attr4"));
+		assertNull(outMap.get("attr5"));
+
+		assertNotNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr1"));
+		assertNotNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr3"));
+		assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr2"));
+		assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr4"));
+		assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr5"));
 	}
 
 	private static class TestView extends AbstractView {
@@ -81,7 +88,7 @@ public class AbstractViewTests {
 
 		@Override
 		protected Mono<Void> renderInternal(Map<String, Object> renderAttributes,
-				MediaType contentType, ServerWebExchange exchange) {
+				@Nullable MediaType contentType, ServerWebExchange exchange) {
 
 			this.attributes = renderAttributes;
 			return Mono.empty();