Resolve async model attributes in AbstractView

This change allows the functional WebFlux API to support natively
reactive types and also makes it possible for View implementations to
disable async attributes resolution if they want for example take
advantage of stream rendering.

It also makes AbstractView#getModelAttributes() asynchronous.

Issue: SPR-15368

Author: sdeleuze

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

@@ -18,13 +18,13 @@
 
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.core.ReactiveAdapter;
+import org.springframework.core.ReactiveAdapterRegistry;
+import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import org.springframework.context.ApplicationContext;
@@ -48,9 +48,13 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 	/** Logger that is available to subclasses */
 	protected final Log logger = LogFactory.getLog(getClass());
 
+	private static final Object NO_VALUE = new Object();
+
 
 	private final List<MediaType> mediaTypes = new ArrayList<>(4);
 
+	private final ReactiveAdapterRegistry adapterRegistry;
+
 	private Charset defaultCharset = StandardCharsets.UTF_8;
 
 	private String requestContextAttribute;
@@ -59,7 +63,12 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 
 
 	public AbstractView() {
+		this(new ReactiveAdapterRegistry());
+	}
+
+	public AbstractView(ReactiveAdapterRegistry registry) {
 		this.mediaTypes.add(ViewResolverSupport.DEFAULT_CONTENT_TYPE);
+		this.adapterRegistry = registry;
 	}
 
 
@@ -146,30 +155,77 @@ public Mono<Void> render(Map<String, ?> model, MediaType contentType,
 			exchange.getResponse().getHeaders().setContentType(contentType);
 		}
 
-		Map<String, Object> mergedModel = getModelAttributes(model, exchange);
-
-		// Expose RequestContext?
-		if (this.requestContextAttribute != null) {
-			mergedModel.put(this.requestContextAttribute, createRequestContext(exchange, mergedModel));
-		}
-
-		return renderInternal(mergedModel, contentType, exchange);
+		return getModelAttributes(model, exchange).then(mergedModel -> {
+			// Expose RequestContext?
+			if (this.requestContextAttribute != null) {
+				mergedModel.put(this.requestContextAttribute, createRequestContext(exchange, mergedModel));
+			}
+			return renderInternal(mergedModel, contentType, exchange);
+		});
 	}
 
 	/**
 	 * Prepare the model to use for rendering.
 	 * <p>The default implementation creates a combined output Map that includes
 	 * model as well as static attributes with the former taking precedence.
 	 */
-	protected Map<String, Object> getModelAttributes(Map<String, ?> model, ServerWebExchange exchange) {
+	protected Mono<Map<String, Object>> getModelAttributes(Map<String, ?> model, ServerWebExchange exchange) {
 		int size = (model != null ? model.size() : 0);
 
 		Map<String, Object> attributes = new LinkedHashMap<>(size);
 		if (model != null) {
 			attributes.putAll(model);
 		}
 
-		return attributes;
+		return resolveAsyncAttributes(attributes).then(Mono.just(attributes));
+	}
+
+	/**
+	 * By default, resolve async attributes supported by the {@link ReactiveAdapterRegistry} to their blocking counterparts.
+	 * <p>View implementations capable of taking advantage of reactive types can override this method if needed.
+	 * @return {@code Mono} to represent when the async attributes have been resolved
+	 */
+	protected Mono<Void> resolveAsyncAttributes(Map<String, Object> model) {
+
+		List<String> names = new ArrayList<>();
+		List<Mono<?>> valueMonos = new ArrayList<>();
+
+		for (Map.Entry<String, ?> entry : model.entrySet()) {
+			Object value =  entry.getValue();
+			if (value == null) {
+				continue;
+			}
+			ReactiveAdapter adapter = this.adapterRegistry.getAdapter(null, value);
+			if (adapter != null) {
+				names.add(entry.getKey());
+				if (adapter.isMultiValue()) {
+					Flux<Object> fluxValue = Flux.from(adapter.toPublisher(value));
+					valueMonos.add(fluxValue.collectList().defaultIfEmpty(Collections.emptyList()));
+				}
+				else {
+					Mono<Object> monoValue = Mono.from(adapter.toPublisher(value));
+					valueMonos.add(monoValue.defaultIfEmpty(NO_VALUE));
+				}
+			}
+		}
+
+		if (names.isEmpty()) {
+			return Mono.empty();
+		}
+
+		return Mono.when(valueMonos,
+				values -> {
+					for (int i=0; i < values.length; i++) {
+						if (values[i] != NO_VALUE) {
+							model.put(names.get(i), values[i]);
+						}
+						else {
+							model.remove(names.get(i));
+						}
+					}
+					return NO_VALUE;
+				})
+				.then();
 	}
 
 	/**

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

@@ -224,10 +224,9 @@ else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(param
 						viewsMono = resolveViews(getDefaultViewName(exchange), locale);
 					}
 
-					return resolveAsyncAttributes(model.asMap())
-							.doOnSuccess(aVoid -> addBindingResult(result.getBindingContext(), exchange))
-							.then(viewsMono)
-							.then(views -> render(views, model.asMap(), exchange));
+					addBindingResult(result.getBindingContext(), exchange);
+
+					return viewsMono.then(views -> render(views, model.asMap(), exchange));
 				});
 	}
 
@@ -274,44 +273,7 @@ private String getNameForReturnValue(Class<?> returnValueType, MethodParameter r
 		return ClassUtils.getShortNameAsProperty(returnValueType);
 	}
 
-	private Mono<Void> resolveAsyncAttributes(Map<String, Object> model) {
-
-		List<String> names = new ArrayList<>();
-		List<Mono<?>> valueMonos = new ArrayList<>();
 
-		for (Map.Entry<String, ?> entry : model.entrySet()) {
-			ReactiveAdapter adapter = getAdapterRegistry().getAdapter(null, entry.getValue());
-			if (adapter != null) {
-				names.add(entry.getKey());
-				if (adapter.isMultiValue()) {
-					Flux<Object> value = Flux.from(adapter.toPublisher(entry.getValue()));
-					valueMonos.add(value.collectList().defaultIfEmpty(Collections.emptyList()));
-				}
-				else {
-					Mono<Object> value = Mono.from(adapter.toPublisher(entry.getValue()));
-					valueMonos.add(value.defaultIfEmpty(NO_VALUE));
-				}
-			}
-		}
-
-		if (names.isEmpty()) {
-			return Mono.empty();
-		}
-
-		return Mono.when(valueMonos,
-				values -> {
-					for (int i=0; i < values.length; i++) {
-						if (values[i] != NO_VALUE) {
-							model.put(names.get(i), values[i]);
-						}
-						else {
-							model.remove(names.get(i));
-						}
-					}
-					return NO_VALUE;
-				})
-				.then();
-	}
 
 	private void addBindingResult(BindingContext context, ServerWebExchange exchange) {
 		Map<String, Object> model = context.getModel().asMap();

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

@@ -0,0 +1,77 @@
+package org.springframework.web.reactive.result.view;
+
+import io.reactivex.Observable;
+import io.reactivex.Single;
+import org.junit.Before;
+import org.junit.Test;
+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.tests.sample.beans.TestBean;
+import org.springframework.ui.Model;
+import org.springframework.web.server.ServerWebExchange;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.test.StepVerifier;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.logging.Level;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * Unit tests for {@link AbstractView}.
+ *
+ * @author Sebastien Deleuze
+ */
+public class AbstractViewTests {
+
+    private MockServerWebExchange exchange;
+
+    @Before
+    public void setup() {
+        this.exchange = MockServerHttpRequest.get("/").toExchange();
+    }
+
+    @Test
+    public void resolveAsyncAttributes() {
+
+        TestBean testBean1 = new TestBean("Bean1");
+        TestBean testBean2 = new TestBean("Bean2");
+        Map<String, Object> attributes = new HashMap();
+        attributes.put("attr1", Mono.just(testBean1));
+        attributes.put("attr2", Flux.just(testBean1, testBean2));
+        attributes.put("attr3", Single.just(testBean2));
+        attributes.put("attr4", Observable.just(testBean1, testBean2));
+        attributes.put("attr5", Mono.empty());
+
+        TestView view = new TestView();
+        StepVerifier.create(view.render(attributes, null, this.exchange)).verifyComplete();
+
+        assertEquals(testBean1, view.attributes.get("attr1"));
+        assertArrayEquals(new TestBean[] {testBean1, testBean2}, ((List<TestBean>)view.attributes.get("attr2")).toArray());
+        assertEquals(testBean2, view.attributes.get("attr3"));
+        assertArrayEquals(new TestBean[] {testBean1, testBean2}, ((List<TestBean>)view.attributes.get("attr4")).toArray());
+        assertNull(view.attributes.get("attr5"));
+    }
+
+
+    private static class TestView extends AbstractView {
+
+        private Map<String, Object> attributes;
+
+        @Override
+        protected Mono<Void> renderInternal(Map<String, Object> renderAttributes, MediaType contentType, ServerWebExchange exchange) {
+            this.attributes = renderAttributes;
+            return Mono.empty();
+        }
+
+        public Map<String, Object> getAttributes() {
+            return this.attributes;
+        }
+    }
+}

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

@@ -61,7 +61,7 @@ public void noUrlSet() throws Exception {
 	public void defaultStatusCode() {
 		String url = "http://url.somewhere.com";
 		RedirectView view = new RedirectView(url);
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(HttpStatus.SEE_OTHER, this.exchange.getResponse().getStatusCode());
 		assertEquals(URI.create(url), this.exchange.getResponse().getHeaders().getLocation());
 	}
@@ -70,7 +70,7 @@ public void defaultStatusCode() {
 	public void customStatusCode() {
 		String url = "http://url.somewhere.com";
 		RedirectView view = new RedirectView(url, HttpStatus.FOUND);
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(HttpStatus.FOUND, this.exchange.getResponse().getStatusCode());
 		assertEquals(URI.create(url), this.exchange.getResponse().getHeaders().getLocation());
 	}
@@ -79,15 +79,15 @@ public void customStatusCode() {
 	public void contextRelative() {
 		String url = "/test.html";
 		RedirectView view = new RedirectView(url);
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(URI.create("/context/test.html"), this.exchange.getResponse().getHeaders().getLocation());
 	}
 
 	@Test
 	public void contextRelativeQueryParam() {
 		String url = "/test.html?id=1";
 		RedirectView view = new RedirectView(url);
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(URI.create("/context/test.html?id=1"), this.exchange.getResponse().getHeaders().getLocation());
 	}
 
@@ -111,7 +111,7 @@ public void expandUriTemplateVariablesFromModel() {
 		String url = "http://url.somewhere.com?foo={foo}";
 		Map<String, String> model = Collections.singletonMap("foo", "bar");
 		RedirectView view = new RedirectView(url);
-		view.render(model, MediaType.TEXT_HTML, this.exchange);
+		view.render(model, MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(URI.create("http://url.somewhere.com?foo=bar"), this.exchange.getResponse().getHeaders().getLocation());
 	}
 
@@ -121,7 +121,7 @@ public void expandUriTemplateVariablesFromExchangeAttribute() {
 		Map<String, String> attributes = Collections.singletonMap("foo", "bar");
 		this.exchange.getAttributes().put(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, attributes);
 		RedirectView view = new RedirectView(url);
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, exchange).block();
 		assertEquals(URI.create("http://url.somewhere.com?foo=bar"), this.exchange.getResponse().getHeaders().getLocation());
 	}
 
@@ -130,7 +130,7 @@ public void propagateQueryParams() throws Exception {
 		RedirectView view = new RedirectView("http://url.somewhere.com?foo=bar#bazz");
 		view.setPropagateQuery(true);
 		this.exchange = MockServerHttpRequest.get("http://url.somewhere.com?a=b&c=d").toExchange();
-		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange);
+		view.render(new HashMap<>(), MediaType.TEXT_HTML, this.exchange).block();
 		assertEquals(HttpStatus.SEE_OTHER, this.exchange.getResponse().getStatusCode());
 		assertEquals(URI.create("http://url.somewhere.com?foo=bar&a=b&c=d#bazz"),
 				this.exchange.getResponse().getHeaders().getLocation());

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

@@ -32,7 +32,6 @@
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 import rx.Completable;
-import rx.Observable;
 import rx.Single;
 
 import org.springframework.core.MethodParameter;
@@ -249,34 +248,6 @@ public void contentNegotiationWith406() throws Exception {
 				.verify();
 	}
 
-	@Test
-	public void modelWithAsyncAttributes() throws Exception {
-		this.bindingContext.getModel()
-				.addAttribute("attr1", Mono.just(new TestBean("Bean1")))
-				.addAttribute("attr2", Flux.just(new TestBean("Bean1"), new TestBean("Bean2")))
-				.addAttribute("attr3", Single.just(new TestBean("Bean2")))
-				.addAttribute("attr4", Observable.just(new TestBean("Bean1"), new TestBean("Bean2")))
-				.addAttribute("attr5", Mono.empty());
-
-		MethodParameter returnType = on(TestController.class).resolveReturnType(void.class);
-		HandlerResult result = new HandlerResult(new Object(), null, returnType, this.bindingContext);
-		ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account"));
-
-		MockServerWebExchange exchange = get("/account").toExchange();
-
-		handler.handleResult(exchange, result).block(Duration.ofMillis(5000));
-		assertResponseBody(exchange, "account: {" +
-				"attr1=TestBean[name=Bean1], " +
-				"attr2=[TestBean[name=Bean1], TestBean[name=Bean2]], " +
-				"attr3=TestBean[name=Bean2], " +
-				"attr4=[TestBean[name=Bean1], TestBean[name=Bean2]], " +
-				"org.springframework.validation.BindingResult.attr1=" +
-				"org.springframework.validation.BeanPropertyBindingResult: 0 errors, " +
-				"org.springframework.validation.BindingResult.attr3=" +
-				"org.springframework.validation.BeanPropertyBindingResult: 0 errors" +
-				"}");
-	}
-
 
 	private ViewResolutionResultHandler resultHandler(ViewResolver... resolvers) {
 		return resultHandler(Collections.emptyList(), resolvers);