SPR-6909 Improve extension hooks in DefaultDataBinderFactory and subclasses.

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4620 50f2f4bb-b051-0410-bef5-90022cba6387

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

@@ -22,18 +22,17 @@ import java.util.Map;
 import org.springframework.beans.MutablePropertyValues;
 import org.springframework.web.bind.ServletRequestDataBinder;
 import org.springframework.web.bind.WebDataBinder;
+import org.springframework.web.bind.annotation.InitBinder;
 import org.springframework.web.bind.support.WebBindingInitializer;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.RequestAttributes;
-import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.method.annotation.InitBinderDataBinderFactory;
 import org.springframework.web.method.support.InvocableHandlerMethod;
 import org.springframework.web.servlet.HandlerMapping;
 
 /**
- * An {@link InitBinderDataBinderFactory} variation instantiating a data binder of type
- * {@link ServletRequestDataBinder} and further extending it with the ability to add URI template variables
- * to the values used in data binding.
+ * Creates a {@link ServletRequestDataBinder} instance and extends it with the ability to include 
+ * URI template variables in the values used for data binding purposes.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -41,62 +40,44 @@ import org.springframework.web.servlet.HandlerMapping;
 public class ServletRequestDataBinderFactory extends InitBinderDataBinderFactory {
 
 	/**
-	 * Create an {@link ServletRequestDataBinderFactory} instance.
-	 * @param initBinderMethods init binder methods to use to initialize new data binders.
-	 * @param bindingInitializer a WebBindingInitializer to use to initialize created data binder instances.
+	 * Create a new instance.
+	 * @param binderMethods {@link InitBinder} methods to initialize new data binder instances with
+	 * @param iitializer a global initializer to initialize new data binder instances with
 	 */
-	public ServletRequestDataBinderFactory(List<InvocableHandlerMethod> initBinderMethods,
-										   WebBindingInitializer bindingInitializer) {
-		super(initBinderMethods, bindingInitializer);
-	}
-
-	/**
-	 * Returns the more specific {@link ServletRequestDataBinder} created by {@link #createBinderInstance(Object, String)}.
-	 */
-	@Override
-	public ServletRequestDataBinder createBinder(NativeWebRequest request, Object target, String objectName)
-			throws Exception {
-		return (ServletRequestDataBinder) super.createBinder(request, target, objectName);
+	public ServletRequestDataBinderFactory(List<InvocableHandlerMethod> binderMethods, WebBindingInitializer iitializer) {
+		super(binderMethods, iitializer);
 	}
 	
 	/**
-	 * {@inheritDoc}
-	 * <p>This method creates a {@link ServletRequestDataBinder} instance that also adds URI template variables to
-	 * the values used in data binding.
-	 * <p>Subclasses wishing to override this method to provide their own ServletRequestDataBinder type can use the
-	 * {@link #addUriTemplateVariables(MutablePropertyValues)} method to include URI template variables as follows:
-	 * <pre>
-	 * return new CustomServletRequestDataBinder(target, objectName) {
-	 *    protected void doBind(MutablePropertyValues mpvs) {
-	 *        addUriTemplateVariables(mpvs);
-	 *        super.doBind(mpvs);
-	 *    }
-	 * };
-	 * </pre>
+	 * Creates a {@link ServletRequestDataBinder} instance extended with the ability to add 
+	 * URI template variables the values used for data binding.
 	 */
 	@Override
-	protected WebDataBinder createBinderInstance(Object target, String objectName) {
+	protected WebDataBinder createBinderInstance(Object target, String objectName, final NativeWebRequest request) {
 		return new ServletRequestDataBinder(target, objectName) {
 
 			protected void doBind(MutablePropertyValues mpvs) {
-				addUriTemplateVariables(mpvs);
+				addUriTemplateVars(mpvs, request);
 				super.doBind(mpvs);
 			}
 		};
 	}
 
 	/**
-	 * Adds URI template variables to the given property values.
-	 * @param mpvs the PropertyValues to add URI template variables to
+	 * Adds URI template variables to the the property values used for data binding.
+	 * @param mpvs the PropertyValues to use for data binding
 	 */
 	@SuppressWarnings("unchecked")
-	protected void addUriTemplateVariables(MutablePropertyValues mpvs) {
-		RequestAttributes requestAttrs = RequestContextHolder.getRequestAttributes();
-		if (requestAttrs != null) {
-			String key = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
-			int scope = RequestAttributes.SCOPE_REQUEST;
-			Map<String, String> uriTemplateVars = (Map<String, String>) requestAttrs.getAttribute(key, scope);
-			mpvs.addPropertyValues(uriTemplateVars);
+	protected final void addUriTemplateVars(MutablePropertyValues mpvs, NativeWebRequest request) {
+		Map<String, String> uriTemplateVars = 
+			(Map<String, String>) request.getAttribute(
+					HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST);
+		if (uriTemplateVars != null){
+			for (String name : uriTemplateVars.keySet()) {
+				if (!mpvs.contains(name)) {
+					mpvs.addPropertyValue(name, uriTemplateVars.get(name));
+				}
+			}
 		}
 	}
 

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

@@ -73,5 +73,32 @@ public class ServletRequestDataBinderFactoryTests {
 		assertEquals("nameValue", target.getName());
 		assertEquals(25, target.getAge());
 	}
-	
+
+	@Test
+	public void requestParamsOverrideUriTemplateVars() throws Exception {
+		request.addParameter("age", "35");
+		
+		Map<String, String> uriTemplateVars = new HashMap<String, String>();
+		uriTemplateVars.put("name", "nameValue");
+		uriTemplateVars.put("age", "25");
+		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars);
+		
+		TestBean target = new TestBean();
+		WebDataBinder binder = binderFactory.createBinder(webRequest, target, "");
+		((ServletRequestDataBinder) binder).bind(request);
+
+		assertEquals("nameValue", target.getName());
+		assertEquals(35, target.getAge());
+	}
+
+	@Test
+	public void noUriTemplateVars() throws Exception {
+		TestBean target = new TestBean();
+		WebDataBinder binder = binderFactory.createBinder(webRequest, target, "");
+		((ServletRequestDataBinder) binder).bind(request);
+
+		assertEquals(null, target.getName());
+		assertEquals(0, target.getAge());
+	}
+
 }

org.springframework.web/src/main/java/org/springframework/web/bind/support/DefaultDataBinderFactory.java

@@ -20,45 +20,58 @@ import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.context.request.NativeWebRequest;
 
 /**
- * A {@link WebDataBinderFactory} that creates {@link WebDataBinder} and initializes them 
- * with a {@link WebBindingInitializer}.
+ * Creates a {@link WebRequestDataBinder} and initializes it through a {@link WebBindingInitializer}.
  * 
  * @author Rossen Stoyanchev
  * @since 3.1
  */
 public class DefaultDataBinderFactory implements WebDataBinderFactory {
 
-	private final WebBindingInitializer bindingInitializer;
+	private final WebBindingInitializer initializer;
 
 	/**
 	 * Create {@link DefaultDataBinderFactory} instance.
-	 * @param bindingInitializer a {@link WebBindingInitializer} to initialize new data binder instances with
+	 * @param initializer a global initializer to initialize new data binder instances with
 	 */
-	public DefaultDataBinderFactory(WebBindingInitializer bindingInitializer) {
-		this.bindingInitializer = bindingInitializer;
+	public DefaultDataBinderFactory(WebBindingInitializer initializer) {
+		this.initializer = initializer;
 	}
 
 	/**
 	 * Create a new {@link WebDataBinder} for the given target object and initialize it through 
 	 * a {@link WebBindingInitializer}. 
 	 */
-	public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception {
-		WebDataBinder dataBinder = createBinderInstance(target, objectName);
+	public final WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception {
+		WebDataBinder dataBinder = createBinderInstance(target, objectName, webRequest);
 
-		if (bindingInitializer != null) {
-			this.bindingInitializer.initBinder(dataBinder, webRequest);
+		if (initializer != null) {
+			this.initializer.initBinder(dataBinder, webRequest);
 		}
 
+		initBinder(dataBinder, webRequest);
+		
 		return dataBinder;
 	}
 
 	/**
-	 * Create a {@link WebDataBinder} instance.
-	 * @param target the object to create a data binder for, or {@code null} if creating a binder for a simple type
+	 * Extension hook that subclasses can use to create a data binder of a specific type. 
+	 * The default implementation creates a {@link WebRequestDataBinder}.
+	 * @param target the data binding target object; or {@code null} for type conversion on simple objects.
 	 * @param objectName the name of the target object 
+	 * @param webRequest the current request
 	 */
-	protected WebDataBinder createBinderInstance(Object target, String objectName) {
+	protected WebDataBinder createBinderInstance(Object target, String objectName, NativeWebRequest webRequest) {
 		return new WebRequestDataBinder(target, objectName);
 	}
 
+	/**
+	 * Extension hook that subclasses can override to initialize further the data binder.
+	 * Will be invoked after the data binder is initialized through the {@link WebBindingInitializer}. 
+	 * @param dataBinder the data binder instance to customize
+	 * @param webRequest the current request
+	 * @throws Exception if initialization fails
+	 */
+	protected void initBinder(WebDataBinder dataBinder, NativeWebRequest webRequest) throws Exception {
+	}
+
 }

org.springframework.web/src/main/java/org/springframework/web/method/annotation/InitBinderDataBinderFactory.java

@@ -18,63 +18,67 @@ package org.springframework.web.method.annotation;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Collection;
 import java.util.List;
-import java.util.Set;
 
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.InitBinder;
 import org.springframework.web.bind.support.DefaultDataBinderFactory;
 import org.springframework.web.bind.support.WebBindingInitializer;
 import org.springframework.web.context.request.NativeWebRequest;
+import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.method.support.InvocableHandlerMethod;
 
 /**
- * A specialization of {@link DefaultDataBinderFactory} that further initializes {@link WebDataBinder} instances
- * by invoking {@link InitBinder} methods.
+ * Adds data binder initialization through the invocation of @{@link InitBinder} methods.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
  */
 public class InitBinderDataBinderFactory extends DefaultDataBinderFactory {
 
-	private final List<InvocableHandlerMethod> initBinderMethods;
+	private final List<InvocableHandlerMethod> binderMethods;
 	
 	/**
-	 * Create an {@code InitBinderDataBinderFactory} instance with the given {@link InitBinder} methods.
-	 * @param binderMethods {@link InitBinder} methods to use to invoke to initialize new data binder instances
-	 * @param bindingInitializer a {@link WebBindingInitializer} to initialize new data binder instances with
+	 * Create a new instance.
+	 * @param binderMethods {@link InitBinder} methods to initialize new data binder instances with
+	 * @param initializer a global initializer to initialize new data binder instances with
 	 */
-	public InitBinderDataBinderFactory(List<InvocableHandlerMethod> binderMethods,
-									   WebBindingInitializer bindingInitializer) {
-		super(bindingInitializer);
-		this.initBinderMethods = (binderMethods != null) ? binderMethods : new ArrayList<InvocableHandlerMethod>();
+	public InitBinderDataBinderFactory(List<InvocableHandlerMethod> binderMethods, WebBindingInitializer initializer) {
+		super(initializer);
+		this.binderMethods = (binderMethods != null) ? binderMethods : new ArrayList<InvocableHandlerMethod>();
 	}
 
 	/**
-	 * Create a {@link WebDataBinder} for the given object and initialize it by calling {@link InitBinder} methods.
-	 * Only methods with an {@link InitBinder} annotation value that doesn't list attributes names or methods with
-	 * an {@link InitBinder} annotation value that matches the target object name are invoked.
+	 * Initializes the given data binder through the invocation of @{@link InitBinder} methods.
+	 * An @{@link InitBinder} method that defines names via {@link InitBinder#value()} will 
+	 * not be invoked unless one of the names matches the target object name.
 	 * @see InitBinder#value()
+	 * @throws Exception if one of the invoked @{@link InitBinder} methods fail.
 	 */
 	@Override
-	public WebDataBinder createBinder(NativeWebRequest request, Object target, String objectName) throws Exception {
-		WebDataBinder dataBinder = super.createBinder(request, target, objectName);
-
-		for (InvocableHandlerMethod binderMethod : this.initBinderMethods) {
-			InitBinder annot = binderMethod.getMethodAnnotation(InitBinder.class);
-			Set<String> attributeNames = new HashSet<String>(Arrays.asList(annot.value()));
-			
-			if (attributeNames.size() == 0 || attributeNames.contains(objectName)) {
-				Object returnValue = binderMethod.invokeForRequest(request, null, dataBinder);
-				
-				if (returnValue != null) {
-					throw new IllegalStateException("InitBinder methods must not have a return value: " + binderMethod);
-				}
+	public void initBinder(WebDataBinder binder, NativeWebRequest request) throws Exception {
+		for (InvocableHandlerMethod binderMethod : this.binderMethods) {
+			if (!isBinderMethodApplicable(binderMethod, binder)) {
+				continue;
+			}
+			Object returnValue = binderMethod.invokeForRequest(request, null, binder);
+			if (returnValue != null) {
+				throw new IllegalStateException("This @InitBinder method does not return void: " + binderMethod);
 			}
 		}
-	
-		return dataBinder;
+	}
+
+	/**
+	 * Returns {@code true} if the given @{@link InitBinder} method should be invoked to initialize
+	 * the given {@link WebDataBinder} instance. This implementations returns {@code true} if 
+	 * the @{@link InitBinder} annotation on the method does not define any names or if one of the
+	 * names it defines names matches the target object name.
+	 */
+	protected boolean isBinderMethodApplicable(HandlerMethod binderMethod, WebDataBinder binder) {
+		InitBinder annot = binderMethod.getMethodAnnotation(InitBinder.class);
+		Collection<String> names = Arrays.asList(annot.value());
+		return (names.size() == 0 || names.contains(binder.getObjectName()));
 	}
 	
-}
+}