Ensure RedirectModel is initialized

This commit fixes an old bug in ModelAndViewContainer where getModel
returns a new ModelMap instance that isn't saved and re-used.

Issue: SPR-14045
(cherry picked from commit d7062f6)

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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,24 +44,35 @@
  */
 public class ModelAndViewContainer {
 
-	private Object view;
+	private boolean ignoreDefaultModelOnRedirect = false;
 
-	private boolean requestHandled = false;
+	private Object view;
 
 	private final ModelMap defaultModel = new BindingAwareModelMap();
 
 	private ModelMap redirectModel;
 
 	private boolean redirectModelScenario = false;
 
-	private boolean ignoreDefaultModelOnRedirect = false;
-
 	private final SessionStatus sessionStatus = new SimpleSessionStatus();
 
+	private boolean requestHandled = false;
+
+
 	/**
-	 * Create a new instance.
+	 * By default the content of the "default" model is used both during
+	 * rendering and redirect scenarios. Alternatively controller methods
+	 * can declare an argument of type {@code RedirectAttributes} and use
+	 * it to provide attributes to prepare the redirect URL.
+	 * <p>Setting this flag to {@code true} guarantees the "default" model is
+	 * never used in a redirect scenario even if a RedirectAttributes argument
+	 * is not declared. Setting it to {@code false} means the "default" model
+	 * may be used in a redirect if the controller method doesn't declare a
+	 * RedirectAttributes argument.
+	 * <p>The default setting is {@code false}.
 	 */
-	public ModelAndViewContainer() {
+	public void setIgnoreDefaultModelOnRedirect(boolean ignoreDefaultModelOnRedirect) {
+		this.ignoreDefaultModelOnRedirect = ignoreDefaultModelOnRedirect;
 	}
 
 	/**
@@ -105,47 +116,28 @@ public boolean isViewReference() {
 	}
 
 	/**
-	 * Signal a scenario where the request is handled directly.
-	 * <p>A {@link HandlerMethodReturnValueHandler} may use this flag to
-	 * indicate the response has been fully handled and view resolution
-	 * is not required (e.g. {@code @ResponseBody}).
-	 * <p>A {@link HandlerMethodArgumentResolver} may also use this flag
-	 * to indicate the presence of an argument (e.g.
-	 * {@code ServletResponse} or {@code OutputStream}) that may lead to
-	 * a complete response depending on the method return value.
-	 * <p>The default value is {@code true}.
-	 */
-	public void setRequestHandled(boolean requestHandled) {
-		this.requestHandled = requestHandled;
-	}
-
-	/**
-	 * Whether the request is handled directly.
-	 */
-	public boolean isRequestHandled() {
-		return this.requestHandled;
-	}
-
-	/**
-	 * Return the model to use: the "default" or the "redirect" model.
-	 * <p>The default model is used if {@code "redirectModelScenario=false"} or
-	 * if the redirect model is {@code null} (i.e. it wasn't declared as a
-	 * method argument) and {@code ignoreDefaultModelOnRedirect=false}.
+	 * Return the model to use: either the "default" or the "redirect" model.
+	 * The default model is used if {@code redirectModelScenario=false} or
+	 * there is no redirect model (i.e. RedirectAttributes was not declared as
+	 * a method argument) and {@code ignoreDefaultModelOnRedirect=false}.
 	 */
 	public ModelMap getModel() {
 		if (useDefaultModel()) {
 			return this.defaultModel;
 		}
 		else {
-			return (this.redirectModel != null) ? this.redirectModel : new ModelMap();
+			if (this.redirectModel == null) {
+				this.redirectModel = new ModelMap();
+			}
+			return this.redirectModel;
 		}
 	}
 
 	/**
 	 * Whether to use the default model or the redirect model.
 	 */
 	private boolean useDefaultModel() {
-		return !this.redirectModelScenario || ((this.redirectModel == null) && !this.ignoreDefaultModelOnRedirect);
+		return (!this.redirectModelScenario || (this.redirectModel == null && !this.ignoreDefaultModelOnRedirect));
 	}
 
 	/**
@@ -159,31 +151,37 @@ public void setRedirectModel(ModelMap redirectModel) {
 	}
 
 	/**
-	 * Signal the conditions are in place for using a redirect model.
-	 * Typically that means the controller has returned a redirect instruction.
+	 * Whether the controller has returned a redirect instruction, e.g. a
+	 * "redirect:" prefixed view name, a RedirectView instance, etc.
 	 */
 	public void setRedirectModelScenario(boolean redirectModelScenario) {
 		this.redirectModelScenario = redirectModelScenario;
 	}
 
 	/**
-	 * When set to {@code true} the default model is never used in a redirect
-	 * scenario. So if a redirect model is not available, an empty model is
-	 * used instead.
-	 * <p>When set to {@code false} the default model can be used in a redirect
-	 * scenario if a redirect model is not available.
-	 * <p>The default setting is {@code false}.
+	 * Return the {@link SessionStatus} instance to use that can be used to
+	 * signal that session processing is complete.
 	 */
-	public void setIgnoreDefaultModelOnRedirect(boolean ignoreDefaultModelOnRedirect) {
-		this.ignoreDefaultModelOnRedirect = ignoreDefaultModelOnRedirect;
+	public SessionStatus getSessionStatus() {
+		return this.sessionStatus;
 	}
 
 	/**
-	 * Return the {@link SessionStatus} instance to use that can be used to
-	 * signal that session processing is complete.
+	 * Whether the request has been handled fully within the handler, e.g.
+	 * {@code @ResponseBody} method, and therefore view resolution is not
+	 * necessary. This flag can also be set when controller methods declare an
+	 * argument of type {@code ServletResponse} or {@code OutputStream}).
+	 * <p>The default value is {@code false}.
 	 */
-	public SessionStatus getSessionStatus() {
-		return sessionStatus;
+	public void setRequestHandled(boolean requestHandled) {
+		this.requestHandled = requestHandled;
+	}
+
+	/**
+	 * Whether the request has been handled fully within the handler.
+	 */
+	public boolean isRequestHandled() {
+		return this.requestHandled;
 	}
 
 	/**
@@ -243,6 +241,7 @@ public boolean containsAttribute(String name) {
 		return getModel().containsAttribute(name);
 	}
 
+
 	/**
 	 * Return diagnostic information.
 	 */

spring-web/src/test/java/org/springframework/web/method/support/ModelAndViewContainerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -16,12 +16,13 @@
 
 package org.springframework.web.method.support;
 
-import static org.junit.Assert.assertEquals;
-
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.ui.ModelMap;
 
+import static org.junit.Assert.*;
+
 /**
  * Test fixture for {@link ModelAndViewContainer}.
  *
@@ -32,44 +33,57 @@ public class ModelAndViewContainerTests {
 
 	private ModelAndViewContainer mavContainer;
 
+
 	@Before
 	public void setup() {
 		this.mavContainer = new ModelAndViewContainer();
 	}
 
+
 	@Test
 	public void getModel() {
 		this.mavContainer.addAttribute("name", "value");
 		assertEquals(1, this.mavContainer.getModel().size());
+		assertEquals("value", this.mavContainer.getModel().get("name"));
 	}
 
 	@Test
-	public void getModelRedirectModel() {
-		ModelMap redirectModel = new ModelMap("name", "redirectValue");
-		this.mavContainer.setRedirectModel(redirectModel);
-		this.mavContainer.addAttribute("name", "value");
+	public void redirectScenarioWithRedirectModel() {
+		this.mavContainer.addAttribute("name1", "value1");
+		this.mavContainer.setRedirectModel(new ModelMap("name2", "value2"));
+		this.mavContainer.setRedirectModelScenario(true);
 
-		assertEquals("Default model should be used if not in redirect scenario",
-				"value", this.mavContainer.getModel().get("name"));
+		assertEquals(1, this.mavContainer.getModel().size());
+		assertEquals("value2", this.mavContainer.getModel().get("name2"));
+	}
 
+	@Test
+	public void redirectScenarioWithoutRedirectModel() {
+		this.mavContainer.addAttribute("name", "value");
 		this.mavContainer.setRedirectModelScenario(true);
 
-		assertEquals("Redirect model should be used in redirect scenario",
-				"redirectValue", this.mavContainer.getModel().get("name"));
+		assertEquals(1, this.mavContainer.getModel().size());
+		assertEquals("value", this.mavContainer.getModel().get("name"));
 	}
 
 	@Test
-	public void getModelIgnoreDefaultModelOnRedirect() {
+	public void ignoreDefaultModel() {
+		this.mavContainer.setIgnoreDefaultModelOnRedirect(true);
 		this.mavContainer.addAttribute("name", "value");
 		this.mavContainer.setRedirectModelScenario(true);
 
-		assertEquals("Default model should be used since no redirect model was provided",
-				1, this.mavContainer.getModel().size());
+		assertTrue(this.mavContainer.getModel().isEmpty());
+	}
 
+	@Test  // SPR-14045
+	public void ignoreDefaultModelAndWithoutRedirectModel() {
 		this.mavContainer.setIgnoreDefaultModelOnRedirect(true);
+		this.mavContainer.setRedirectModelScenario(true);
+		this.mavContainer.addAttribute("name", "value");
 
-		assertEquals("Empty model should be returned if no redirect model is available",
-				0, this.mavContainer.getModel().size());
+		assertEquals(1, this.mavContainer.getModel().size());
+		assertEquals("value", this.mavContainer.getModel().get("name"));
 	}
 
+
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ModelAndViewMethodReturnValueHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -16,16 +16,11 @@
 
 package org.springframework.web.servlet.mvc.method.annotation;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-
 import java.lang.reflect.Method;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.ui.ModelMap;
@@ -35,6 +30,8 @@
 import org.springframework.web.servlet.mvc.support.RedirectAttributesModelMap;
 import org.springframework.web.servlet.view.RedirectView;
 
+import static org.junit.Assert.*;
+
 /**
  * Test fixture with {@link ModelAndViewMethodReturnValueHandler}.
  *
@@ -50,6 +47,7 @@ public class ModelAndViewMethodReturnValueHandlerTests {
 
 	private MethodParameter returnParamModelAndView;
 
+
 	@Before
 	public void setUp() throws Exception {
 		this.handler = new ModelAndViewMethodReturnValueHandler();
@@ -58,6 +56,7 @@ public void setUp() throws Exception {
 		this.returnParamModelAndView = getReturnValueParam("modelAndView");
 	}
 
+
 	@Test
 	public void supportsReturnType() throws Exception {
 		assertTrue(handler.supportsReturnType(returnParamModelAndView));
@@ -131,16 +130,33 @@ public void handleRedirectAttributesWithoutRedirect() throws Exception {
 		assertNotSame("RedirectAttributes should not be used if controller doesn't redirect", redirectAttributes, model);
 	}
 
+	@Test  // SPR-14045
+	public void handleRedirectWithIgnoreDefaultModel() throws Exception {
+		mavContainer.setIgnoreDefaultModelOnRedirect(true);
+
+		RedirectView redirectView = new RedirectView();
+		ModelAndView mav = new ModelAndView(redirectView, "name", "value");
+		handler.handleReturnValue(mav, returnParamModelAndView, mavContainer, webRequest);
+
+		ModelMap model = mavContainer.getModel();
+		assertSame(redirectView, mavContainer.getView());
+		assertEquals(1, model.size());
+		assertEquals("value", model.get("name"));
+	}
+
 
 	private MethodParameter getReturnValueParam(String methodName) throws Exception {
 		Method method = getClass().getDeclaredMethod(methodName);
 		return new MethodParameter(method, -1);
 	}
 
+
+	@SuppressWarnings("unused")
 	ModelAndView modelAndView() {
 		return null;
 	}
 
+	@SuppressWarnings("unused")
 	String viewName() {
 		return null;
 	}