SPR-6801 @ModelAttribute instantiation refinement.

Instantiate the model attribute from a URI var or a request param only
if the name matches and there is a registered Converter<String, ?>.

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java

@@ -22,6 +22,10 @@ import java.util.Map;
 import javax.servlet.ServletRequest;
 
 import org.springframework.core.MethodParameter;
+import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.core.convert.converter.Converter;
+import org.springframework.util.StringUtils;
 import org.springframework.validation.DataBinder;
 import org.springframework.web.bind.ServletRequestDataBinder;
 import org.springframework.web.bind.WebDataBinder;
@@ -36,9 +40,9 @@ import org.springframework.web.servlet.mvc.method.annotation.ServletRequestDataB
  * A Servlet-specific {@link ModelAttributeMethodProcessor} that applies data
  * binding through a WebDataBinder of type {@link ServletRequestDataBinder}. 
  * 
- * <p>Adds a fall-back strategy to instantiate a model attribute from a 
+ * <p>Also adds a fall-back strategy to instantiate the model attribute from a
- * URI template variable combined with type conversion, if the model attribute 
+ * URI template variable or from a request parameter if the name matches the
- * name matches to a URI template variable name.
+ * model attribute name and there is an appropriate type conversion strategy.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -56,36 +60,89 @@ public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodPr
 	}
 
 	/**
-	 * Add a fall-back strategy to instantiate the model attribute from a URI 
+	 * Instantiate the model attribute from a URI template variable or from a
-	 * template variable with type conversion, if the model attribute name 
+	 * request parameter if the name matches to the model attribute name and
-	 * matches to a URI variable name.
+	 * if there is an appropriate type conversion strategy. If none of these
+	 * are true delegate back to the base class.
+	 * @see #createAttributeFromUriValue
 	 */
 	@Override
-	protected Object createAttribute(String attributeName, 
+	protected final Object createAttribute(String attributeName, 
-									 MethodParameter parameter, 
+										   MethodParameter parameter, 
-									 WebDataBinderFactory binderFactory, 
+										   WebDataBinderFactory binderFactory, 
-									 NativeWebRequest request) throws Exception {
+										   NativeWebRequest request) throws Exception {
-		
+
-		Map<String, String> uriVariables = getUriTemplateVariables(request);
+		String value = getRequestValueForAttribute(attributeName, request);
-		
+		if (value != null) {
-		if (uriVariables.containsKey(attributeName)) {
+			Object attribute = createAttributeFromRequestValue(value, attributeName, parameter, binderFactory, request);
-			DataBinder binder = binderFactory.createBinder(request, null, attributeName);
+			if (attribute != null) {
-			return binder.convertIfNecessary(uriVariables.get(attributeName), parameter.getParameterType());
+				return attribute;
+			}
 		}
-		
+
 		return super.createAttribute(attributeName, parameter, binderFactory, request);
 	}
+	
+	/**
+	 * Obtain a value from the request that may be used to instantiate the
+	 * model attribute through type conversion from String to the target type. 
+	 * <p>The default implementation looks for the attribute name to match
+	 * a URI variable first and then a request parameter.
+	 * @param attributeName the model attribute name
+	 * @param request the current request
+	 * @return the request value to try to convert or {@code null}
+	 */
+	protected String getRequestValueForAttribute(String attributeName, NativeWebRequest request) {
+		Map<String, String> variables = getUriTemplateVariables(request);
+		if (StringUtils.hasText(variables.get(attributeName))) {
+			return variables.get(attributeName);
+		}
+		else if (StringUtils.hasText(request.getParameter(attributeName))) {
+			return request.getParameter(attributeName);
+		}
+		else {
+			return null;
+		}
+	}
 
 	@SuppressWarnings("unchecked")
-	private Map<String, String> getUriTemplateVariables(NativeWebRequest request) {
+	protected final Map<String, String> getUriTemplateVariables(NativeWebRequest request) {
-		
+		Map<String, String> variables = 
-		Map<String, String> uriTemplateVars = 
 			(Map<String, String>) request.getAttribute(
 					HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST);
-		
+		return (variables != null) ? variables : Collections.<String, String>emptyMap();
-		return (uriTemplateVars != null) ? uriTemplateVars : Collections.<String, String>emptyMap();
 	}
 
+	/**
+	 * Create a model attribute from a String request value (e.g. URI template
+	 * variable, request parameter) using type conversion.
+	 * <p>The default implementation converts only if there a registered 
+	 * {@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 parameter the method parameter
+	 * @param binderFactory for creating WebDataBinder instance
+	 * @param request the current request
+	 * @return the created model attribute, or {@code null}
+	 * @throws Exception
+	 */
+	protected Object createAttributeFromRequestValue(String sourceValue,
+												 String attributeName, 
+												 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(parameter);
+			if (conversionService.canConvert(source, target)) {
+				return binder.convertIfNecessary(sourceValue, parameter.getParameterType(), parameter);
+			}
+		}
+		return null;
+	}
+	
 	/**
 	 * {@inheritDoc}
 	 * <p>Downcast {@link WebDataBinder} to {@link ServletRequestDataBinder} before binding.

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SerlvetModelAttributeMethodProcessorTests.java

@@ -27,8 +27,10 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.TestBean;
 import org.springframework.core.MethodParameter;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.web.bind.annotation.ModelAttribute;
+import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.bind.support.WebDataBinderFactory;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
@@ -60,44 +62,72 @@ public class SerlvetModelAttributeMethodProcessorTests {
 
 	@Before
 	public void setUp() throws Exception {
-		processor = new ServletModelAttributeMethodProcessor(false);
+		this.processor = new ServletModelAttributeMethodProcessor(false);
 
 		Method method = getClass().getDeclaredMethod("modelAttribute", 
 				TestBean.class, TestBeanWithoutStringConstructor.class);
 		
-		testBeanModelAttr = new MethodParameter(method, 0);
+		this.testBeanModelAttr = new MethodParameter(method, 0);
-		testBeanWithoutStringConstructorModelAttr = new MethodParameter(method, 1);
+		this.testBeanWithoutStringConstructorModelAttr = new MethodParameter(method, 1);
 		
-		binderFactory = new ServletRequestDataBinderFactory(null, null);
+		ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
-		mavContainer = new ModelAndViewContainer();
+		initializer.setConversionService(new DefaultConversionService());
 		
-		request = new MockHttpServletRequest();
+		this.binderFactory = new ServletRequestDataBinderFactory(null, initializer );
-		webRequest = new ServletWebRequest(request);
+		this.mavContainer = new ModelAndViewContainer();
+		
+		this.request = new MockHttpServletRequest();
+		this.webRequest = new ServletWebRequest(request);
 	}
 
 	@Test
-	public void createAttributeViaPathVariable() throws Exception {
+	public void createAttributeUriTemplateVar() throws Exception {
 		Map<String, String> uriTemplateVars = new HashMap<String, String>();
-		uriTemplateVars.put("testBean1", "pathy");
+		uriTemplateVars.put("testBean1", "Patty");
-		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars);
+		this.request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars);
 
-		// Type conversion from "pathy" to TestBean via TestBean(String) constructor
+		// Type conversion from "Patty" to TestBean via TestBean(String) constructor
 
 		TestBean testBean = 
-			(TestBean) processor.resolveArgument(testBeanModelAttr, mavContainer, webRequest, binderFactory);
+			(TestBean) this.processor.resolveArgument(
+					this.testBeanModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
 		
-		assertEquals("pathy", testBean.getName());
+		assertEquals("Patty", testBean.getName());
 	}
 
 	@Test
-	public void createAttributeAfterPathVariableConversionError() throws Exception {
+	public void createAttributeUriTemplateVarCannotConvert() throws Exception {
 		Map<String, String> uriTemplateVars = new HashMap<String, String>();
-		uriTemplateVars.put("testBean1", "pathy");
+		uriTemplateVars.put("testBean2", "Patty");
 		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars);
 
 		TestBeanWithoutStringConstructor testBean = 
-			(TestBeanWithoutStringConstructor) processor.resolveArgument(
+			(TestBeanWithoutStringConstructor) this.processor.resolveArgument(
-				testBeanWithoutStringConstructorModelAttr, mavContainer, webRequest, binderFactory);
+					this.testBeanWithoutStringConstructorModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+		
+		assertNotNull(testBean);
+	}
+
+	@Test
+	public void createAttributeRequestParameter() throws Exception {
+		this.request.addParameter("testBean1", "Patty");
+
+		// Type conversion from "Patty" to TestBean via TestBean(String) constructor
+
+		TestBean testBean = 
+			(TestBean) this.processor.resolveArgument(
+					this.testBeanModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+		
+		assertEquals("Patty", testBean.getName());
+	}
+
+	@Test
+	public void createAttributeRequestParameterCannotConvert() throws Exception {
+		this.request.addParameter("testBean1", "Patty");
+
+		TestBeanWithoutStringConstructor testBean = 
+			(TestBeanWithoutStringConstructor) this.processor.resolveArgument(
+					this.testBeanWithoutStringConstructorModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
 		
 		assertNotNull(testBean);
 	}

org.springframework.web/src/test/java/org/springframework/web/method/annotation/InitBinderDataBinderFactoryTests.java

@@ -99,6 +99,14 @@ public class InitBinderDataBinderFactoryTests {
 		assertNull(dataBinder.getDisallowedFields());
 	}
 	
+	@Test
+	public void createBinderNullAttrName() throws Exception {
+		WebDataBinderFactory factory = createBinderFactory("initBinderWithAttributeName", WebDataBinder.class);
+		WebDataBinder dataBinder = factory.createBinder(webRequest, null, null);
+		
+		assertNull(dataBinder.getDisallowedFields());
+	}
+	
 	@Test(expected=IllegalStateException.class)
 	public void returnValueNotExpected() throws Exception {
 		WebDataBinderFactory factory = createBinderFactory("initBinderReturnValue", WebDataBinder.class);

spring-framework-reference/src/mvc.xml

@@ -1607,9 +1607,8 @@ public String save(@ModelAttribute("account") Account account) {
 
 		<para>In this example the name of the model attribute (i.e. "account") 
 		matches the name of a URI template variable. If you register 
-		<classname>Converter&lt;String, Account&gt;</classname>
+		<classname>Converter&lt;String, Account&gt;</classname> that can turn the 
-		(or <classname>PropertyEditor</classname>) that can turn the 
+		<literal>String</literal> account value into an <classname>Account</classname>
-		<literal>String</literal>-based account into an <classname>Account</classname>
 		instance, then the above example will work without the need for an
 		<interfacename>@ModelAttribute</interfacename> method.</para>