ModelAttributeMethodProcessor detects re-enabled binding declaration

Issue: SPR-16083

(cherry picked from commit bec1fc1)

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

@@ -65,7 +65,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * Class constructor.
 	 * @param annotationNotRequired if "true", non-simple method arguments and
 	 * return values are considered model attributes with or without a
-	 * {@code @ModelAttribute} annotation.
+	 * {@code @ModelAttribute} annotation
 	 */
 	public ModelAttributeMethodProcessor(boolean annotationNotRequired) {
 		this.annotationNotRequired = annotationNotRequired;
@@ -89,24 +89,22 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * with request values via data binding and optionally validated
 	 * if {@code @java.validation.Valid} is present on the argument.
 	 * @throws BindException if data binding and validation result in an error
-	 * and the next method parameter is not of type {@link Errors}.
-	 * @throws Exception if WebDataBinder initialization fails.
+	 * and the next method parameter is not of type {@link Errors}
+	 * @throws Exception if WebDataBinder initialization fails
 	 */
 	@Override
 	public final Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
 			NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception {
 
 		String name = ModelFactory.getNameForParameter(parameter);
+		ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class);
+		if (ann != null) {
+			mavContainer.setBinding(name, ann.binding());
+		}
+
 		Object attribute = (mavContainer.containsAttribute(name) ? mavContainer.getModel().get(name) :
 				createAttribute(name, parameter, binderFactory, webRequest));
 
-		if (!mavContainer.isBindingDisabled(name)) {
-			ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class);
-			if (ann != null && !ann.binding()) {
-				mavContainer.setBindingDisabled(name);
-			}
-		}
-
 		WebDataBinder binder = binderFactory.createBinder(webRequest, attribute, name);
 		if (binder.getTarget() != null) {
 			if (!mavContainer.isBindingDisabled(name)) {
@@ -130,15 +128,15 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * Extension point to create the model attribute if not found in the model.
 	 * The default implementation uses the default constructor.
 	 * @param attributeName the name of the attribute (never {@code null})
-	 * @param methodParam the method parameter
+	 * @param parameter the method parameter
 	 * @param binderFactory for creating WebDataBinder instance
-	 * @param request the current request
+	 * @param webRequest the current request
 	 * @return the created model attribute (never {@code null})
 	 */
-	protected Object createAttribute(String attributeName, MethodParameter methodParam,
-			WebDataBinderFactory binderFactory, NativeWebRequest request) throws Exception {
+	protected Object createAttribute(String attributeName, MethodParameter parameter,
+			WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception {
 
-		return BeanUtils.instantiateClass(methodParam.getParameterType());
+		return BeanUtils.instantiateClass(parameter.getParameterType());
 	}
 
 	/**
@@ -156,10 +154,10 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * Spring's {@link org.springframework.validation.annotation.Validated},
 	 * and custom annotations whose name starts with "Valid".
 	 * @param binder the DataBinder to be used
-	 * @param methodParam the method parameter
+	 * @param parameter the method parameter declaration
 	 */
-	protected void validateIfApplicable(WebDataBinder binder, MethodParameter methodParam) {
-		Annotation[] annotations = methodParam.getParameterAnnotations();
+	protected void validateIfApplicable(WebDataBinder binder, MethodParameter parameter) {
+		Annotation[] annotations = parameter.getParameterAnnotations();
 		for (Annotation ann : annotations) {
 			Validated validatedAnn = AnnotationUtils.getAnnotation(ann, Validated.class);
 			if (validatedAnn != null || ann.annotationType().getSimpleName().startsWith("Valid")) {
@@ -174,12 +172,12 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	/**
 	 * Whether to raise a fatal bind exception on validation errors.
 	 * @param binder the data binder used to perform data binding
-	 * @param methodParam the method argument
-	 * @return {@code true} if the next method argument is not of type {@link Errors}
+	 * @param parameter the method parameter declaration
+	 * @return {@code true} if the next method parameter is not of type {@link Errors}
 	 */
-	protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter methodParam) {
-		int i = methodParam.getParameterIndex();
-		Class<?>[] paramTypes = methodParam.getMethod().getParameterTypes();
+	protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter parameter) {
+		int i = parameter.getParameterIndex();
+		Class<?>[] paramTypes = parameter.getMethod().getParameterTypes();
 		boolean hasBindingResult = (paramTypes.length > (i + 1) && Errors.class.isAssignableFrom(paramTypes[i + 1]));
 		return !hasBindingResult;
 	}

spring-web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.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.
@@ -57,11 +57,12 @@ public class ModelAndViewContainer {
 
 	private boolean redirectModelScenario = false;
 
-	/* Names of attributes with binding disabled */
-	private final Set<String> bindingDisabledAttributes = new HashSet<String>(4);
-
 	private HttpStatus status;
 
+	private final Set<String> noBinding = new HashSet<String>(4);
+
+	private final Set<String> bindingDisabled = new HashSet<String>(4);
+
 	private final SessionStatus sessionStatus = new SimpleSessionStatus();
 
 	private boolean requestHandled = false;
@@ -141,24 +142,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.
 	 */
@@ -199,15 +182,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
 	 */
@@ -223,6 +198,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/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessor.java

@@ -43,6 +43,7 @@ import org.springframework.web.servlet.HandlerMapping;
  * model attribute name and there is an appropriate type conversion strategy.
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  * @since 3.1
  */
 public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodProcessor {
@@ -66,19 +67,19 @@ public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodPr
 	 * @see #createAttributeFromRequestValue
 	 */
 	@Override
-	protected final Object createAttribute(String attributeName, MethodParameter methodParam,
+	protected final Object createAttribute(String attributeName, MethodParameter parameter,
 			WebDataBinderFactory binderFactory, NativeWebRequest request) throws Exception {
 
 		String value = getRequestValueForAttribute(attributeName, request);
 		if (value != null) {
 			Object attribute = createAttributeFromRequestValue(
-					value, attributeName, methodParam, binderFactory, request);
+					value, attributeName, parameter, binderFactory, request);
 			if (attribute != null) {
 				return attribute;
 			}
 		}
 
-		return super.createAttribute(attributeName, methodParam, binderFactory, request);
+		return super.createAttribute(attributeName, parameter, binderFactory, request);
 	}
 
 	/**
@@ -117,24 +118,23 @@ public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodPr
 	 * {@link Converter} that can perform the conversion.
 	 * @param sourceValue the source value to create the model attribute from
 	 * @param attributeName the name of the attribute (never {@code null})
-	 * @param methodParam the method parameter
+	 * @param parameter the method parameter
 	 * @param binderFactory for creating WebDataBinder instance
 	 * @param request the current request
 	 * @return the created model attribute, or {@code null} if no suitable
 	 * conversion found
-	 * @throws Exception
 	 */
 	protected Object createAttributeFromRequestValue(String sourceValue, String attributeName,
-			MethodParameter methodParam, WebDataBinderFactory binderFactory, NativeWebRequest request)
+			MethodParameter parameter, WebDataBinderFactory binderFactory, NativeWebRequest request)
 			throws Exception {
 
 		DataBinder binder = binderFactory.createBinder(request, null, attributeName);
 		ConversionService conversionService = binder.getConversionService();
 		if (conversionService != null) {
 			TypeDescriptor source = TypeDescriptor.valueOf(String.class);
-			TypeDescriptor target = new TypeDescriptor(methodParam);
+			TypeDescriptor target = new TypeDescriptor(parameter);
 			if (conversionService.canConvert(source, target)) {
-				return binder.convertIfNecessary(sourceValue, methodParam.getParameterType(), methodParam);
+				return binder.convertIfNecessary(sourceValue, parameter.getParameterType(), parameter);
 			}
 		}
 		return null;

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.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.
@@ -159,6 +159,7 @@ import static org.junit.Assert.*;
  * </ul>
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  */
 public class ServletAnnotationControllerHandlerMethodTests extends AbstractServletHandlerMethodTests {
 
@@ -580,6 +581,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(new ApplicationContextInitializer<GenericWebApplicationContext>() {
@@ -2157,6 +2172,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 {