diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java index 755de18e31..a3c867ec52 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java b/spring-web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java index ec97682464..f0cca06bd7 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java +++ b/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 bindingDisabledAttributes = new HashSet(4); - private HttpStatus status; + private final Set noBinding = new HashSet(4); + + private final Set bindingDisabled = new HashSet(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. + *

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 diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java index 24c618255a..ee84950b35 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java +++ b/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 { } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessor.java index 293b27d6b6..b74f6f9687 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessor.java +++ b/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; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 1504318893..06fd3cce6c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/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.*; * * * @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() { @@ -2157,6 +2172,29 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } + @Controller + public static class LateBindingFormController { + + @ModelAttribute("testBeanList") + public List getTestBeans(@ModelAttribute(name="myCommand", binding=false) TestBean tb) { + List 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 extends MyFormController {