DataBinder activates autoGrowNestedPaths by default; fixed enum binding with WebRequestDataBinder

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

org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapper.java

@@ -78,4 +78,18 @@ public interface BeanWrapper extends ConfigurablePropertyAccessor {
 	 */
 	PropertyDescriptor getPropertyDescriptor(String propertyName) throws InvalidPropertyException;
 
+	/**
+	 * Set if this BeanWrapper should attempt to "auto-grow" a nested path that contains a null value.
+	 * <p>If true, a null path location will be populated with a default object value and traversed
+	 * instead of resulting in a {@link NullValueInNestedPathException}. Turning this flag on also
+	 * enables auto-growth of collection elements when an index that is out of bounds is accessed.
+	 * <p>Default is false.
+	 */
+	void setAutoGrowNestedPaths(boolean autoGrowNestedPaths);
+
+	/**
+	 * Return whether "auto-growing" of nested paths has been activated.
+	 */
+	boolean isAutoGrowNestedPaths();
+
 }

org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java

@@ -114,7 +114,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 	/** The security context used for invoking the property methods */
 	private AccessControlContext acc;
 
-	private boolean autoGrowNestedPaths;
+	private boolean autoGrowNestedPaths = false;
+
 
 	/**
 	 * Create new empty BeanWrapperImpl. Wrapped instance needs to be set afterwards.
@@ -252,25 +253,14 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		return (this.rootObject != null ? this.rootObject.getClass() : null);
 	}
 
-	/**
-	 * If this BeanWrapper should attempt to "autogrow" a nested path that contains a null value.
-	 * If true, a null path location will be populated with a default object value and traversed instead of resulting in a {@link NullValueInNestedPathException}.
-	 * Turning this flag on also enables auto-growth of collection elements when an index that is out of bounds is accessed.
-	 */
-	public boolean getAutoGrowNestedPaths() {
-		return this.autoGrowNestedPaths;
-	}
-
-	/**
-	 * Sets if this BeanWrapper should attempt to "autogrow" a nested path that contains a null value.
-	 * If true, a null path location will be populated with a default object value and traversed instead of resulting in a {@link NullValueInNestedPathException}.
-	 * Turning this flag on also enables auto-growth of collection elements when an index that is out of bounds is accessed.
-	 * Default is false.
-	 */
 	public void setAutoGrowNestedPaths(boolean autoGrowNestedPaths) {
 		this.autoGrowNestedPaths = autoGrowNestedPaths;
 	}
 
+	public boolean isAutoGrowNestedPaths() {
+		return this.autoGrowNestedPaths;
+	}
+
 	/**
 	 * Set the class to introspect.
 	 * Needs to be called when the target object changes.
@@ -506,9 +496,10 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		String canonicalName = tokens.canonicalName;
 		Object propertyValue = getPropertyValue(tokens);
 		if (propertyValue == null) {
-			if (autoGrowNestedPaths) {
+			if (this.autoGrowNestedPaths) {
 				propertyValue = setDefaultValue(tokens);
-			} else {
+			}
+			else {
 				throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + canonicalName);				
 			}
 		}
@@ -561,20 +552,22 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 					Object array = Array.newInstance(componentType, 1);
 					Array.set(array, 0, Array.newInstance(componentType.getComponentType(), 0));
 					return array;
-				} else {
+				}
+				else {
 					return Array.newInstance(componentType, 0);
 				}
-			} else {
+			}
+			else {
 				if (Collection.class.isAssignableFrom(type)) {
 					return CollectionFactory.createCollection(type, 16);
-				} else {
+				}
+				else {
 					return type.newInstance();
 				}
 			}
-		} catch (InstantiationException e) {
-			throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + name, "Could not instantiate propertyType [" + type.getName() + "] to auto-grow nested property path");
-		} catch (IllegalAccessException e) {
-			throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + name, "Could not instantiate propertyType [" + type.getName() + "] to auto-grow nested property path");
+		}
+		catch (Exception ex) {
+			throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + name, "Could not instantiate property type [" + type.getName() + "] to auto-grow nested property path: " + ex);
 		}
 	}
 	
@@ -685,9 +678,10 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 			
 			if (tokens.keys != null) {				
 				if (value == null) {
-					if (autoGrowNestedPaths) {
+					if (this.autoGrowNestedPaths) {
 						value = setDefaultValue(tokens.actualName);
-					} else {
+					}
+					else {
 						throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + propertyName,
 								"Cannot access indexed value of property referenced in indexed " +
 								"property path '" + propertyName + "': returned null");							
@@ -775,7 +769,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 	}
 
 	private Object growArrayIfNecessary(Object array, int index, String name) {
-		if (!autoGrowNestedPaths) {
+		if (!this.autoGrowNestedPaths) {
 			return array;
 		}
 		int length = Array.getLength(array);
@@ -794,7 +788,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 	}
 	
 	private void growCollectionIfNecessary(Collection collection, int index, String name, PropertyDescriptor pd, int nestingLevel) {
-		if (!autoGrowNestedPaths) {
+		if (!this.autoGrowNestedPaths) {
 			return;
 		}
 		if (index >= collection.size()) {

org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java

@@ -24,6 +24,7 @@ import java.lang.reflect.Field;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Arrays;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -192,11 +193,7 @@ class TypeConverterDelegate {
 			// Try to apply some standard type conversion rules if appropriate.
 
 			if (convertedValue != null) {
-				if (String.class.equals(requiredType) && ClassUtils.isPrimitiveOrWrapper(convertedValue.getClass())) {
-					// We can stringify any primitive value...
-					return (T) convertedValue.toString();
-				}
-				else if (requiredType.isArray()) {
+				if (requiredType.isArray()) {
 					// Array required -> apply appropriate conversion of elements.
 					return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType());
 				}
@@ -210,6 +207,13 @@ class TypeConverterDelegate {
 					convertedValue = convertToTypedMap(
 							(Map) convertedValue, propertyName, requiredType, methodParam);
 				}
+				if (convertedValue.getClass().isArray() && Array.getLength(convertedValue) == 1) {
+					convertedValue = Array.get(convertedValue, 0);
+				}
+				if (String.class.equals(requiredType) && ClassUtils.isPrimitiveOrWrapper(convertedValue.getClass())) {
+					// We can stringify any primitive value...
+					return (T) convertedValue.toString();
+				}
 				else if (convertedValue instanceof String && !requiredType.isInstance(convertedValue)) {
 					if (!requiredType.isInterface() && !requiredType.isEnum()) {
 						try {
@@ -264,20 +268,19 @@ class TypeConverterDelegate {
 	private Object attemptToConvertStringToEnum(Class<?> requiredType, String trimmedValue, Object currentConvertedValue) {
 		Object convertedValue = currentConvertedValue;
 
-		if(Enum.class.equals(requiredType)) {
+		if (Enum.class.equals(requiredType)) {
 			// target type is declared as raw enum, treat the trimmed value as <enum.fqn>.FIELD_NAME
 			int index = trimmedValue.lastIndexOf(".");
-			if(index > - 1) {
+			if (index > - 1) {
 				String enumType = trimmedValue.substring(0, index);
 				String fieldName = trimmedValue.substring(index + 1);
-
 				ClassLoader loader = this.targetObject.getClass().getClassLoader();
-
 				try {
 					Class<?> enumValueType = loader.loadClass(enumType);
 					Field enumField = enumValueType.getField(fieldName);
 					convertedValue = enumField.get(null);
-				} catch(ClassNotFoundException ex) {
+				}
+				catch (ClassNotFoundException ex) {
 					if(logger.isTraceEnabled()) {
 						logger.trace("Enum class [" + enumType + "] cannot be loaded from [" + loader + "]", ex);
 					}
@@ -289,6 +292,7 @@ class TypeConverterDelegate {
 				}
 			}
 		}
+
 		if (convertedValue == currentConvertedValue) {
 			// Try field lookup as fallback: for JDK 1.5 enum or custom enum
 			// with values defined as static fields. Resulting value still needs

org.springframework.context/src/main/java/org/springframework/validation/BeanPropertyBindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2009 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.
@@ -73,6 +73,7 @@ public class BeanPropertyBindingResult extends AbstractPropertyBindingResult imp
 		if (this.beanWrapper == null) {
 			this.beanWrapper = createBeanWrapper();
 			this.beanWrapper.setExtractOldValueForEditor(true);
+			this.beanWrapper.setAutoGrowNestedPaths(true);
 		}
 		return this.beanWrapper;
 	}

org.springframework.web/src/test/java/org/springframework/beans/TestBean.java

@@ -206,6 +206,14 @@ public class TestBean implements BeanNameAware, BeanFactoryAware, ITestBean, IOt
 		return (spouses != null ? spouses[0] : null);
 	}
 
+	public void setConcreteSpouse(TestBean spouse) {
+		this.spouses = new ITestBean[] {spouse};
+	}
+
+	public TestBean getConcreteSpouse() {
+		return (spouses != null ? (TestBean) spouses[0] : null);
+	}
+
 	public void setSpouse(ITestBean spouse) {
 		this.spouses = new ITestBean[] {spouse};
 	}

org.springframework.web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java

@@ -0,0 +1,281 @@
+/*
+ * Copyright 2002-2009 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.bind.support;
+
+import java.beans.PropertyEditorSupport;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.*;
+import org.junit.Test;
+
+import org.springframework.beans.ITestBean;
+import org.springframework.beans.PropertyValue;
+import org.springframework.beans.PropertyValues;
+import org.springframework.beans.TestBean;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.web.bind.ServletRequestParameterPropertyValues;
+import org.springframework.web.context.request.ServletWebRequest;
+
+/**
+ * @author Juergen Hoeller
+ */
+public class WebRequestDataBinderTests {
+
+	@Test
+	public void testBindingWithNestedObjectCreation() throws Exception {
+		TestBean tb = new TestBean();
+
+		WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
+		binder.registerCustomEditor(ITestBean.class, new PropertyEditorSupport() {
+			public void setAsText(String text) throws IllegalArgumentException {
+				setValue(new TestBean());
+			}
+		});
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("spouse", "someValue");
+		request.addParameter("spouse.name", "test");
+		binder.bind(new ServletWebRequest(request));
+
+		assertNotNull(tb.getSpouse());
+		assertEquals("test", tb.getSpouse().getName());
+	}
+
+	@Test
+	public void testBindingWithNestedObjectCreationThroughAutoGrow() throws Exception {
+		TestBean tb = new TestBean();
+
+		WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
+		binder.setIgnoreUnknownFields(false);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("concreteSpouse.name", "test");
+		binder.bind(new ServletWebRequest(request));
+
+		assertNotNull(tb.getSpouse());
+		assertEquals("test", tb.getSpouse().getName());
+	}
+
+	@Test
+	public void testFieldPrefixCausesFieldReset() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("_postProcessed", "visible");
+		request.addParameter("postProcessed", "on");
+		binder.bind(new ServletWebRequest(request));
+		assertTrue(target.isPostProcessed());
+
+		request.removeParameter("postProcessed");
+		binder.bind(new ServletWebRequest(request));
+		assertFalse(target.isPostProcessed());
+	}
+
+	@Test
+	public void testFieldPrefixCausesFieldResetWithIgnoreUnknownFields() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+		binder.setIgnoreUnknownFields(false);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("_postProcessed", "visible");
+		request.addParameter("postProcessed", "on");
+		binder.bind(new ServletWebRequest(request));
+		assertTrue(target.isPostProcessed());
+
+		request.removeParameter("postProcessed");
+		binder.bind(new ServletWebRequest(request));
+		assertFalse(target.isPostProcessed());
+	}
+
+	@Test
+	public void testFieldDefault() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("!postProcessed", "off");
+		request.addParameter("postProcessed", "on");
+		binder.bind(new ServletWebRequest(request));
+		assertTrue(target.isPostProcessed());
+
+		request.removeParameter("postProcessed");
+		binder.bind(new ServletWebRequest(request));
+		assertFalse(target.isPostProcessed());
+	}
+
+	@Test
+	public void testFieldDefaultPreemptsFieldMarker() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("!postProcessed", "on");
+		request.addParameter("_postProcessed", "visible");
+		request.addParameter("postProcessed", "on");
+		binder.bind(new ServletWebRequest(request));
+		assertTrue(target.isPostProcessed());
+
+		request.removeParameter("postProcessed");
+		binder.bind(new ServletWebRequest(request));
+		assertTrue(target.isPostProcessed());
+
+		request.removeParameter("!postProcessed");
+		binder.bind(new ServletWebRequest(request));
+		assertFalse(target.isPostProcessed());
+	}
+
+	@Test
+	public void testFieldDefaultNonBoolean() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("!name", "anonymous");
+		request.addParameter("name", "Scott");
+		binder.bind(new ServletWebRequest(request));
+		assertEquals("Scott", target.getName());
+
+		request.removeParameter("name");
+		binder.bind(new ServletWebRequest(request));
+		assertEquals("anonymous", target.getName());
+	}
+
+	@Test
+	public void testWithCommaSeparatedStringArray() throws Exception {
+		TestBean target = new TestBean();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("stringArray", "bar");
+		request.addParameter("stringArray", "abc");
+		request.addParameter("stringArray", "123,def");
+		binder.bind(new ServletWebRequest(request));
+		assertEquals("Expected all three items to be bound", 3, target.getStringArray().length);
+
+		request.removeParameter("stringArray");
+		request.addParameter("stringArray", "123,def");
+		binder.bind(new ServletWebRequest(request));
+		assertEquals("Expected only 1 item to be bound", 1, target.getStringArray().length);
+	}
+
+	@Test
+	public void testEnumBinding() {
+		EnumHolder target = new EnumHolder();
+		WebRequestDataBinder binder = new WebRequestDataBinder(target);
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("myEnum", "FOO");
+		binder.bind(new ServletWebRequest(request));
+		assertEquals(MyEnum.FOO, target.getMyEnum());
+	}
+
+	@Test
+	public void testNoPrefix() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("forname", "Tony");
+		request.addParameter("surname", "Blair");
+		request.addParameter("age", "" + 50);
+
+		ServletRequestParameterPropertyValues pvs = new ServletRequestParameterPropertyValues(request);
+		doTestTony(pvs);
+	}
+
+	@Test
+	public void testPrefix() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("test_forname", "Tony");
+		request.addParameter("test_surname", "Blair");
+		request.addParameter("test_age", "" + 50);
+
+		ServletRequestParameterPropertyValues pvs = new ServletRequestParameterPropertyValues(request);
+		assertTrue("Didn't fidn normal when given prefix", !pvs.contains("forname"));
+		assertTrue("Did treat prefix as normal when not given prefix", pvs.contains("test_forname"));
+
+		pvs = new ServletRequestParameterPropertyValues(request, "test");
+		doTestTony(pvs);
+	}
+
+	@Test
+	public void testNoParameters() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		ServletRequestParameterPropertyValues pvs = new ServletRequestParameterPropertyValues(request);
+		assertTrue("Found no parameters", pvs.getPropertyValues().length == 0);
+	}
+
+	@Test
+	public void testMultipleValuesForParameter() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		String[] original = new String[] {"Tony", "Rod"};
+		request.addParameter("forname", original);
+
+		ServletRequestParameterPropertyValues pvs = new ServletRequestParameterPropertyValues(request);
+		assertTrue("Found 1 parameter", pvs.getPropertyValues().length == 1);
+		assertTrue("Found array value", pvs.getPropertyValue("forname").getValue() instanceof String[]);
+		String[] values = (String[]) pvs.getPropertyValue("forname").getValue();
+		assertEquals("Correct values", Arrays.asList(values), Arrays.asList(original));
+	}
+
+	/**
+	 * Must contain: forname=Tony surname=Blair age=50
+	 */
+	protected void doTestTony(PropertyValues pvs) throws Exception {
+		assertTrue("Contains 3", pvs.getPropertyValues().length == 3);
+		assertTrue("Contains forname", pvs.contains("forname"));
+		assertTrue("Contains surname", pvs.contains("surname"));
+		assertTrue("Contains age", pvs.contains("age"));
+		assertTrue("Doesn't contain tory", !pvs.contains("tory"));
+
+		PropertyValue[] pvArray = pvs.getPropertyValues();
+		Map<String, String> m = new HashMap<String, String>();
+		m.put("forname", "Tony");
+		m.put("surname", "Blair");
+		m.put("age", "50");
+		for (PropertyValue pv : pvArray) {
+			Object val = m.get(pv.getName());
+			assertTrue("Can't have unexpected value", val != null);
+			assertTrue("Val i string", val instanceof String);
+			assertTrue("val matches expected", val.equals(pv.getValue()));
+			m.remove(pv.getName());
+		}
+		assertTrue("Map size is 0", m.size() == 0);
+	}
+
+
+	public static class EnumHolder {
+
+	   private MyEnum myEnum;
+
+	   public MyEnum getMyEnum() {
+		   return myEnum;
+	   }
+
+	   public void setMyEnum(MyEnum myEnum) {
+		   this.myEnum = myEnum;
+	   }
+   }
+
+
+   public enum MyEnum {
+
+	   FOO, BAR
+   }
+
+}