ModelAttributeMethodProcessor detects re-enabled binding declaration

Issue: SPR-16083

spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

@@ -115,11 +115,9 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 		Assert.state(binderFactory != null, "ModelAttributeMethodProcessor requires WebDataBinderFactory");
 
 		String name = ModelFactory.getNameForParameter(parameter);
-		if (!mavContainer.isBindingDisabled(name)) {
-			ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class);
-			if (ann != null && !ann.binding()) {
-				mavContainer.setBindingDisabled(name);
-			}
+		ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class);
+		if (ann != null) {
+			mavContainer.setBinding(name, ann.binding());
 		}
 
 		Object attribute = null;

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

@@ -44,6 +44,7 @@ import org.springframework.web.bind.support.SimpleSessionStatus;
  * returns the redirect model instead of the default model.
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  * @since 3.1
  */
 public class ModelAndViewContainer {
@@ -60,12 +61,13 @@ public class ModelAndViewContainer {
 
 	private boolean redirectModelScenario = false;
 
-	/* Names of attributes with binding disabled */
-	private final Set<String> bindingDisabledAttributes = new HashSet<>(4);
-
 	@Nullable
 	private HttpStatus status;
 
+	private final Set<String> noBinding = new HashSet<>(4);
+
+	private final Set<String> bindingDisabled = new HashSet<>(4);
+
 	private final SessionStatus sessionStatus = new SimpleSessionStatus();
 
 	private boolean requestHandled = false;
@@ -147,24 +149,6 @@ public class ModelAndViewContainer {
 		}
 	}
 
-	/**
-	 * Register an attribute for which data binding should not occur, for example
-	 * corresponding to an {@code @ModelAttribute(binding=false)} declaration.
-	 * @param attributeName the name of the attribute
-	 * @since 4.3
-	 */
-	public void setBindingDisabled(String attributeName) {
-		this.bindingDisabledAttributes.add(attributeName);
-	}
-
-	/**
-	 * Whether binding is disabled for the given model attribute.
-	 * @since 4.3
-	 */
-	public boolean isBindingDisabled(String name) {
-		return this.bindingDisabledAttributes.contains(name);
-	}
-
 	/**
 	 * Whether to use the default model or the redirect model.
 	 */
@@ -205,15 +189,7 @@ public class ModelAndViewContainer {
 	}
 
 	/**
-	 * Return the {@link SessionStatus} instance to use that can be used to
-	 * signal that session processing is complete.
-	 */
-	public SessionStatus getSessionStatus() {
-		return this.sessionStatus;
-	}
-
-	/**
-	 * Provide a HTTP status that will be passed on to with the
+	 * Provide an HTTP status that will be passed on to with the
 	 * {@code ModelAndView} used for view rendering purposes.
 	 * @since 4.3
 	 */
@@ -230,6 +206,49 @@ public class ModelAndViewContainer {
 		return this.status;
 	}
 
+	/**
+	 * Programmatically register an attribute for which data binding should not occur,
+	 * not even for a subsequent {@code @ModelAttribute} declaration.
+	 * @param attributeName the name of the attribute
+	 * @since 4.3
+	 */
+	public void setBindingDisabled(String attributeName) {
+		this.bindingDisabled.add(attributeName);
+	}
+
+	/**
+	 * Whether binding is disabled for the given model attribute.
+	 * @since 4.3
+	 */
+	public boolean isBindingDisabled(String name) {
+		return (this.bindingDisabled.contains(name) || this.noBinding.contains(name));
+	}
+
+	/**
+	 * Register whether data binding should occur for a corresponding model attribute,
+	 * corresponding to an {@code @ModelAttribute(binding=true/false)} declaration.
+	 * <p>Note: While this flag will be taken into account by {@link #isBindingDisabled},
+	 * a hard {@link #setBindingDisabled} declaration will always override it.
+	 * @param attributeName the name of the attribute
+	 * @since 4.3.13
+	 */
+	public void setBinding(String attributeName, boolean enabled) {
+		if (!enabled) {
+			this.noBinding.add(attributeName);
+		}
+		else {
+			this.noBinding.remove(attributeName);
+		}
+	}
+
+	/**
+	 * Return the {@link SessionStatus} instance to use that can be used to
+	 * signal that session processing is complete.
+	 */
+	public SessionStatus getSessionStatus() {
+		return this.sessionStatus;
+	}
+
 	/**
 	 * Whether the request has been handled fully within the handler, e.g.
 	 * {@code @ResponseBody} method, and therefore view resolution is not

spring-web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -49,7 +49,6 @@ import static org.junit.Assert.fail;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.BDDMockito.mock;
 
-
 /**
  * Text fixture for {@link ModelFactory} tests.
  *
@@ -158,7 +157,7 @@ public class ModelFactoryTests {
 			modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 			fail("Expected HttpSessionRequiredException");
 		}
-		catch (HttpSessionRequiredException e) {
+		catch (HttpSessionRequiredException ex) {
 			// expected
 		}
 
@@ -229,9 +228,7 @@ public class ModelFactoryTests {
 		assertNull(this.attributeStore.retrieveAttribute(this.webRequest, attributeName));
 	}
 
-	// SPR-12542
-
-	@Test
+	@Test  // SPR-12542
 	public void updateModelWhenRedirecting() throws Exception {
 		String attributeName = "sessionAttr";
 		String attribute = "value";
@@ -274,8 +271,8 @@ public class ModelFactoryTests {
 	}
 
 
-	@SessionAttributes({"sessionAttr", "foo"}) @SuppressWarnings("unused")
-	private static class TestController {
+	@SessionAttributes({"sessionAttr", "foo"})
+	static class TestController {
 
 		@ModelAttribute
 		public void modelAttr(Model model) {
@@ -309,6 +306,7 @@ public class ModelFactoryTests {
 		}
 	}
 
+
 	private static class Foo {
 	}
 

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

@@ -146,6 +146,7 @@ import static org.junit.Assert.*;
 
 /**
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  */
 public class ServletAnnotationControllerHandlerMethodTests extends AbstractServletHandlerMethodTests {
 
@@ -535,6 +536,20 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		assertEquals("myPath-name1-typeMismatch-tb1-myValue-yourValue", response.getContentAsString());
 	}
 
+	@Test
+	public void lateBindingFormController() throws Exception {
+		initServlet(
+				wac -> wac.registerBeanDefinition("viewResolver", new RootBeanDefinition(TestViewResolver.class)),
+				LateBindingFormController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/myPath.do");
+		request.addParameter("name", "name1");
+		request.addParameter("age", "value2");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		getServlet().service(request, response);
+		assertEquals("myView-name1-typeMismatch-tb1-myValue", response.getContentAsString());
+	}
+
 	@Test
 	public void proxiedFormController() throws Exception {
 		initServlet(wac -> {
@@ -2224,6 +2239,29 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		}
 	}
 
+	@Controller
+	public static class LateBindingFormController {
+
+		@ModelAttribute("testBeanList")
+		public List<TestBean> getTestBeans(@ModelAttribute(name="myCommand", binding=false) TestBean tb) {
+			List<TestBean> list = new LinkedList<>();
+			list.add(new TestBean("tb1"));
+			list.add(new TestBean("tb2"));
+			return list;
+		}
+
+		@RequestMapping("/myPath.do")
+		public String myHandle(@ModelAttribute(name="myCommand", binding=true) TestBean tb, BindingResult errors, ModelMap model) {
+			FieldError error = errors.getFieldError("age");
+			assertNotNull("Must have field error for age property", error);
+			assertEquals("value2", error.getRejectedValue());
+			if (!model.containsKey("myKey")) {
+				model.addAttribute("myKey", "myValue");
+			}
+			return "myView";
+		}
+	}
+
 	@Controller
 	static class MyCommandProvidingFormController<T, TB, TB2> extends MyFormController {