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 5ac23425f61..a14c85888f8 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 @@ -17,10 +17,10 @@ package org.springframework.web.method.annotation; import java.lang.annotation.Annotation; +import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.beans.BeanUtils; import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotationUtils; @@ -31,7 +31,6 @@ import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.bind.support.WebRequestDataBinder; import org.springframework.web.context.request.NativeWebRequest; -import org.springframework.web.method.annotation.ModelFactory; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; @@ -99,10 +98,10 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol throws Exception { String name = ModelFactory.getNameForParameter(parameter); - Object target = (mavContainer.containsAttribute(name)) ? + Object attribute = (mavContainer.containsAttribute(name)) ? mavContainer.getModel().get(name) : createAttribute(name, parameter, binderFactory, request); - WebDataBinder binder = binderFactory.createBinder(request, target, name); + WebDataBinder binder = binderFactory.createBinder(request, attribute, name); if (binder.getTarget() != null) { bindRequestParameters(binder, request); validateIfApplicable(binder, parameter); @@ -113,7 +112,12 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } } - mavContainer.addAllAttributes(binder.getBindingResult().getModel()); + // Add resolved attribute and BindingResult at the end of the model + + Map bindingResultModel = binder.getBindingResult().getModel(); + mavContainer.removeAttributes(bindingResultModel); + mavContainer.addAllAttributes(bindingResultModel); + return binder.getTarget(); } 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 94096df0062..e8036f7d4a1 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-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -25,29 +25,29 @@ import org.springframework.web.bind.support.SessionStatus; import org.springframework.web.bind.support.SimpleSessionStatus; /** - * Records model and view related decisions made by - * {@link HandlerMethodArgumentResolver}s and - * {@link HandlerMethodReturnValueHandler}s during the course of invocation of + * Records model and view related decisions made by + * {@link HandlerMethodArgumentResolver}s and + * {@link HandlerMethodReturnValueHandler}s during the course of invocation of * a controller method. - * + * *

The {@link #setRequestHandled} flag can be used to indicate the request * has been handled directly and view resolution is not required. - * - *

A default {@link Model} is automatically created at instantiation. - * An alternate model instance may be provided via {@link #setRedirectModel} - * for use in a redirect scenario. When {@link #setRedirectModelScenario} is set - * to {@code true} signalling a redirect scenario, the {@link #getModel()} + * + *

A default {@link Model} is automatically created at instantiation. + * An alternate model instance may be provided via {@link #setRedirectModel} + * for use in a redirect scenario. When {@link #setRedirectModelScenario} is set + * to {@code true} signalling a redirect scenario, the {@link #getModel()} * returns the redirect model instead of the default model. - * + * * @author Rossen Stoyanchev * @since 3.1 */ public class ModelAndViewContainer { private Object view; - + private boolean requestHandled = false; - + private final ModelMap defaultModel = new BindingAwareModelMap(); private ModelMap redirectModel; @@ -57,7 +57,7 @@ public class ModelAndViewContainer { private boolean ignoreDefaultModelOnRedirect = false; private final SessionStatus sessionStatus = new SimpleSessionStatus(); - + /** * Create a new instance. */ @@ -65,7 +65,7 @@ public class ModelAndViewContainer { } /** - * Set a view name to be resolved by the DispatcherServlet via a ViewResolver. + * Set a view name to be resolved by the DispatcherServlet via a ViewResolver. * Will override any pre-existing view name or View. */ public void setViewName(String viewName) { @@ -73,15 +73,15 @@ public class ModelAndViewContainer { } /** - * Return the view name to be resolved by the DispatcherServlet via a + * Return the view name to be resolved by the DispatcherServlet via a * ViewResolver, or {@code null} if a View object is set. */ public String getViewName() { return (this.view instanceof String ? (String) this.view : null); } - + /** - * Set a View object to be used by the DispatcherServlet. + * Set a View object to be used by the DispatcherServlet. * Will override any pre-existing view name or View. */ public void setView(Object view) { @@ -97,28 +97,28 @@ public class ModelAndViewContainer { } /** - * Whether the view is a view reference specified via a name to be + * Whether the view is a view reference specified via a name to be * resolved by the DispatcherServlet via a ViewResolver. */ public boolean isViewReference() { return (this.view instanceof String); } - + /** - * Signal a scenario where the request is handled directly. - *

A {@link HandlerMethodReturnValueHandler} may use this flag to - * indicate the response has been fully handled and view resolution + * Signal a scenario where the request is handled directly. + *

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}). *

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 + * 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. *

The default value is {@code true}. */ public void setRequestHandled(boolean requestHandled) { this.requestHandled = requestHandled; } - + /** * Whether the request is handled directly. */ @@ -129,7 +129,7 @@ public class ModelAndViewContainer { /** * Return the model to use: the "default" or the "redirect" model. *

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 + * if the redirect model is {@code null} (i.e. it wasn't declared as a * method argument) and {@code ignoreDefaultModelOnRedirect=false}. */ public ModelMap getModel() { @@ -140,17 +140,17 @@ public class ModelAndViewContainer { return (this.redirectModel != null) ? this.redirectModel : new ModelMap(); } } - + /** * Whether to use the default model or the redirect model. */ private boolean useDefaultModel() { return !this.redirectModelScenario || ((this.redirectModel == null) && !this.ignoreDefaultModelOnRedirect); } - + /** - * Provide a separate model instance to use in a redirect scenario. - * The provided additional model however is not used used unless + * Provide a separate model instance to use in a redirect scenario. + * The provided additional model however is not used used unless * {@link #setRedirectModelScenario(boolean)} gets set to {@code true} to signal * a redirect scenario. */ @@ -168,7 +168,7 @@ public class ModelAndViewContainer { /** * 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 + * scenario. So if a redirect model is not available, an empty model is * used instead. *

When set to {@code false} the default model can be used in a redirect * scenario if a redirect model is not available. @@ -179,7 +179,7 @@ public class ModelAndViewContainer { } /** - * Return the {@link SessionStatus} instance to use that can be used to + * Return the {@link SessionStatus} instance to use that can be used to * signal that session processing is complete. */ public SessionStatus getSessionStatus() { @@ -188,16 +188,16 @@ public class ModelAndViewContainer { /** * Add the supplied attribute to the underlying model. - * @see ModelMap#addAttribute(String, Object) + * A shortcut for {@code getModel().addAttribute(String, Object)}. */ public ModelAndViewContainer addAttribute(String name, Object value) { getModel().addAttribute(name, value); return this; } - + /** * Add the supplied attribute to the underlying model. - * @see Model#addAttribute(Object) + * A shortcut for {@code getModel().addAttribute(Object)}. */ public ModelAndViewContainer addAttribute(Object value) { getModel().addAttribute(value); @@ -206,7 +206,7 @@ public class ModelAndViewContainer { /** * Copy all attributes to the underlying model. - * @see ModelMap#addAllAttributes(Map) + * A shortcut for {@code getModel().addAllAttributes(Map)}. */ public ModelAndViewContainer addAllAttributes(Map attributes) { getModel().addAllAttributes(attributes); @@ -214,18 +214,30 @@ public class ModelAndViewContainer { } /** - * Copy attributes in the supplied Map with existing objects of + * Copy attributes in the supplied Map with existing objects of * the same name taking precedence (i.e. not getting replaced). - * @see ModelMap#mergeAttributes(Map) + * A shortcut for {@code getModel().mergeAttributes(Map)}. */ public ModelAndViewContainer mergeAttributes(Map attributes) { getModel().mergeAttributes(attributes); return this; } + /** + * Remove the given attributes from the model. + */ + public ModelAndViewContainer removeAttributes(Map attributes) { + if (attributes != null) { + for (String key : attributes.keySet()) { + getModel().remove(key); + } + } + return this; + } + /** * Whether the underlying model contains the given attribute name. - * @see ModelMap#containsAttribute(String) + * A shortcut for {@code getModel().containsAttribute(String)}. */ public boolean containsAttribute(String name) { return getModel().containsAttribute(name); @@ -257,5 +269,5 @@ public class ModelAndViewContainer { } return sb.toString(); } - + } diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java index 823d26ae937..8576f9351c8 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -27,6 +27,7 @@ import org.springframework.beans.TestBean; import org.springframework.core.MethodParameter; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.validation.BindException; +import org.springframework.validation.BindingResult; import org.springframework.validation.Errors; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.ModelAttribute; @@ -46,13 +47,13 @@ import static org.junit.Assert.*; /** * Test fixture with {@link ModelAttributeMethodProcessor}. - * + * * @author Rossen Stoyanchev */ public class ModelAttributeMethodProcessorTests { private ModelAttributeMethodProcessor processor; - + private MethodParameter paramNamedValidModelAttr; private MethodParameter paramErrors; @@ -62,31 +63,31 @@ public class ModelAttributeMethodProcessorTests { private MethodParameter paramModelAttr; private MethodParameter paramNonSimpleType; - + private MethodParameter returnParamNamedModelAttr; - + private MethodParameter returnParamNonSimpleType; private ModelAndViewContainer mavContainer; private NativeWebRequest webRequest; - + @Before public void setUp() throws Exception { processor = new ModelAttributeMethodProcessor(false); - Method method = ModelAttributeHandler.class.getDeclaredMethod("modelAttribute", + Method method = ModelAttributeHandler.class.getDeclaredMethod("modelAttribute", TestBean.class, Errors.class, int.class, TestBean.class, TestBean.class); - + paramNamedValidModelAttr = new MethodParameter(method, 0); paramErrors = new MethodParameter(method, 1); paramInt = new MethodParameter(method, 2); paramModelAttr = new MethodParameter(method, 3); paramNonSimpleType = new MethodParameter(method, 4); - + returnParamNamedModelAttr = new MethodParameter(getClass().getDeclaredMethod("annotatedReturnValue"), -1); returnParamNonSimpleType = new MethodParameter(getClass().getDeclaredMethod("notAnnotatedReturnValue"), -1); - + mavContainer = new ModelAndViewContainer(); webRequest = new ServletWebRequest(new MockHttpServletRequest()); @@ -97,16 +98,16 @@ public class ModelAttributeMethodProcessorTests { // Only @ModelAttribute arguments assertTrue(processor.supportsParameter(paramNamedValidModelAttr)); assertTrue(processor.supportsParameter(paramModelAttr)); - + assertFalse(processor.supportsParameter(paramErrors)); assertFalse(processor.supportsParameter(paramInt)); assertFalse(processor.supportsParameter(paramNonSimpleType)); } - + @Test public void supportedParametersInDefaultResolutionMode() throws Exception { processor = new ModelAttributeMethodProcessor(true); - + // Only non-simple types, even if not annotated assertTrue(processor.supportsParameter(paramNamedValidModelAttr)); assertTrue(processor.supportsParameter(paramErrors)); @@ -115,21 +116,21 @@ public class ModelAttributeMethodProcessorTests { assertFalse(processor.supportsParameter(paramInt)); } - + @Test public void supportedReturnTypes() throws Exception { processor = new ModelAttributeMethodProcessor(false); assertTrue(processor.supportsReturnType(returnParamNamedModelAttr)); assertFalse(processor.supportsReturnType(returnParamNonSimpleType)); } - + @Test public void supportedReturnTypesInDefaultResolutionMode() throws Exception { processor = new ModelAttributeMethodProcessor(true); assertTrue(processor.supportsReturnType(returnParamNamedModelAttr)); assertTrue(processor.supportsReturnType(returnParamNonSimpleType)); } - + @Test public void bindExceptionRequired() throws Exception { assertTrue(processor.isBindExceptionRequired(null, paramNonSimpleType)); @@ -141,13 +142,13 @@ public class ModelAttributeMethodProcessorTests { } @Test - public void getAttributeFromModel() throws Exception { - testGetAttributeFromModel("attrName", paramNamedValidModelAttr); - testGetAttributeFromModel("testBean", paramModelAttr); - testGetAttributeFromModel("testBean", paramNonSimpleType); + public void resovleArgumentFromModel() throws Exception { + getAttributeFromModel("attrName", paramNamedValidModelAttr); + getAttributeFromModel("testBean", paramModelAttr); + getAttributeFromModel("testBean", paramNonSimpleType); } - private void testGetAttributeFromModel(String expectedAttributeName, MethodParameter param) throws Exception { + private void getAttributeFromModel(String expectedAttributeName, MethodParameter param) throws Exception { Object target = new TestBean(); mavContainer.addAttribute(expectedAttributeName, target); @@ -155,56 +156,83 @@ public class ModelAttributeMethodProcessorTests { WebDataBinderFactory factory = createMock(WebDataBinderFactory.class); expect(factory.createBinder(webRequest, target, expectedAttributeName)).andReturn(dataBinder); replay(factory); - + processor.resolveArgument(param, mavContainer, webRequest, factory); - + verify(factory); } @Test - public void createAttribute() throws Exception { + public void resovleArgumentViaDefaultConstructor() throws Exception { WebDataBinder dataBinder = new WebRequestDataBinder(null); WebDataBinderFactory factory = createMock(WebDataBinderFactory.class); expect(factory.createBinder((NativeWebRequest) anyObject(), notNull(), eq("attrName"))).andReturn(dataBinder); replay(factory); - + processor.resolveArgument(paramNamedValidModelAttr, mavContainer, webRequest, factory); - + verify(factory); } @Test - public void automaticValidation() throws Exception { + public void resovleArgumentValidation() throws Exception { + String name = "attrName"; Object target = new TestBean(); - mavContainer.addAttribute("attrName", target); - - StubRequestDataBinder dataBinder = new StubRequestDataBinder(target); + mavContainer.addAttribute(name, target); + + StubRequestDataBinder dataBinder = new StubRequestDataBinder(target, name); WebDataBinderFactory binderFactory = createMock(WebDataBinderFactory.class); - expect(binderFactory.createBinder(webRequest, target, "attrName")).andReturn(dataBinder); + expect(binderFactory.createBinder(webRequest, target, name)).andReturn(dataBinder); replay(binderFactory); - + processor.resolveArgument(paramNamedValidModelAttr, mavContainer, webRequest, binderFactory); assertTrue(dataBinder.isBindInvoked()); assertTrue(dataBinder.isValidateInvoked()); } - + @Test(expected=BindException.class) - public void bindException() throws Exception { + public void resovleArgumentBindException() throws Exception { + String name = "testBean"; Object target = new TestBean(); mavContainer.getModel().addAttribute(target); - StubRequestDataBinder dataBinder = new StubRequestDataBinder(target); + StubRequestDataBinder dataBinder = new StubRequestDataBinder(target, name); dataBinder.getBindingResult().reject("error"); WebDataBinderFactory binderFactory = createMock(WebDataBinderFactory.class); - expect(binderFactory.createBinder(webRequest, target, "testBean")).andReturn(dataBinder); + expect(binderFactory.createBinder(webRequest, target, name)).andReturn(dataBinder); replay(binderFactory); - + processor.resolveArgument(paramNonSimpleType, mavContainer, webRequest, binderFactory); } - + + // SPR-9378 + + @Test + public void resolveArgumentOrdering() throws Exception { + String name = "testBean"; + Object testBean = new TestBean(name); + mavContainer.addAttribute(name, testBean); + mavContainer.addAttribute(BindingResult.MODEL_KEY_PREFIX + name, testBean); + + Object anotherTestBean = new TestBean(); + mavContainer.addAttribute("anotherTestBean", anotherTestBean); + + StubRequestDataBinder dataBinder = new StubRequestDataBinder(testBean, name); + WebDataBinderFactory binderFactory = createMock(WebDataBinderFactory.class); + expect(binderFactory.createBinder(webRequest, testBean, name)).andReturn(dataBinder); + replay(binderFactory); + + processor.resolveArgument(paramModelAttr, mavContainer, webRequest, binderFactory); + + assertSame("Resolved attribute should be updated to be last in the order", + testBean, mavContainer.getModel().values().toArray()[1]); + assertSame("BindingResult of resolved attribute should be last in the order", + dataBinder.getBindingResult(), mavContainer.getModel().values().toArray()[2]); + } + @Test public void handleAnnotatedReturnValue() throws Exception { processor.handleReturnValue("expected", returnParamNamedModelAttr, mavContainer, webRequest); @@ -215,18 +243,18 @@ public class ModelAttributeMethodProcessorTests { public void handleNotAnnotatedReturnValue() throws Exception { TestBean testBean = new TestBean("expected"); processor.handleReturnValue(testBean, returnParamNonSimpleType, mavContainer, webRequest); - + assertSame(testBean, mavContainer.getModel().get("testBean")); } - + private static class StubRequestDataBinder extends WebRequestDataBinder { - + private boolean bindInvoked; - + private boolean validateInvoked; - public StubRequestDataBinder(Object target) { - super(target); + public StubRequestDataBinder(Object target, String objectName) { + super(target, objectName); } public boolean isBindInvoked() { @@ -248,6 +276,8 @@ public class ModelAttributeMethodProcessorTests { public void validate(Object... validationHints) { validateInvoked = true; } + + } @Target({ METHOD, FIELD, CONSTRUCTOR, PARAMETER }) @@ -258,7 +288,7 @@ public class ModelAttributeMethodProcessorTests { @SessionAttributes(types=TestBean.class) private static class ModelAttributeHandler { @SuppressWarnings("unused") - public void modelAttribute(@ModelAttribute("attrName") @Valid TestBean annotatedAttr, + public void modelAttribute(@ModelAttribute("attrName") @Valid TestBean annotatedAttr, Errors errors, int intArg, @ModelAttribute TestBean defaultNameAttr, @@ -276,5 +306,5 @@ public class ModelAttributeMethodProcessorTests { private TestBean notAnnotatedReturnValue() { return null; } - + } \ No newline at end of file diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index a534caeb080..1c94b659184 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -14,6 +14,8 @@ Changes in version 3.2 M1 * add Servlet 3.0 based async support * fix issue with encoded params in UriComponentsBuilder * add Jackson 2 HttpMessageConverter and View types +* add pretty print option to Jackson HttpMessageConverter and View types +* fix issue with resolving Errors controller method argument Changes in version 3.1.1 (2012-02-16) -------------------------------------