Polish view resolution

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

@@ -127,6 +127,9 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 		}
 	}
 
+	/**
+	 * Return the configured default {@code View}'s.
+	 */
 	public List<View> getDefaultViews() {
 		return this.defaultViews;
 	}
@@ -187,46 +190,48 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 		}
 
 		Mono<Object> viewMono;
-		if (isViewReturnType(result, elementType)) {
-			viewMono = valueMono.otherwiseIfEmpty(selectDefaultViewName(exchange, result));
+		if (isViewNameOrReference(elementType, result)) {
+			Mono<Object> viewName = getDefaultViewNameMono(exchange, result);
+			viewMono = valueMono.otherwiseIfEmpty(viewName);
 		}
 		else {
-			viewMono = valueMono.map(value -> updateModel(result, value))
+			viewMono = valueMono.map(value -> updateModel(value, result))
 					.defaultIfEmpty(result.getModel())
-					.then(model -> selectDefaultViewName(exchange, result));
+					.then(model -> getDefaultViewNameMono(exchange, result));
 		}
 
-		return viewMono.then(returnValue -> {
-			if (returnValue instanceof View) {
-				return ((View) returnValue).render(result, null, exchange);
+		return viewMono.then(view -> {
+			if (view instanceof View) {
+				return ((View) view).render(result, null, exchange);
 			}
-			else if (returnValue instanceof CharSequence) {
-				String viewName = returnValue.toString();
+			else if (view instanceof CharSequence) {
+				String viewName = view.toString();
 				Locale locale = Locale.getDefault(); // TODO
-				return resolveViewAndRender(viewName, locale, result, exchange);
+				return resolveAndRender(viewName, locale, result, exchange);
 
 			}
 			else {
 				// Should not happen
-				return Mono.error(new IllegalStateException("Unexpected return value"));
+				return Mono.error(new IllegalStateException("Unexpected view type"));
 			}
 		});
 	}
 
-	private boolean isViewReturnType(HandlerResult result, ResolvableType elementType) {
+	private boolean isViewNameOrReference(ResolvableType elementType, HandlerResult result) {
 		Class<?> clazz = elementType.getRawClass();
 		return (View.class.isAssignableFrom(clazz) ||
 				(CharSequence.class.isAssignableFrom(clazz) && !hasModelAttributeAnnotation(result)));
 	}
 
-	private Mono<Object> selectDefaultViewName(ServerWebExchange exchange, HandlerResult result) {
-		String defaultViewName = getDefaultViewName(exchange, result);
+	private Mono<Object> getDefaultViewNameMono(ServerWebExchange exchange, HandlerResult result) {
+		String defaultViewName = getDefaultViewName(result, exchange);
 		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"));
+			return Mono.error(new IllegalStateException(
+					"Handler [" + result.getHandler() + "] " +
+							"neither returned a view name nor a View object"));
 		}
 	}
 
@@ -239,7 +244,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 	 * processing will result in an IllegalStateException.
 	 */
 	@SuppressWarnings("UnusedParameters")
-	protected String getDefaultViewName(ServerWebExchange exchange, HandlerResult result) {
+	protected String getDefaultViewName(HandlerResult result, ServerWebExchange exchange) {
 		String path = this.pathHelper.getLookupPathForRequest(exchange);
 		if (path.startsWith("/")) {
 			path = path.substring(1);
@@ -250,7 +255,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 		return StringUtils.stripFilenameExtension(path);
 	}
 
-	private Object updateModel(HandlerResult result, Object value) {
+	private Object updateModel(Object value, HandlerResult result) {
 		if (value instanceof Model) {
 			result.getModel().addAllAttributes(((Model) value).asMap());
 		}
@@ -293,7 +298,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 		}
 	}
 
-	private Mono<? extends Void> resolveViewAndRender(String viewName, Locale locale,
+	private Mono<? extends Void> resolveAndRender(String viewName, Locale locale,
 			HandlerResult result, ServerWebExchange exchange) {
 
 		return Flux.fromIterable(getViewResolvers())

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

@@ -43,6 +43,7 @@ import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.core.convert.support.ReactiveStreamsToRxJava1Converter;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DefaultDataBufferFactory;
+import org.springframework.core.io.buffer.support.DataBufferTestUtils;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
 import org.springframework.http.server.reactive.MockServerHttpRequest;
@@ -88,7 +89,7 @@ public class ViewResolutionResultHandlerTests {
 
 
 	@Test
-	public void supportsWithNullReturnValue() throws Exception {
+	public void supports() throws Exception {
 		testSupports("handleString", true);
 		testSupports("handleView", true);
 		testSupports("handleMonoString", true);
@@ -120,7 +121,8 @@ public class ViewResolutionResultHandlerTests {
 		handle("/path", value, "handleView");
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
@@ -129,7 +131,8 @@ public class ViewResolutionResultHandlerTests {
 		handle("/path", value, "handleMonoView");
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
@@ -139,16 +142,18 @@ public class ViewResolutionResultHandlerTests {
 
 		TestSubscriber<DataBuffer> subscriber = new TestSubscriber<>();
 		subscriber.bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void viewNameInMono() throws Exception {
+	public void viewNameMono() throws Exception {
 		Object value = Mono.just("account");
 		handle("/path", value, "handleMonoString", new TestViewResolver("account"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
@@ -158,7 +163,8 @@ public class ViewResolutionResultHandlerTests {
 				new TestViewResolver("account"), new TestViewResolver("profile"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("profile: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("profile: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
@@ -173,15 +179,18 @@ public class ViewResolutionResultHandlerTests {
 
 		handle("/account", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 
 		handle("/account/", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 
 		handle("/account.123", null, "handleString", resolver);
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
@@ -190,47 +199,52 @@ public class ViewResolutionResultHandlerTests {
 		handle("/account", value, "handleMonoString", new TestViewResolver("account"));
 
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("account: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void model() throws Exception {
+	public void modelReturnValue() 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)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void map() throws Exception {
+	public void mapReturnValue() 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)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void modelAttributeAnnotation() throws Exception {
+	public void modelAttributeAnnotationReturnValue() 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)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123, name=Joe}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void testBean() throws Exception {
+	public void objectReturnValue() 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)));
+				.assertValuesWith(buf -> assertEquals("account: {id=123, testBean=TestBean[name=Joe]}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void selectBestMediaType() throws Exception {
+	public void contentNegotiation() throws Exception {
 		TestView htmlView = new TestView("account");
 		htmlView.setMediaTypes(Collections.singletonList(MediaType.TEXT_HTML));
 
@@ -245,11 +259,12 @@ public class ViewResolutionResultHandlerTests {
 
 		assertEquals(MediaType.APPLICATION_JSON, this.response.getHeaders().getContentType());
 		new TestSubscriber<DataBuffer>().bindTo(this.response.getBody())
-				.assertValuesWith(buf -> assertEquals("defaultView: {id=123}", asString(buf)));
+				.assertValuesWith(buf -> assertEquals("defaultView: {id=123}",
+						DataBufferTestUtils.dumpString(buf, Charset.forName("UTF-8"))));
 	}
 
 	@Test
-	public void selectBestMediaTypeNotAcceptable() throws Exception {
+	public void contentNegotiationNotAcceptable() throws Exception {
 		TestView htmlView = new TestView("account");
 		htmlView.setMediaTypes(Collections.singletonList(MediaType.TEXT_HTML));
 
@@ -304,18 +319,6 @@ public class ViewResolutionResultHandlerTests {
 		return subscriber.bindTo(mono).await(Duration.ofSeconds(1));
 	}
 
-	private static DataBuffer asDataBuffer(String value) {
-		ByteBuffer byteBuffer = ByteBuffer.wrap(value.getBytes(Charset.forName("UTF-8")));
-		return new DefaultDataBufferFactory().wrap(byteBuffer);
-	}
-
-	private static String asString(DataBuffer dataBuffer) {
-		ByteBuffer byteBuffer = dataBuffer.asByteBuffer();
-		final byte[] bytes = new byte[byteBuffer.remaining()];
-		byteBuffer.get(bytes);
-		return new String(bytes, Charset.forName("UTF-8"));
-	}
-
 
 	private static class TestViewResolver implements ViewResolver, Ordered {
 
@@ -381,7 +384,9 @@ public class ViewResolutionResultHandlerTests {
 			if (mediaType != null) {
 				response.getHeaders().setContentType(mediaType);
 			}
-			return response.writeWith(Flux.just(asDataBuffer(value)));
+			ByteBuffer byteBuffer = ByteBuffer.wrap(value.getBytes(Charset.forName("UTF-8")));
+			DataBuffer dataBuffer = new DefaultDataBufferFactory().wrap(byteBuffer);
+			return response.writeWith(Flux.just(dataBuffer));
 		}
 	}