Merge pull request #1162 from poutsma/spock_model_and_view

* spock_model_and_view:
  Fix using system default charset in view rendering
  Changed View.render method to take Map<String, ?>

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

@@ -16,6 +16,8 @@
 
 package org.springframework.web.reactive.result.view;
 
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -28,9 +30,7 @@ import reactor.core.publisher.Mono;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.http.MediaType;
-import org.springframework.ui.ModelMap;
 import org.springframework.util.Assert;
-import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
@@ -47,6 +47,8 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 
 	private final List<MediaType> mediaTypes = new ArrayList<>(4);
 
+	private Charset defaultCharset = StandardCharsets.UTF_8;
+
 	private ApplicationContext applicationContext;
 
 
@@ -75,6 +77,24 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 		return this.mediaTypes;
 	}
 
+	/**
+	 * Set the default charset for this view, used when the
+	 * {@linkplain #setSupportedMediaTypes(List) content type} does not contain one.
+	 * Default is {@linkplain StandardCharsets#UTF_8 UTF 8}.
+	 */
+	public void setDefaultCharset(Charset defaultCharset) {
+		Assert.notNull(defaultCharset, "'defaultCharset' must not be null");
+		this.defaultCharset = defaultCharset;
+	}
+
+	/**
+	 * Return the default charset, used when the
+	 * {@linkplain #setSupportedMediaTypes(List) content type} does not contain one.
+	 */
+	public Charset getDefaultCharset() {
+		return this.defaultCharset;
+	}
+
 	@Override
 	public void setApplicationContext(ApplicationContext applicationContext) {
 		this.applicationContext = applicationContext;
@@ -84,29 +104,29 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 		return applicationContext;
 	}
 
-
 	/**
 	 * Prepare the model to render.
-	 * @param result the result from handler execution
+	 * @param model Map with name Strings as keys and corresponding model
+	 * objects as values (Map can also be {@code null} in case of empty model)
 	 * @param contentType the content type selected to render with which should
 	 * match one of the {@link #getSupportedMediaTypes() supported media types}.
 	 * @param exchange the current exchange
 	 * @return {@code Mono} to represent when and if rendering succeeds
 	 */
 	@Override
-	public Mono<Void> render(HandlerResult result, MediaType contentType,
+	public Mono<Void> render(Map<String, ?> model, MediaType contentType,
 			ServerWebExchange exchange) {
 
 		if (logger.isTraceEnabled()) {
-			logger.trace("Rendering view with model " + result.getModel());
+			logger.trace("Rendering view with model " + model);
 		}
 
 		if (contentType != null) {
 			exchange.getResponse().getHeaders().setContentType(contentType);
 		}
 
-		Map<String, Object> mergedModel = getModelAttributes(result, exchange);
-		return renderInternal(mergedModel, exchange);
+		Map<String, Object> mergedModel = getModelAttributes(model, exchange);
+		return renderInternal(mergedModel, contentType, exchange);
 	}
 
 	/**
@@ -114,8 +134,7 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 	 * <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(HandlerResult result, ServerWebExchange exchange) {
-		ModelMap model = result.getModel();
+	protected Map<String, Object> getModelAttributes(Map<String, ?> model, ServerWebExchange exchange) {
 		int size = (model != null ? model.size() : 0);
 
 		Map<String, Object> attributes = new LinkedHashMap<>(size);
@@ -130,11 +149,12 @@ public abstract class AbstractView implements View, ApplicationContextAware {
 	 * Subclasses must implement this method to actually render the view.
 	 * @param renderAttributes combined output Map (never {@code null}),
 	 * with dynamic values taking precedence over static attributes
-	 * @param exchange current exchange
-	 * @return {@code Mono} to represent when and if rendering succeeds
+	 * @param contentType the content type selected to render with which should
+	 * match one of the {@link #getSupportedMediaTypes() supported media types}.
+	 *@param exchange current exchange  @return {@code Mono} to represent when and if rendering succeeds
 	 */
 	protected abstract Mono<Void> renderInternal(Map<String, Object> renderAttributes,
-			ServerWebExchange exchange);
+			MediaType contentType, ServerWebExchange exchange);
 
 
 	@Override

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view;
 
 import java.util.HashMap;
@@ -30,9 +31,7 @@ import org.springframework.http.MediaType;
 import org.springframework.http.codec.EncoderHttpMessageWriter;
 import org.springframework.http.codec.HttpMessageWriter;
 import org.springframework.http.server.reactive.ServerHttpResponse;
-import org.springframework.ui.ModelMap;
 import org.springframework.util.Assert;
-import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.server.ServerWebExchange;
 
 
@@ -105,15 +104,15 @@ public class HttpMessageWriterView implements View {
 	}
 
 	@Override
-	public Mono<Void> render(HandlerResult result, MediaType contentType, ServerWebExchange exchange) {
-		Object value = extractObjectToRender(result);
+	public Mono<Void> render(Map<String, ?> model, MediaType contentType,
+			ServerWebExchange exchange) {
+		Object value = extractObjectToRender(model);
 		return applyMessageWriter(value, contentType, exchange);
 	}
 
-	protected Object extractObjectToRender(HandlerResult result) {
-		ModelMap model = result.getModel();
+	protected Object extractObjectToRender(Map<String, ?> model) {
 		Map<String, Object> map = new HashMap<>(model.size());
-		for (Map.Entry<String, Object> entry : model.entrySet()) {
+		for (Map.Entry<String, ?> entry : model.entrySet()) {
 			if (isEligibleAttribute(entry.getKey(), entry.getValue())) {
 				map.put(entry.getKey(), entry.getValue());
 			}

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view;
 
 import java.util.Locale;
@@ -194,6 +195,7 @@ public class UrlBasedViewResolver extends ViewResolverSupport implements ViewRes
 	protected AbstractUrlBasedView createUrlBasedView(String viewName) {
 		AbstractUrlBasedView view = (AbstractUrlBasedView) BeanUtils.instantiateClass(getViewClass());
 		view.setSupportedMediaTypes(getSupportedMediaTypes());
+		view.setDefaultCharset(getDefaultCharset());
 		view.setUrl(getPrefix() + viewName + getSuffix());
 		return view;
 	}

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

@@ -13,9 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view;
 
 import java.util.List;
+import java.util.Map;
 
 import reactor.core.publisher.Mono;
 
@@ -48,12 +50,13 @@ public interface View {
 	/**
 	 * Render the view based on the given {@link HandlerResult}. Implementations
 	 * can access and use the model or only a specific attribute in it.
-	 * @param result the result from handler execution
+	 * @param model Map with name Strings as keys and corresponding model
+	 * objects as values (Map can also be {@code null} in case of empty model)
 	 * @param contentType the content type selected to render with which should
 	 * match one of the {@link #getSupportedMediaTypes() supported media types}.
 	 * @param exchange the current exchange
 	 * @return {@code Mono} to represent when and if rendering succeeds
 	 */
-	Mono<Void> render(HandlerResult result, MediaType contentType, ServerWebExchange exchange);
+	Mono<Void> render(Map<String, ?> model, MediaType contentType, ServerWebExchange exchange);
 
 }

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view;
 
 import java.lang.reflect.Method;
@@ -205,15 +206,15 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 					.defaultIfEmpty(result.getModel())
 					.then(model -> getDefaultViewNameMono(exchange, result));
 		}
-
+		Map<String, ?> model = result.getModel();
 		return viewMono.then(view -> {
 			if (view instanceof View) {
-				return ((View) view).render(result, null, exchange);
+				return ((View) view).render(model, null, exchange);
 			}
 			else if (view instanceof CharSequence) {
 				String viewName = view.toString();
 				Locale locale = Locale.getDefault(); // TODO
-				return resolveAndRender(viewName, locale, result, exchange);
+				return resolveAndRender(viewName, locale, model, exchange);
 
 			}
 			else {
@@ -305,7 +306,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 	}
 
 	private Mono<? extends Void> resolveAndRender(String viewName, Locale locale,
-			HandlerResult result, ServerWebExchange exchange) {
+			Map<String, ?> model, ServerWebExchange exchange) {
 
 		return Flux.fromIterable(getViewResolvers())
 				.concatMap(resolver -> resolver.resolveViewName(viewName, locale))
@@ -323,7 +324,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler
 						for (View view : views) {
 							for (MediaType supported : view.getSupportedMediaTypes()) {
 								if (supported.isCompatibleWith(bestMediaType)) {
-									return view.render(result, bestMediaType, exchange);
+									return view.render(model, bestMediaType, exchange);
 								}
 							}
 						}

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

@@ -16,6 +16,8 @@
 
 package org.springframework.web.reactive.result.view;
 
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -38,6 +40,8 @@ public abstract class ViewResolverSupport implements ApplicationContextAware, Or
 
 	private List<MediaType> mediaTypes = new ArrayList<>(4);
 
+	private Charset defaultCharset = StandardCharsets.UTF_8;
+
 	private ApplicationContext applicationContext;
 
 	private int order = Integer.MAX_VALUE;
@@ -67,6 +71,25 @@ public abstract class ViewResolverSupport implements ApplicationContextAware, Or
 		return this.mediaTypes;
 	}
 
+	/**
+	 * Set the default charset for this view, used when the
+	 * {@linkplain #setSupportedMediaTypes(List) content type} does not contain one.
+	 * Default is {@linkplain StandardCharsets#UTF_8 UTF 8}.
+	 */
+	public void setDefaultCharset(Charset defaultCharset) {
+		Assert.notNull(defaultCharset, "'defaultCharset' must not be null");
+		this.defaultCharset = defaultCharset;
+	}
+
+	/**
+	 * Return the default charset, used when the
+	 * {@linkplain #setSupportedMediaTypes(List) content type} does not contain one.
+	 */
+	public Charset getDefaultCharset() {
+		return this.defaultCharset;
+	}
+
+
 	@Override
 	public void setApplicationContext(ApplicationContext applicationContext) {
 		this.applicationContext = applicationContext;

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

@@ -13,14 +13,17 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view.freemarker;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.nio.charset.Charset;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 
 import freemarker.core.ParseException;
 import freemarker.template.Configuration;
@@ -37,6 +40,7 @@ import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.context.ApplicationContextException;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.http.MediaType;
 import org.springframework.web.reactive.result.view.AbstractUrlBasedView;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -158,7 +162,8 @@ public class FreeMarkerView extends AbstractUrlBasedView {
 	}
 
 	@Override
-	protected Mono<Void> renderInternal(Map<String, Object> renderAttributes, ServerWebExchange exchange) {
+	protected Mono<Void> renderInternal(Map<String, Object> renderAttributes, MediaType contentType,
+			ServerWebExchange exchange) {
 		// Expose all standard FreeMarker hash models.
 		SimpleHash freeMarkerModel = getTemplateModel(renderAttributes, exchange);
 		if (logger.isDebugEnabled()) {
@@ -167,8 +172,8 @@ public class FreeMarkerView extends AbstractUrlBasedView {
 		Locale locale = Locale.getDefault(); // TODO
 		DataBuffer dataBuffer = exchange.getResponse().bufferFactory().allocateBuffer();
 		try {
-			// TODO: pass charset
-			Writer writer = new OutputStreamWriter(dataBuffer.asOutputStream());
+			Charset charset = getCharset(contentType).orElse(getDefaultCharset());
+			Writer writer = new OutputStreamWriter(dataBuffer.asOutputStream(), charset);
 			getTemplate(locale).process(freeMarkerModel, writer);
 		}
 		catch (IOException ex) {
@@ -181,6 +186,10 @@ public class FreeMarkerView extends AbstractUrlBasedView {
 		return exchange.getResponse().writeWith(Flux.just(dataBuffer));
 	}
 
+	private static Optional<Charset> getCharset(MediaType mediaType) {
+		return mediaType != null ? Optional.ofNullable(mediaType.getCharset()) : Optional.empty();
+	}
+
 	/**
 	 * Build a FreeMarker template model for the given model Map.
 	 * <p>The default implementation builds a {@link SimpleHash}.

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

@@ -25,10 +25,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.junit.Before;
 import org.junit.Test;
 
-import org.springframework.core.MethodParameter;
 import org.springframework.core.codec.CharSequenceEncoder;
 import org.springframework.core.io.buffer.support.DataBufferTestUtils;
 import org.springframework.http.HttpMethod;
@@ -41,18 +39,13 @@ import org.springframework.tests.TestSubscriber;
 import org.springframework.ui.ExtendedModelMap;
 import org.springframework.ui.ModelMap;
 import org.springframework.util.MimeType;
-import org.springframework.web.reactive.HandlerResult;
-import org.springframework.web.reactive.result.ResolvableMethod;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.adapter.DefaultServerWebExchange;
 import org.springframework.web.server.session.DefaultWebSessionManager;
 import org.springframework.web.server.session.WebSessionManager;
 
 import static junit.framework.TestCase.assertTrue;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 
 /**
@@ -63,18 +56,9 @@ public class HttpMessageWriterViewTests {
 
 	private HttpMessageWriterView view = new HttpMessageWriterView(new Jackson2JsonEncoder());
 
-	private HandlerResult result;
-
 	private ModelMap model = new ExtendedModelMap();
 
 
-	@Before
-	public void setup() throws Exception {
-		MethodParameter param = ResolvableMethod.onClass(this.getClass()).name("handle").resolveReturnType();
-		this.result = new HandlerResult(this, null, param, this.model);
-	}
-
-
 	@Test
 	public void supportedMediaTypes() throws Exception {
 		List<MimeType> mimeTypes = Arrays.asList(
@@ -91,7 +75,7 @@ public class HttpMessageWriterViewTests {
 		this.model.addAttribute("foo2", "bar2");
 		this.model.addAttribute("foo3", "bar3");
 
-		assertEquals("bar2", this.view.extractObjectToRender(this.result));
+		assertEquals("bar2", this.view.extractObjectToRender(this.model));
 	}
 
 	@Test
@@ -99,7 +83,7 @@ public class HttpMessageWriterViewTests {
 		this.view.setModelKeys(Collections.singleton("foo2"));
 		this.model.addAttribute("foo1", "bar1");
 
-		assertNull(this.view.extractObjectToRender(this.result));
+		assertNull(this.view.extractObjectToRender(this.model));
 	}
 
 	@Test
@@ -109,7 +93,7 @@ public class HttpMessageWriterViewTests {
 		this.model.addAttribute("foo2", "bar2");
 		this.model.addAttribute("foo3", "bar3");
 
-		Object value = this.view.extractObjectToRender(this.result);
+		Object value = this.view.extractObjectToRender(this.model);
 		assertNotNull(value);
 		assertEquals(HashMap.class, value.getClass());
 
@@ -127,7 +111,7 @@ public class HttpMessageWriterViewTests {
 		this.model.addAttribute("foo2", "bar2");
 
 		try {
-			view.extractObjectToRender(this.result);
+			view.extractObjectToRender(this.model);
 			fail();
 		}
 		catch (IllegalStateException ex) {
@@ -143,7 +127,7 @@ public class HttpMessageWriterViewTests {
 		this.model.addAttribute("foo1", "bar1");
 
 		try {
-			view.extractObjectToRender(this.result);
+			view.extractObjectToRender(this.model);
 			fail();
 		}
 		catch (IllegalStateException ex) {
@@ -165,7 +149,7 @@ public class HttpMessageWriterViewTests {
 		WebSessionManager manager = new DefaultWebSessionManager();
 		ServerWebExchange exchange = new DefaultServerWebExchange(request, response, manager);
 
-		this.view.render(result, MediaType.APPLICATION_JSON, exchange);
+		this.view.render(this.model, MediaType.APPLICATION_JSON, exchange);
 
 		TestSubscriber
 				.subscribe(response.getBody())

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view;
 
 import java.util.Locale;
@@ -22,6 +23,7 @@ import org.junit.Test;
 import reactor.core.publisher.Mono;
 
 import org.springframework.context.support.StaticApplicationContext;
+import org.springframework.http.MediaType;
 import org.springframework.web.server.ServerWebExchange;
 
 import static org.junit.Assert.assertNotNull;
@@ -61,7 +63,8 @@ public class UrlBasedViewResolverTests {
 		}
 
 		@Override
-		protected Mono<Void> renderInternal(Map<String, Object> attributes, ServerWebExchange exchange) {
+		protected Mono<Void> renderInternal(Map<String, Object> attributes, MediaType contentType,
+				ServerWebExchange exchange) {
 			return Mono.empty();
 		}
 	}

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

@@ -347,8 +347,9 @@ public class ViewResolutionResultHandlerTests {
 		}
 
 		@Override
-		public Mono<Void> render(HandlerResult result, MediaType mediaType, ServerWebExchange exchange) {
-			String value = this.name + ": " + result.getModel().toString();
+		public Mono<Void> render(Map<String, ?> model, MediaType mediaType,
+				ServerWebExchange exchange) {
+			String value = this.name + ": " + model.toString();
 			assertNotNull(value);
 			ServerHttpResponse response = exchange.getResponse();
 			if (mediaType != null) {

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.reactive.result.view.freemarker;
 
 import java.nio.ByteBuffer;
@@ -27,7 +28,6 @@ import org.junit.rules.ExpectedException;
 
 import org.springframework.context.ApplicationContextException;
 import org.springframework.context.support.GenericApplicationContext;
-import org.springframework.core.MethodParameter;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.http.HttpMethod;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
@@ -35,7 +35,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse
 import org.springframework.tests.TestSubscriber;
 import org.springframework.ui.ExtendedModelMap;
 import org.springframework.ui.ModelMap;
-import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.adapter.DefaultServerWebExchange;
 import org.springframework.web.server.session.DefaultWebSessionManager;
@@ -122,9 +121,7 @@ public class FreeMarkerViewTests {
 
 		ModelMap model = new ExtendedModelMap();
 		model.addAttribute("hello", "hi FreeMarker");
-		MethodParameter returnType = new MethodParameter(getClass().getDeclaredMethod("handle"), -1);
-		HandlerResult result = new HandlerResult(new Object(), "", returnType, model);
-		view.render(result, null, this.exchange);
+		view.render(model, null, this.exchange);
 
 		TestSubscriber
 				.subscribe(this.response.getBody())