Support Model-related return values

This commit adds support for Model-related return values types such as
Map, Model, @ModelAttribute annotated, and non-simple types, which
helps to clarify the logic in ViewResolutionResultHandler.

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

@@ -15,22 +15,31 @@
  */
 package org.springframework.web.reactive.result.view;
 
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Optional;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
+import org.springframework.beans.BeanUtils;
+import org.springframework.core.Conventions;
+import org.springframework.core.GenericTypeResolver;
+import org.springframework.core.MethodParameter;
 import org.springframework.core.Ordered;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.ui.Model;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
+import org.springframework.web.bind.annotation.ModelAttribute;
+import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.reactive.HandlerResultHandler;
 import org.springframework.web.server.ServerWebExchange;
@@ -38,10 +47,22 @@ import org.springframework.web.util.HttpRequestPathHelper;
 
 
 /**
- * {@code HandlerResultHandler} that performs view resolution by resolving a
- * {@link View} instance first and then rendering the response with it.
- * If the return value is a String, the configured {@link ViewResolver}s will
- * be consulted to resolve that to a {@link View} instance.
+ * {@code HandlerResultHandler} that encapsulates the view resolution algorithm
+ * supporting the following return types:
+ * <ul>
+ *     <li>String-based view name
+ *     <li>Reference to a {@link View}
+ *     <li>{@link Model}
+ *     <li>{@link Map}
+ *     <li>Return types annotated with {@code @ModelAttribute}
+ *     <li>{@link BeanUtils#isSimpleProperty Non-simple} return types are
+ *     treated as a model attribute
+ * </ul>
+ *
+ * <p>A String-based view name is resolved through the configured
+ * {@link ViewResolver} instances into a {@link View} to use for rendering.
+ * If a view is left unspecified (e.g. by returning {@code null} or a
+ * model-related return value), a default view name is selected.
  *
  * <p>This result handler should be ordered late relative to other result
  * handlers. See {@link #setOrder(int)} for more details.
@@ -96,51 +117,72 @@ public class ViewResolutionResultHandler implements HandlerResultHandler, Ordere
 		return this.order;
 	}
 
-	// TODO: Support for Model, ModelAndView, @ModelAttribute, Object with no method annotations
-
 	@Override
 	public boolean supports(HandlerResult result) {
 		Class<?> clazz = result.getReturnValueType().getRawClass();
-		if (isStringOrViewReference(clazz)) {
+		if (hasModelAttributeAnnotation(result)) {
+			return true;
+		}
+		if (isSupportedType(clazz)) {
 			return true;
 		}
 		if (this.conversionService.canConvert(clazz, Mono.class)) {
 			clazz = result.getReturnValueType().getGeneric(0).getRawClass();
-			return isStringOrViewReference(clazz);
+			return isSupportedType(clazz);
 		}
 		return false;
 	}
 
-	private boolean isStringOrViewReference(Class<?> clazz) {
-		return (CharSequence.class.isAssignableFrom(clazz) || View.class.isAssignableFrom(clazz));
+	private boolean hasModelAttributeAnnotation(HandlerResult result) {
+		if (result.getHandler() instanceof HandlerMethod) {
+			MethodParameter returnType = ((HandlerMethod) result.getHandler()).getReturnType();
+			if (returnType.hasMethodAnnotation(ModelAttribute.class)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private boolean isSupportedType(Class<?> clazz) {
+		return (CharSequence.class.isAssignableFrom(clazz) || View.class.isAssignableFrom(clazz) ||
+				Model.class.isAssignableFrom(clazz) || Map.class.isAssignableFrom(clazz) ||
+				!BeanUtils.isSimpleProperty(clazz));
 	}
 
 	@Override
 	public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result) {
 
-		Mono<Object> mono;
+		Mono<Object> valueMono;
 		ResolvableType elementType;
 		ResolvableType returnType = result.getReturnValueType();
 
 		if (this.conversionService.canConvert(returnType.getRawClass(), Mono.class)) {
 			Optional<Object> optionalValue = result.getReturnValue();
 			if (optionalValue.isPresent()) {
-				Mono<?> convertedMono = this.conversionService.convert(optionalValue.get(), Mono.class);
-				mono = convertedMono.map(o -> o);
+				Mono<?> converted = this.conversionService.convert(optionalValue.get(), Mono.class);
+				valueMono = converted.map(o -> o);
 			}
 			else {
-				mono = Mono.empty();
+				valueMono = Mono.empty();
 			}
 			elementType = returnType.getGeneric(0);
 		}
 		else {
-			mono = Mono.justOrEmpty(result.getReturnValue());
+			valueMono = Mono.justOrEmpty(result.getReturnValue());
 			elementType = returnType;
 		}
 
-		mono = mono.otherwiseIfEmpty(handleMissingReturnValue(exchange, result, elementType));
+		Mono<Object> viewMono;
+		if (isViewReturnType(result, elementType)) {
+			viewMono = valueMono.otherwiseIfEmpty(selectDefaultViewName(exchange, result));
+		}
+		else {
+			viewMono = valueMono.map(value -> updateModel(result, value))
+					.defaultIfEmpty(result.getModel())
+					.then(model -> selectDefaultViewName(exchange, result));
+		}
 
-		return mono.then(returnValue -> {
+		return viewMono.then(returnValue -> {
 			if (returnValue instanceof View) {
 				Flux<DataBuffer> body = ((View) returnValue).render(result, null, exchange);
 				return exchange.getResponse().setBody(body);
@@ -158,28 +200,26 @@ public class ViewResolutionResultHandler implements HandlerResultHandler, Ordere
 						});
 			}
 			else {
-				// Eventually for model-related return values (should not happen now)
+				// Should not happen
 				return Mono.error(new IllegalStateException("Unexpected return value"));
 			}
 		});
 	}
 
-	private Mono<Object> handleMissingReturnValue(ServerWebExchange exchange, HandlerResult result,
-			ResolvableType elementType) {
+	private boolean isViewReturnType(HandlerResult result, ResolvableType elementType) {
+		Class<?> clazz = elementType.getRawClass();
+		return (View.class.isAssignableFrom(clazz) ||
+				(CharSequence.class.isAssignableFrom(clazz) && !hasModelAttributeAnnotation(result)));
+	}
 
-		if (isStringOrViewReference(elementType.getRawClass())) {
-			String defaultViewName = getDefaultViewName(exchange, result);
-			if (defaultViewName != null) {
-				return Mono.just(defaultViewName);
-			}
-			else {
-				return Mono.error(new IllegalStateException("Handler [" + result.getHandler() + "] " +
-						"neither returned a view name nor a View object"));
-			}
+	private Mono<Object> selectDefaultViewName(ServerWebExchange exchange, HandlerResult result) {
+		String defaultViewName = getDefaultViewName(exchange, result);
+		if (defaultViewName != null) {
+			return Mono.just(defaultViewName);
 		}
 		else {
-			// Eventually for model-related return values (should not happen now)
-			return Mono.error(new IllegalStateException("Unexpected return value type"));
+			return Mono.error(new IllegalStateException("Handler [" + result.getHandler() + "] " +
+					"neither returned a view name nor a View object"));
 		}
 	}
 
@@ -191,6 +231,7 @@ public class ViewResolutionResultHandler implements HandlerResultHandler, Ordere
 	 * @return the default view name to use; if {@code null} is returned
 	 * processing will result in an IllegalStateException.
 	 */
+	@SuppressWarnings("UnusedParameters")
 	protected String getDefaultViewName(ServerWebExchange exchange, HandlerResult result) {
 		String path = this.pathHelper.getLookupPathForRequest(exchange);
 		if (path.startsWith("/")) {
@@ -202,6 +243,49 @@ public class ViewResolutionResultHandler implements HandlerResultHandler, Ordere
 		return StringUtils.stripFilenameExtension(path);
 	}
 
+	private Object updateModel(HandlerResult result, Object value) {
+		if (value instanceof Model) {
+			result.getModel().addAllAttributes(((Model) value).asMap());
+		}
+		else if (value instanceof Map) {
+			//noinspection unchecked
+			result.getModel().addAllAttributes((Map<String, ?>) value);
+		}
+		else if (result.getHandler() instanceof HandlerMethod) {
+			MethodParameter returnType = ((HandlerMethod) result.getHandler()).getReturnType();
+			String name = getNameForReturnValue(value, returnType);
+			result.getModel().addAttribute(name, value);
+		}
+		else {
+			result.getModel().addAttribute(value);
+		}
+		return value;
+	}
+
+	/**
+	 * Derive the model attribute name for the given return value using one of:
+	 * <ol>
+	 * <li>The method {@code ModelAttribute} annotation value
+	 * <li>The declared return type if it is more specific than {@code Object}
+	 * <li>The actual return value type
+	 * </ol>
+	 * @param returnValue the value returned from a method invocation
+	 * @param returnType the return type of the method
+	 * @return the model name, never {@code null} nor empty
+	 */
+	private static String getNameForReturnValue(Object returnValue, MethodParameter returnType) {
+		ModelAttribute annotation = returnType.getMethodAnnotation(ModelAttribute.class);
+		if (annotation != null && StringUtils.hasText(annotation.value())) {
+			return annotation.value();
+		}
+		else {
+			Method method = returnType.getMethod();
+			Class<?> containingClass = returnType.getContainingClass();
+			Class<?> resolvedType = GenericTypeResolver.resolveReturnType(method, containingClass);
+			return Conventions.getVariableNameForReturnType(method, resolvedType, returnValue);
+		}
+	}
+
 	private Mono<View> handleUnresolvedViewName(String viewName) {
 		return Mono.error(new IllegalStateException(
 				"Could not resolve view with name '" + viewName + "'."));

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

@@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.view;
 
 import java.lang.reflect.Method;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.time.Duration;
@@ -38,6 +37,8 @@ import rx.Single;
 
 import org.springframework.core.Ordered;
 import org.springframework.core.ResolvableType;
+import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.support.ConfigurableConversionService;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.core.convert.support.ReactiveStreamsToRxJava1Converter;
 import org.springframework.core.io.buffer.DataBuffer;
@@ -48,7 +49,10 @@ import org.springframework.http.server.reactive.MockServerHttpRequest;
 import org.springframework.http.server.reactive.MockServerHttpResponse;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.ui.ExtendedModelMap;
+import org.springframework.ui.Model;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.bind.annotation.ModelAttribute;
+import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.reactive.HandlerResultHandler;
 import org.springframework.web.server.ServerWebExchange;
@@ -56,7 +60,10 @@ import org.springframework.web.server.adapter.DefaultServerWebExchange;
 import org.springframework.web.server.session.DefaultWebSessionManager;
 import org.springframework.web.server.session.WebSessionManager;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -65,32 +72,30 @@ import static org.mockito.Mockito.mock;
  */
 public class ViewResolutionResultHandlerTests {
 
-	private static final Charset UTF_8 = Charset.forName("UTF-8");
-
-
 	private MockServerHttpResponse response;
 
 	private ModelMap model;
 
-	private DefaultConversionService conversionService;
-
 
 	@Before
 	public void setUp() throws Exception {
 		this.model = new ExtendedModelMap().addAttribute("id", "123");
-		this.conversionService = new DefaultConversionService();
-		this.conversionService.addConverter(new ReactiveStreamsToRxJava1Converter());
 	}
 
 
 	@Test
 	public void supportsWithNullReturnValue() throws Exception {
-		testSupports("handleString", null);
-		testSupports("handleView", null);
-		testSupports("handleMonoString", null);
-		testSupports("handleMonoView", null);
-		testSupports("handleSingleString", null);
-		testSupports("handleSingleView", null);
+		testSupports("handleString", true);
+		testSupports("handleView", true);
+		testSupports("handleMonoString", true);
+		testSupports("handleMonoView", true);
+		testSupports("handleSingleString", true);
+		testSupports("handleSingleView", true);
+		testSupports("handleModel", true);
+		testSupports("handleMap", true);
+		testSupports("handleModelAttributeAnnotation", true);
+		testSupports("handleTestBean", true);
+		testSupports("handleInteger", false);
 	}
 
 	@Test
@@ -100,15 +105,15 @@ public class ViewResolutionResultHandlerTests {
 		resolver1.setOrder(2);
 		resolver2.setOrder(1);
 
-		assertEquals(Arrays.asList(resolver2, resolver1),
-				new ViewResolutionResultHandler(Arrays.asList(resolver1, resolver2), this.conversionService)
-						.getViewResolvers());
+		assertEquals(Arrays.asList(resolver2, resolver1), new ViewResolutionResultHandler(
+				Arrays.asList(resolver1, resolver2), new DefaultConversionService())
+				.getViewResolvers());
 	}
 
 	@Test
 	public void viewReference() throws Exception {
 		Object value = new TestView("account");
-		handle("/path", value, ResolvableType.forClass(View.class));
+		handle("/path", value, "handleView");
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
@@ -117,7 +122,7 @@ public class ViewResolutionResultHandlerTests {
 	@Test
 	public void viewReferenceInMono() throws Exception {
 		Object value = Mono.just(new TestView("account"));
-		handle("/path", value, returnTypeFor("handleMonoView"));
+		handle("/path", value, "handleMonoView");
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
@@ -126,7 +131,7 @@ public class ViewResolutionResultHandlerTests {
 	@Test
 	public void viewName() throws Exception {
 		Object value = "account";
-		handle("/path", value, ResolvableType.forClass(String.class), new TestViewResolver("account"));
+		handle("/path", value, "handleString", new TestViewResolver("account"));
 
 		TestSubscriber<DataBuffer> subscriber = new TestSubscriber<>();
 		subscriber.bindTo(this.response.getBody())
@@ -136,7 +141,7 @@ public class ViewResolutionResultHandlerTests {
 	@Test
 	public void viewNameInMono() throws Exception {
 		Object value = Mono.just("account");
-		handle("/path", value, returnTypeFor("handleMonoString"), new TestViewResolver("account"));
+		handle("/path", value, "handleMonoString", new TestViewResolver("account"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
@@ -145,7 +150,7 @@ public class ViewResolutionResultHandlerTests {
 	@Test
 	public void viewNameWithMultipleResolvers() throws Exception {
 		String value = "profile";
-		handle("/path", value, ResolvableType.forClass(String.class),
+		handle("/path", value, "handleString",
 				new TestViewResolver("account"), new TestViewResolver("profile"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
@@ -154,7 +159,7 @@ public class ViewResolutionResultHandlerTests {
 
 	@Test
 	public void viewNameUnresolved() throws Exception {
-		handle("/path", "account", ResolvableType.forClass(String.class))
+		handle("/path", "account", "handleString")
 				.assertErrorMessage("Could not resolve view with name 'account'.");
 	}
 
@@ -162,15 +167,15 @@ public class ViewResolutionResultHandlerTests {
 	public void viewNameIsNull() throws Exception {
 		ViewResolver resolver = new TestViewResolver("account");
 
-		handle("/account", null, ResolvableType.forClass(String.class), resolver);
+		handle("/account", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
 
-		handle("/account/", null, ResolvableType.forClass(String.class), resolver);
+		handle("/account/", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
 
-		handle("/account.123", null, ResolvableType.forClass(String.class), resolver);
+		handle("/account.123", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
 	}
@@ -178,28 +183,75 @@ public class ViewResolutionResultHandlerTests {
 	@Test
 	public void viewNameIsEmptyMono() throws Exception {
 		Object value = Mono.empty();
-		handle("/account", value, returnTypeFor("handleMonoString"), new TestViewResolver("account"));
+		handle("/account", value, "handleMonoString", new TestViewResolver("account"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
 				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
 	}
 
+	@Test
+	public void model() throws Exception {
+		Model value = new ExtendedModelMap().addAttribute("name", "Joe");
+		handle("/account", value, "handleModel", new TestViewResolver("account"));
+
+		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}", asString(buf)));
+	}
+
+	@Test
+	public void map() throws Exception {
+		Map<String, String> value = Collections.singletonMap("name", "Joe");
+		handle("/account", value, "handleMap", new TestViewResolver("account"));
+
+		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}", asString(buf)));
+	}
+
+	@Test
+	public void modelAttributeAnnotation() throws Exception {
+		String value = "Joe";
+		handle("/account", value, "handleModelAttributeAnnotation", new TestViewResolver("account"));
+
+		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}", asString(buf)));
+	}
+
+	@Test
+	public void testBean() throws Exception {
+		Object value = new TestBean("Joe");
+		handle("/account", value, "handleTestBean", new TestViewResolver("account"));
+
+		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
+				.assertValuesWith(buf -> assertEquals("account: {id=123, testBean=TestBean[name=Joe]}", asString(buf)));
+	}
+
 
-	private void testSupports(String methodName, Object returnValue) throws NoSuchMethodException {
+	private void testSupports(String methodName, boolean supports) throws NoSuchMethodException {
 		Method method = TestController.class.getMethod(methodName);
 		ResolvableType returnType = ResolvableType.forMethodParameter(method, -1);
-		HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.model);
+		HandlerResult result = new HandlerResult(new Object(), null, returnType, this.model);
 		List<ViewResolver> resolvers = Collections.singletonList(mock(ViewResolver.class));
-		ViewResolutionResultHandler handler = new ViewResolutionResultHandler(resolvers, this.conversionService);
-		assertTrue(handler.supports(result));
+		ConfigurableConversionService conversionService = new DefaultConversionService();
+		conversionService.addConverter(new ReactiveStreamsToRxJava1Converter());
+		ViewResolutionResultHandler handler = new ViewResolutionResultHandler(resolvers, conversionService);
+		if (supports) {
+			assertTrue(handler.supports(result));
+		}
+		else {
+			assertFalse(handler.supports(result));
+		}
 	}
 
-	private TestSubscriber<Void> handle(String path, Object value, ResolvableType type,
-			ViewResolver... resolvers) throws URISyntaxException {
+	private TestSubscriber<Void> handle(String path, Object value, String methodName,
+			ViewResolver... resolvers) throws Exception {
 
 		List<ViewResolver> resolverList = Arrays.asList(resolvers);
-		HandlerResultHandler handler = new ViewResolutionResultHandler(resolverList, this.conversionService);
-		HandlerResult handlerResult = new HandlerResult(new Object(), value, type, this.model);
+		ConversionService conversionService = new DefaultConversionService();
+		HandlerResultHandler handler = new ViewResolutionResultHandler(resolverList, conversionService);
+		Method method = TestController.class.getMethod(methodName);
+		HandlerMethod handlerMethod = new HandlerMethod(new TestController(), method);
+		ResolvableType type = ResolvableType.forMethodReturnType(method);
+		HandlerResult handlerResult = new HandlerResult(handlerMethod, value, type, this.model);
 
 		ServerHttpRequest request = new MockServerHttpRequest(HttpMethod.GET, new URI(path));
 		this.response = new MockServerHttpResponse();
@@ -212,13 +264,8 @@ public class ViewResolutionResultHandlerTests {
 		return subscriber.bindTo(mono).await(Duration.ofSeconds(1));
 	}
 
-	private ResolvableType returnTypeFor(String methodName, Class<?>... args) throws NoSuchMethodException {
-		Method method = TestController.class.getDeclaredMethod(methodName, args);
-		return ResolvableType.forMethodReturnType(method);
-	}
-
 	private static DataBuffer asDataBuffer(String value) {
-		ByteBuffer byteBuffer = ByteBuffer.wrap(value.getBytes(UTF_8));
+		ByteBuffer byteBuffer = ByteBuffer.wrap(value.getBytes(Charset.forName("UTF-8")));
 		return new DefaultDataBufferFactory().wrap(byteBuffer);
 	}
 
@@ -226,7 +273,7 @@ public class ViewResolutionResultHandlerTests {
 		ByteBuffer byteBuffer = dataBuffer.asByteBuffer();
 		final byte[] bytes = new byte[byteBuffer.remaining()];
 		byteBuffer.get(bytes);
-		return new String(bytes, UTF_8);
+		return new String(bytes, Charset.forName("UTF-8"));
 	}
 
 
@@ -310,6 +357,45 @@ public class ViewResolutionResultHandlerTests {
 		public Single<View> handleSingleView() {
 			return null;
 		}
+
+		public Model handleModel() {
+			return null;
+		}
+
+		public Map<String, String> handleMap() {
+			return null;
+		}
+
+		@ModelAttribute("name")
+		public String handleModelAttributeAnnotation() {
+			return null;
+		}
+
+		public TestBean handleTestBean() {
+			return null;
+		}
+
+		public int handleInteger() {
+			return 0;
+		}
+	}
+
+	private static class TestBean {
+
+		private final String name;
+
+		public TestBean(String name) {
+			this.name = name;
+		}
+
+		public String getName() {
+			return this.name;
+		}
+
+		@Override
+		public String toString() {
+			return "TestBean[name=" + this.name + "]";
+		}
 	}
 
 }