Merge pull request #94 from dsyer/SPR-9498

SPR-9498: relax logic detecting successful property editor after conversion exception

.gitignore

@@ -4,6 +4,7 @@
 .DS_Store
 .settings
 .springBeans
+target
 bin
 build.sh
 integration-repo
@@ -17,6 +18,7 @@ build
 .classpath
 .project
 argfile*
+pom.xml
 
 # IDEA metadata and output dirs
 *.iml

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

@@ -44,6 +44,7 @@ import org.springframework.util.StringUtils;
  *
  * @author Juergen Hoeller
  * @author Rob Harrop
+ * @author Dave Syer
  * @since 2.0
  * @see BeanWrapperImpl
  * @see SimpleTypeConverter
@@ -244,7 +245,7 @@ class TypeConverterDelegate {
 		}
 
 		if (firstAttemptEx != null) {
-			if (editor == null && convertedValue == newValue) {
+			if (editor == null && convertedValue == newValue && requiredType!=null && !ClassUtils.isAssignableValue(requiredType, convertedValue)) {
 				throw firstAttemptEx;
 			}
 			logger.debug("Original ConversionService attempt failed - ignored since " +

spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java

@@ -43,14 +43,16 @@ import java.util.TreeMap;
 import java.util.TreeSet;
 
 import org.apache.commons.logging.LogFactory;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowire;
 import org.springframework.beans.propertyeditors.CustomNumberEditor;
 import org.springframework.beans.propertyeditors.StringArrayPropertyEditor;
 import org.springframework.beans.propertyeditors.StringTrimmerEditor;
 import org.springframework.beans.support.DerivedFromProtectedBaseBean;
+import org.springframework.core.convert.ConversionFailedException;
+import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.support.DefaultConversionService;
+import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.util.StopWatch;
 import org.springframework.util.StringUtils;
 
@@ -66,9 +68,34 @@ import test.beans.TestBean;
  * @author Alef Arendsen
  * @author Arjen Poutsma
  * @author Chris Beams
+ * @author Dave Syer
  */
 public final class BeanWrapperTests {
 
+	@Test
+	public void testNullNestedTypeDescriptorWithNoConversionService() {
+		Foo foo = new Foo();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(foo);
+		wrapper.setAutoGrowNestedPaths(true);
+		wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9");
+		assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber"));
+	}
+	
+	@Test
+	public void testNullNestedTypeDescriptorWithBadConversionService() {
+		Foo foo = new Foo();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(foo);
+		wrapper.setConversionService(new GenericConversionService() {
+			@Override
+			public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
+				throw new ConversionFailedException(sourceType, targetType, source, null);
+			}
+		});
+		wrapper.setAutoGrowNestedPaths(true);
+		wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9");
+		assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber"));
+	}
+	
 	@Test
 	public void testNullNestedTypeDescriptor() {
 		Foo foo = new Foo();

spring-core/src/main/java/org/springframework/core/CollectionFactory.java

@@ -35,6 +35,8 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArraySet;
 
 import org.springframework.util.LinkedCaseInsensitiveMap;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
 
 /**
  * Factory for collections, being aware of Java 5 and Java 6 collections.
@@ -305,6 +307,9 @@ public abstract class CollectionFactory {
 			else if (SortedMap.class.equals(mapType) || mapType.equals(navigableMapClass)) {
 				return new TreeMap();
 			}
+			else if (MultiValueMap.class.equals(mapType)) {
+				return new LinkedMultiValueMap();
+			}
 			else {
 				throw new IllegalArgumentException("Unsupported Map interface: " + mapType.getName());
 			}

spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java

@@ -24,9 +24,12 @@ import java.util.Set;
 
 import junit.framework.TestCase;
 
+import org.springframework.util.MultiValueMap;
+
 /**
  * @author Darren Davison
  * @author Juergen Hoeller
+ * @author Dave Syer
  */
 public class CollectionFactoryTests extends TestCase {
 
@@ -50,6 +53,11 @@ public class CollectionFactoryTests extends TestCase {
 		assertTrue(map.getClass().getName().endsWith("ConcurrentHashMap"));
 	}
 
+	public void testMultiValueMap() {
+		Map map = CollectionFactory.createMap(MultiValueMap.class, 16);
+		assertTrue(map.getClass().getName().endsWith("MultiValueMap"));
+	}
+
 	public void testConcurrentMapWithExplicitInterface() {
 		ConcurrentMap map = CollectionFactory.createConcurrentMap(16);
 		assertTrue(map.getClass().getSuperclass().getName().endsWith("ConcurrentHashMap"));