Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix issue with resolving Errors controller argument

Browse Source 
The ErrorsMethodArgumentResolver expects the preceding @ModelAttribute
in the controller method signature to be the last one added in the
model -- an assumption that can break if a model attribute is added
earlier (e.g. through a @ModelAttribute method) and more attributes
are added as well. This fix ensures when an @ModelAttribute is resolved
as a controller method argument it has the highest index in the model.

Issue: SPR-9378
This commit is contained in:
Rossen Stoyanchev
2012-05-10 16:01:14 -04:00
parent e04b322110
commit c499df2315
4 changed files with 140 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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<String, Object> bindingResultModel = binder.getBindingResult().getModel();
mavContainer.removeAttributes(bindingResultModel);
mavContainer.addAllAttributes(bindingResultModel);
return binder.getTarget();
}

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

@@ -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.
*
*
* <p>The {@link #setRequestHandled} flag can be used to indicate the request
* has been handled directly and view resolution is not required.
*
* <p>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()}
*
* <p>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.
* <p>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.
* <p>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}).
* <p>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.
* <p>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.
* <p>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.
* <p>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<String, ?> attributes) {
getModel().addAllAttributes(attributes);
@@ -214,18 +214,30 @@ public class ModelAndViewContainer {
}
/**
* Copy attributes in the supplied <code>Map</code> with existing objects of
* Copy attributes in the supplied <code>Map</code> 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<String, ?>)}.
*/
public ModelAndViewContainer mergeAttributes(Map<String, ?> attributes) {
getModel().mergeAttributes(attributes);
return this;
}
/**
* Remove the given attributes from the model.
*/
public ModelAndViewContainer removeAttributes(Map<String, ?> 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();
}
}