From 1e07af8827c77aab3f5c215cdcdc89afff918658 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 24 Jun 2011 17:18:53 +0000 Subject: [PATCH] SPR-7608 Add fallback mechanism for instantiating a model attribute from a path variable --- .../PathVariableMethodArgumentResolver.java | 6 +- .../ServletModelAttributeMethodProcessor.java | 55 +++++++- ...vetModelAttributeMethodProcessorTests.java | 123 ++++++++++++++++++ .../ModelAttributeMethodProcessor.java | 75 ++++++----- .../ModelAttributeMethodProcessorTests.java | 82 +++++++----- 5 files changed, 268 insertions(+), 73 deletions(-) create mode 100644 org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SerlvetModelAttributeMethodProcessorTests.java diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java index c3ddfab1c4f..82c1208a8cd 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java @@ -64,9 +64,9 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod @Override @SuppressWarnings("unchecked") protected Object resolveName(String name, MethodParameter parameter, NativeWebRequest request) throws Exception { - String key = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE; - int scope = RequestAttributes.SCOPE_REQUEST; - Map uriTemplateVars = (Map) request.getAttribute(key, scope); + Map uriTemplateVars = + (Map) request.getAttribute( + HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST); return (uriTemplateVars != null) ? uriTemplateVars.get(name) : null; } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java index 7d5c900ff67..0646a7ef7fd 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java @@ -16,17 +16,31 @@ package org.springframework.web.servlet.mvc.method.annotation.support; +import java.beans.PropertyEditor; +import java.util.Map; + import javax.servlet.ServletRequest; import org.springframework.beans.BeanUtils; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.converter.Converter; +import org.springframework.validation.DataBinder; import org.springframework.web.bind.ServletRequestDataBinder; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.method.annotation.support.ModelAttributeMethodProcessor; +import org.springframework.web.servlet.HandlerMapping; /** - * A Servlet-specific {@link ModelAttributeMethodProcessor} variant that casts the {@link WebDataBinder} + * A Servlet-specific {@link ModelAttributeMethodProcessor} variant with the following further benefits: + *
    + *
  • Casts the data binder down to {@link ServletRequestDataBinder} prior to invoking bind on it + *
  • Attempts to instantiate the model attribute using a path variable and type conversion + *
+ * that casts * instance to {@link ServletRequestDataBinder} prior to invoking data binding. * * @author Rossen Stoyanchev @@ -44,12 +58,43 @@ public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodPr } /** - * {@inheritDoc} - *

This method downcasts the binder instance to {@link ServletRequestDataBinder} and invokes - * its bind method passing a {@link ServletRequest} to it. + * Instantiates the model attribute by trying to match the model attribute name to a path variable. + * If a match is found an attempt is made to convert the String path variable to the expected + * method parameter type through a registered {@link Converter} or {@link PropertyEditor}. + * If this fails the call is delegated back to the parent for default constructor instantiation. */ @Override - protected void doBind(WebDataBinder binder, NativeWebRequest request) { + @SuppressWarnings("unchecked") + protected Object createAttribute(String attributeName, + MethodParameter parameter, + WebDataBinderFactory binderFactory, + NativeWebRequest request) throws Exception { + Map uriTemplateVars = + (Map) request.getAttribute( + HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST); + + if (uriTemplateVars != null && uriTemplateVars.containsKey(attributeName)) { + try { + String var = uriTemplateVars.get(attributeName); + DataBinder binder = binderFactory.createBinder(request, null, attributeName); + return binder.convertIfNecessary(var, parameter.getParameterType()); + + } catch (Exception exception) { + logger.info("Model attribute '" + attributeName + "' matches to a URI template variable name. " + + "The URI template variable however couldn't converted to a model attribute instance: " + + exception.getMessage()); + } + } + + return super.createAttribute(attributeName, parameter, binderFactory, request); + } + + /** + * {@inheritDoc} + *

This implementation downcasts to {@link ServletRequestDataBinder} before invoking the bind operation. + */ + @Override + protected void bindRequestParameters(WebDataBinder binder, NativeWebRequest request) { ServletRequest servletRequest = request.getNativeRequest(ServletRequest.class); ServletRequestDataBinder servletBinder = (ServletRequestDataBinder) binder; servletBinder.bind(servletRequest); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SerlvetModelAttributeMethodProcessorTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SerlvetModelAttributeMethodProcessorTests.java new file mode 100644 index 00000000000..abc85ab3d99 --- /dev/null +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SerlvetModelAttributeMethodProcessorTests.java @@ -0,0 +1,123 @@ +/* + * Copyright 2002-2011 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.mvc.method.annotation; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; + +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.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.support.WebDataBinderFactory; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.method.support.ModelAndViewContainer; +import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.mvc.method.annotation.support.ServletModelAttributeMethodProcessor; + +/** + * Test fixture for {@link ServletModelAttributeMethodProcessor} specific tests. + * Also see org.springframework.web.method.annotation.support.ModelAttributeMethodProcessorTests + * + * @author Rossen Stoyanchev + */ +public class SerlvetModelAttributeMethodProcessorTests { + + private ServletModelAttributeMethodProcessor processor; + + private MethodParameter testBeanModelAttr; + + private MethodParameter testBeanWithoutStringConstructorModelAttr; + + private ModelAndViewContainer mavContainer; + + private NativeWebRequest webRequest; + + private MockHttpServletRequest request; + + private WebDataBinderFactory binderFactory; + + @Before + public void setUp() throws Exception { + processor = new ServletModelAttributeMethodProcessor(false); + + Method method = getClass().getDeclaredMethod("modelAttribute", + TestBean.class, TestBeanWithoutStringConstructor.class); + + testBeanModelAttr = new MethodParameter(method, 0); + testBeanWithoutStringConstructorModelAttr = new MethodParameter(method, 1); + + binderFactory = new ServletRequestDataBinderFactory(null, null); + mavContainer = new ModelAndViewContainer(); + + request = new MockHttpServletRequest(); + webRequest = new ServletWebRequest(request); + } + + @Test + public void createAttributeViaPathVariable() throws Exception { + Map uriTemplateVars = new HashMap(); + uriTemplateVars.put("testBean1", "pathy"); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + // Type conversion from "pathy" to TestBean via TestBean(String) constructor + + TestBean testBean = + (TestBean) processor.resolveArgument(testBeanModelAttr, mavContainer, webRequest, binderFactory); + + assertEquals("pathy", testBean.getName()); + } + + @Test + public void createAttributeAfterPathVariableConversionError() throws Exception { + Map uriTemplateVars = new HashMap(); + uriTemplateVars.put("testBean1", "pathy"); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + TestBeanWithoutStringConstructor testBean = + (TestBeanWithoutStringConstructor) processor.resolveArgument( + testBeanWithoutStringConstructorModelAttr, mavContainer, webRequest, binderFactory); + + assertNotNull(testBean); + } + + + @SuppressWarnings("unused") + private void modelAttribute(@ModelAttribute("testBean1") TestBean testBean1, + @ModelAttribute("testBean2") TestBeanWithoutStringConstructor testBean2) { + } + + + @SuppressWarnings("unused") + private static class TestBeanWithoutStringConstructor { + + public TestBeanWithoutStringConstructor() { + } + + public TestBeanWithoutStringConstructor(int i) { + } + + } + +} diff --git a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java index 9633857675b..56b5a36441a 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java +++ b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java @@ -18,11 +18,12 @@ package org.springframework.web.method.annotation.support; import java.lang.annotation.Annotation; +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.validation.BindException; import org.springframework.validation.BindingResult; -import org.springframework.validation.DataBinder; import org.springframework.validation.Errors; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.ModelAttribute; @@ -50,6 +51,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; */ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResolver, HandlerMethodReturnValueHandler { + protected Log logger = LogFactory.getLog(this.getClass()); + private final boolean useDefaultResolution; /** @@ -82,24 +85,31 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * default constructor. Data binding and optionally validation is then applied through a {@link WebDataBinder} * instance. Validation is invoked optionally when the method parameter is annotated with an {@code @Valid}. * - * @throws Exception if a {@link WebDataBinder} could not be created or if data binding and validation result in - * an error and the next method parameter is not of type {@link Errors} or {@link BindingResult}. + * @throws BindException if data binding and validation result in an error and the next method parameter + * is neither of type {@link Errors} nor {@link BindingResult}. + * @throws Exception if a {@link WebDataBinder} could not be created. */ public final Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, - NativeWebRequest webRequest, + NativeWebRequest request, WebDataBinderFactory binderFactory) throws Exception { - WebDataBinder binder = createDataBinder(parameter, mavContainer, webRequest, binderFactory); + String name = ModelFactory.getNameForParameter(parameter); + Object target = (mavContainer.containsAttribute(name)) ? + mavContainer.getAttribute(name) : createAttribute(name, parameter, binderFactory, request); + + WebDataBinder binder = binderFactory.createBinder(request, target, name); if (binder.getTarget() != null) { - doBind(binder, webRequest); - - if (shouldValidate(binder, parameter)) { + bindRequestParameters(binder, request); + + if (isValidationApplicable(binder, parameter)) { binder.validate(); } - - if (failOnError(binder, parameter) && binder.getBindingResult().hasErrors()) { - throw new BindException(binder.getBindingResult()); + + if (binder.getBindingResult().hasErrors()) { + if (isBindExceptionRequired(binder, parameter)) { + throw new BindException(binder.getBindingResult()); + } } } @@ -109,23 +119,22 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } /** - * Creates a {@link WebDataBinder} for a target object. + * Creates an instance of the specified model attribute. This method is invoked only if the attribute is + * not available in the model. This default implementation uses the no-argument constructor. + * Subclasses can override to provide additional means of creating the model attribute. + * + * @param attributeName the name of the model attribute + * @param parameter the method argument declaring the model attribute + * @param binderFactory a factory for creating {@link WebDataBinder} instances + * @param request the current request + * @return the created model attribute; never {@code null} + * @throws Exception raised in the process of creating the instance */ - private WebDataBinder createDataBinder(MethodParameter parameter, - ModelAndViewContainer mavContainer, - NativeWebRequest webRequest, - WebDataBinderFactory binderFactory) throws Exception { - String attrName = ModelFactory.getNameForParameter(parameter); - - Object target; - if (mavContainer.containsAttribute(attrName)) { - target = mavContainer.getAttribute(attrName); - } - else { - target = BeanUtils.instantiateClass(parameter.getParameterType()); - } - - return binderFactory.createBinder(webRequest, target, attrName); + protected Object createAttribute(String attributeName, + MethodParameter parameter, + WebDataBinderFactory binderFactory, + NativeWebRequest request) throws Exception { + return BeanUtils.instantiateClass(parameter.getParameterType()); } /** @@ -134,17 +143,17 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * @param binder the binder with the target object to apply request values to * @param request the current request */ - protected void doBind(WebDataBinder binder, NativeWebRequest request) { + protected void bindRequestParameters(WebDataBinder binder, NativeWebRequest request) { ((WebRequestDataBinder) binder).bind(request); } /** - * Whether to validate the target object of the given {@link WebDataBinder} instance. + * Whether to validate the model attribute inside the given data binder instance. * @param binder the data binder containing the validation candidate - * @param parameter the method argument for which data binding is performed - * @return true if {@link DataBinder#validate()} should be invoked, false otherwise. + * @param parameter the method argument declaring the validation candidate + * @return {@code true} if validation should be applied, {@code false} otherwise. */ - protected boolean shouldValidate(WebDataBinder binder, MethodParameter parameter) { + protected boolean isValidationApplicable(WebDataBinder binder, MethodParameter parameter) { Annotation[] annotations = parameter.getParameterAnnotations(); for (Annotation annot : annotations) { if ("Valid".equals(annot.annotationType().getSimpleName())) { @@ -160,7 +169,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * @param parameter the method argument for which data binding is performed * @return true if the binding or validation errors should result in a {@link BindException}, false otherwise. */ - protected boolean failOnError(WebDataBinder binder, MethodParameter parameter) { + 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])); diff --git a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessorTests.java b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessorTests.java index 7d7b34b71c1..371040f7eea 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessorTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessorTests.java @@ -83,7 +83,7 @@ public class ModelAttributeMethodProcessorTests { @Before public void setUp() throws Exception { - processor = new ModelAttributeMethodProcessor(true); + processor = new ModelAttributeMethodProcessor(false); Method method = ModelAttributeHandler.class.getDeclaredMethod("modelAttribute", TestBean.class, Errors.class, int.class, TestBean.class, TestBean.class); @@ -103,68 +103,86 @@ public class ModelAttributeMethodProcessorTests { } @Test - public void supportParameter() throws Exception { - processor = new ModelAttributeMethodProcessor(true); + public void supportedParameters() throws Exception { + // Only @ModelAttribute arguments assertTrue(processor.supportsParameter(paramNamedValidModelAttr)); - assertTrue(processor.supportsParameter(paramErrors)); - assertFalse(processor.supportsParameter(paramInt)); assertTrue(processor.supportsParameter(paramModelAttr)); - assertTrue(processor.supportsParameter(paramNonSimpleType)); - processor = new ModelAttributeMethodProcessor(false); - assertTrue(processor.supportsParameter(paramNamedValidModelAttr)); assertFalse(processor.supportsParameter(paramErrors)); assertFalse(processor.supportsParameter(paramInt)); - assertTrue(processor.supportsParameter(paramModelAttr)); assertFalse(processor.supportsParameter(paramNonSimpleType)); } @Test - public void supportsReturnType() throws Exception { + public void supportedParametersInDefaultResolutionMode() throws Exception { processor = new ModelAttributeMethodProcessor(true); - assertTrue(processor.supportsReturnType(returnParamNamedModelAttr)); - assertFalse(processor.supportsReturnType(returnParamNonSimpleType)); + + // Only non-simple types, even if not annotated + assertTrue(processor.supportsParameter(paramNamedValidModelAttr)); + assertTrue(processor.supportsParameter(paramErrors)); + assertTrue(processor.supportsParameter(paramModelAttr)); + assertTrue(processor.supportsParameter(paramNonSimpleType)); + 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 shouldValidate() throws Exception { - assertTrue(processor.shouldValidate(null, paramNamedValidModelAttr)); - assertFalse(processor.shouldValidate(null, paramNonSimpleType)); + public void supportedReturnTypesInDefaultResolutionMode() throws Exception { + processor = new ModelAttributeMethodProcessor(true); + assertTrue(processor.supportsReturnType(returnParamNamedModelAttr)); + assertFalse(processor.supportsReturnType(returnParamNonSimpleType)); + } + + @Test + public void validationApplicable() throws Exception { + assertTrue(processor.isValidationApplicable(null, paramNamedValidModelAttr)); + } + + @Test + public void validationNotApplicable() throws Exception { + assertFalse(processor.isValidationApplicable(null, paramNonSimpleType)); } @Test - public void failOnError() throws Exception { - assertFalse("Shouldn't failOnError with BindingResult", processor.failOnError(null, paramNamedValidModelAttr)); - assertTrue("Should failOnError without BindingResult", processor.failOnError(null, paramNonSimpleType)); + public void bindExceptionRequired() throws Exception { + assertTrue(processor.isBindExceptionRequired(null, paramNonSimpleType)); } @Test - public void createBinderFromModelAttribute() throws Exception { - createBinderFromModelAttr("attrName", paramNamedValidModelAttr); - createBinderFromModelAttr("testBean", paramModelAttr); - createBinderFromModelAttr("testBean", paramNonSimpleType); + public void bindExceptionNotRequired() throws Exception { + assertFalse(processor.isBindExceptionRequired(null, paramNamedValidModelAttr)); } - private void createBinderFromModelAttr(String expectedAttrName, MethodParameter param) throws Exception { + @Test + public void getAttributeFromModel() throws Exception { + testGetAttributeFromModel("attrName", paramNamedValidModelAttr); + testGetAttributeFromModel("testBean", paramModelAttr); + testGetAttributeFromModel("testBean", paramNonSimpleType); + } + + private void testGetAttributeFromModel(String expectedAttributeName, MethodParameter param) throws Exception { Object target = new TestBean(); - mavContainer.addAttribute(expectedAttrName, target); + mavContainer.addAttribute(expectedAttributeName, target); WebDataBinder dataBinder = new WebRequestDataBinder(target); - WebDataBinderFactory binderFactory = createMock(WebDataBinderFactory.class); - expect(binderFactory.createBinder(webRequest, target, expectedAttrName)).andReturn(dataBinder); - replay(binderFactory); + WebDataBinderFactory factory = createMock(WebDataBinderFactory.class); + expect(factory.createBinder(webRequest, target, expectedAttributeName)).andReturn(dataBinder); + replay(factory); - processor.resolveArgument(param, mavContainer, webRequest, binderFactory); + processor.resolveArgument(param, mavContainer, webRequest, factory); - verify(binderFactory); + verify(factory); } @Test - public void createBinderWithAttributeConstructor() throws Exception { + public void createAttribute() throws Exception { WebDataBinder dataBinder = new WebRequestDataBinder(null); WebDataBinderFactory factory = createMock(WebDataBinderFactory.class); @@ -177,7 +195,7 @@ public class ModelAttributeMethodProcessorTests { } @Test - public void bindAndValidate() throws Exception { + public void automaticValidation() throws Exception { Object target = new TestBean(); mavContainer.addAttribute("attrName", target); @@ -193,7 +211,7 @@ public class ModelAttributeMethodProcessorTests { } @Test(expected=BindException.class) - public void bindAndFail() throws Exception { + public void bindException() throws Exception { Object target = new TestBean(); mavContainer.getModel().addAttribute(target);