diff --git a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java index 5ac23425f61..d305d7a4bf7 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java +++ b/org.springframework.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; @@ -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/org.springframework.web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java b/org.springframework.web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java index 94096df0062..b5fd76422de 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java +++ b/org.springframework.web/src/main/java/org/springframework/web/method/support/ModelAndViewContainer.java @@ -223,6 +223,18 @@ public class ModelAndViewContainer { 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) diff --git a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java index 823d26ae937..d1707ead669 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java @@ -16,17 +16,34 @@ package org.springframework.web.method.annotation; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.notNull; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + import java.lang.annotation.Retention; import java.lang.annotation.Target; import java.lang.reflect.Method; import org.junit.Before; import org.junit.Test; - 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; @@ -36,23 +53,17 @@ import org.springframework.web.bind.support.WebRequestDataBinder; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.request.WebRequest; -import org.springframework.web.method.annotation.ModelAttributeMethodProcessor; import org.springframework.web.method.support.ModelAndViewContainer; -import static java.lang.annotation.ElementType.*; -import static java.lang.annotation.RetentionPolicy.*; -import static org.easymock.EasyMock.*; -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 +73,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 +108,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 +126,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)); @@ -155,9 +166,9 @@ 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); } @@ -168,43 +179,69 @@ public class ModelAttributeMethodProcessorTests { 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 { + 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 { Object target = new TestBean(); mavContainer.getModel().addAttribute(target); - StubRequestDataBinder dataBinder = new StubRequestDataBinder(target); + StubRequestDataBinder dataBinder = new StubRequestDataBinder(target, "testBean"); dataBinder.getBindingResult().reject("error"); WebDataBinderFactory binderFactory = createMock(WebDataBinderFactory.class); expect(binderFactory.createBinder(webRequest, target, "testBean")).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 +252,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() { @@ -258,7 +295,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 +313,5 @@ public class ModelAttributeMethodProcessorTests { private TestBean notAnnotatedReturnValue() { return null; } - + } \ No newline at end of file