revised findCommonElement handling within TypeDescriptor.forObject(Object); we now fully introspect the collection elements to resolve the common type. We also support nested introspection e.g. collections of collections. Object.class is used to indicate no common type, and TypeDescriptor.NULL is used to indicate a null element value

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

org.springframework.core/src/main/java/org/springframework/core/convert/ConversionFailedException.java

@@ -43,8 +43,7 @@ public final class ConversionFailedException extends ConversionException {
 	 * @param cause the cause of the conversion failure
 	 */
 	public ConversionFailedException(TypeDescriptor sourceType, TypeDescriptor targetType, Object value, Throwable cause) {
-		super("Unable to convert value \"" + ObjectUtils.nullSafeToString(value) + "\" from type '" +
-				sourceType.getName() + "' to type '" + targetType.getName() + "'", cause);
+		super("Failed to convert from type " + sourceType + " to type " + targetType + " for value '" + ObjectUtils.nullSafeToString(value) + "'", cause);
 		this.sourceType = sourceType;
 		this.targetType = targetType;
 		this.value = value;

org.springframework.core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java

@@ -37,8 +37,7 @@ public final class ConverterNotFoundException extends ConversionException {
 	 * @param message a descriptive message
 	 */
 	public ConverterNotFoundException(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		super("No converter found capable of converting from '" + sourceType.getName() +
-				"' to '" + targetType.getName() + "'");
+		super("No converter found capable of converting from type " + sourceType + " to type " + targetType);
 		this.sourceType = sourceType;
 		this.targetType = targetType;
 	}

org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java

@@ -20,6 +20,7 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.Map;
 
 import org.springframework.core.GenericCollectionTypeResolver;
@@ -515,20 +516,51 @@ public class TypeDescriptor {
 		return methodParameter;
 	}
 
-	private static Object findCommonElement(Collection<?> values) {
+	private static CommonElement findCommonElement(Collection<?> values) {
+		Class<?> commonType = null;
 		Object candidate = null;
 		for (Object value : values) {
 			if (value != null) {
 				if (candidate == null) {
+					commonType = value.getClass();
 					candidate = value;
-				} else if (candidate.getClass() != value.getClass()) {
-					return null;
+				} else {
+					commonType = commonType(commonType, value.getClass());
+					if (commonType == Object.class) {
+						return null;
+					}
 				}
 			}
 		}
-		return candidate;
+		return new CommonElement(commonType, candidate);
+	}
+
+	private static Class<?> commonType(Class<?> commonType, Class<?> valueClass) {
+		LinkedList<Class<?>> classQueue = new LinkedList<Class<?>>();
+		classQueue.addFirst(commonType);
+		while (!classQueue.isEmpty()) {
+			Class<?> currentClass = classQueue.removeLast();
+			if (currentClass.isAssignableFrom(valueClass)) {
+				return currentClass;
+			}
+			Class<?>[] interfaces = currentClass.getInterfaces();
+			for (Class<?> ifc : interfaces) {
+				addInterfaceHierarchy(ifc, classQueue);
+			}
+			if (currentClass.getSuperclass() != null) {
+				classQueue.addFirst(currentClass.getSuperclass());
+			}
+		}
+		throw new IllegalStateException("Should never be invoked");	
 	}
 	
+	private static void addInterfaceHierarchy(Class<?> ifc, LinkedList<Class<?>> classQueue) {
+		classQueue.addFirst(ifc);
+		for (Class<?> inheritedIfc : ifc.getInterfaces()) {
+			addInterfaceHierarchy(inheritedIfc, classQueue);
+		}
+	}
+
 	// internal constructors
 
 	private TypeDescriptor() {
@@ -545,38 +577,61 @@ public class TypeDescriptor {
 		this.fieldNestingLevel = nestingLevel;
 	}
 
-	public TypeDescriptor(Class<?> mapType, Object commonKey, Object commonValue) {
+	public TypeDescriptor(Class<?> mapType, CommonElement commonKey, CommonElement commonValue) {
 		this.type = mapType;
-		this.mapKeyType = applyIndexedObject(commonKey);
-		this.mapValueType = applyIndexedObject(commonValue);
+		this.mapKeyType = applyCommonElement(commonKey);
+		this.mapValueType = applyCommonElement(commonValue);
 	}
 	
-	public TypeDescriptor(Class<?> collectionType, Object commonElement) {
+	public TypeDescriptor(Class<?> collectionType, CommonElement commonElement) {
 		this.type = collectionType;
-		this.elementType = applyIndexedObject(commonElement);
+		this.elementType = applyCommonElement(commonElement);
 	}
 
-	private TypeDescriptor applyIndexedObject(Object object) {
-		if (object == null) {
+	private TypeDescriptor applyCommonElement(CommonElement commonElement) {
+		if (commonElement == null) {
 			return TypeDescriptor.valueOf(Object.class);
 		}
-		if (object instanceof Collection<?>) {
-			Collection<?> collection = (Collection<?>) object;
+		if (commonElement.getValue() instanceof Collection<?>) {
+			Collection<?> collection = (Collection<?>) commonElement.getValue();
 			if (collection.size() == 0) {
 				return TypeDescriptor.valueOf(Object.class);
 			}
-			return new TypeDescriptor(object.getClass(), findCommonElement((Collection<?>) object));			
+			return new TypeDescriptor(commonElement.getType(), findCommonElement(collection));			
 		}
-		else if (object instanceof Map<?, ?>) {
-			Map<?, ?> map = (Map<?, ?>) object;
+		else if (commonElement.getValue() instanceof Map<?, ?>) {
+			Map<?, ?> map = (Map<?, ?>) commonElement.getValue();
 			if (map.size() == 0) {
 				return TypeDescriptor.valueOf(Object.class);				
 			}
-			return new TypeDescriptor(object.getClass(), findCommonElement(map.keySet()), findCommonElement(map.values()));
+			return new TypeDescriptor(commonElement.getType(), findCommonElement(map.keySet()), findCommonElement(map.values()));
 		}
 		else {
-			return TypeDescriptor.valueOf(object.getClass());
+			return TypeDescriptor.valueOf(commonElement.getType());
 		}
 	}
 
+	// inner classes
+	
+	private static class CommonElement {
+		
+		private Class<?> type;
+		
+		private Object value;
+
+		public CommonElement(Class<?> type, Object value) {
+			this.type = type;
+			this.value = value;
+		}
+
+		public Class<?> getType() {
+			return type;
+		}
+
+		public Object getValue() {
+			return value;
+		}
+		
+	}
+	
 }

org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java

@@ -49,11 +49,7 @@ final class CollectionToArrayConverter implements ConditionalGenericConverter {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		TypeDescriptor sourceElementType = sourceType.getElementTypeDescriptor();
-		if (Object.class.equals(sourceElementType.getType())) {
-			return true;
-		}		
-		return this.conversionService.canConvert(sourceElementType, targetType.getElementTypeDescriptor());
+		return this.conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType.getElementTypeDescriptor());
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {

org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java

@@ -51,9 +51,6 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
 		TypeDescriptor sourceElementType = sourceType.getElementTypeDescriptor();
 		TypeDescriptor targetElementType = targetType.getElementTypeDescriptor();
-		if (Object.class.equals(sourceElementType.getType()) || Object.class.equals(targetElementType.getType())) {
-			return true;
-		}
 		return this.conversionService.canConvert(sourceElementType, targetElementType);
 	}
 	

org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java

@@ -43,11 +43,7 @@ final class CollectionToObjectConverter implements ConditionalGenericConverter {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		TypeDescriptor sourceElementType = sourceType.getElementTypeDescriptor();
-		if (Object.class.equals(sourceElementType.getType())) {
-			return true;
-		}		
-		return this.conversionService.canConvert(sourceElementType, targetType);
+		return this.conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType);
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {

org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java

@@ -49,15 +49,6 @@ final class MapToMapConverter implements ConditionalGenericConverter {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		TypeDescriptor sourceKeyType = sourceType.getMapKeyTypeDescriptor();
-		TypeDescriptor targetKeyType = targetType.getMapKeyTypeDescriptor();
-		TypeDescriptor sourceValueType = sourceType.getMapValueTypeDescriptor();
-		TypeDescriptor targetValueType = targetType.getMapValueTypeDescriptor();
-		if (Object.class.equals(sourceKeyType.getType()) && Object.class.equals(sourceValueType.getType())
-				|| Object.class.equals(targetKeyType.getType()) && Object.class.equals(targetValueType.getType())) {
-			// catches the empty map case
-			return true;
-		}		
 		return this.conversionService.canConvert(sourceType.getMapKeyTypeDescriptor(), targetType.getMapKeyTypeDescriptor()) && 
 			this.conversionService.canConvert(sourceType.getMapValueTypeDescriptor(), targetType.getMapValueTypeDescriptor());
 	}

org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java

@@ -0,0 +1,140 @@
+package org.springframework.core.convert.support;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.core.convert.ConverterNotFoundException;
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.FileSystemResource;
+import org.springframework.core.io.Resource;
+
+public class CollectionToCollectionConverterTests {
+
+	private GenericConversionService conversionService = new GenericConversionService();
+
+	@Before
+	public void setUp() {
+		conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
+	}
+
+	@Test
+	public void differentImpls() throws Exception {
+		List<Resource> resources = new ArrayList<Resource>();
+		resources.add(new ClassPathResource("test"));
+		resources.add(new FileSystemResource("test"));
+		resources.add(new TestResource());
+		TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
+		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
+	}
+
+	@Test
+	public void mixedInNulls() throws Exception {
+		List<Resource> resources = new ArrayList<Resource>();
+		resources.add(new ClassPathResource("test"));
+		resources.add(null);
+		resources.add(new FileSystemResource("test"));
+		resources.add(new TestResource());
+		TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
+		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
+	}
+
+	@Test
+	public void allNulls() throws Exception {
+		List<Resource> resources = new ArrayList<Resource>();
+		resources.add(null);
+		resources.add(null);
+		TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
+		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
+	}
+	
+	@Test(expected=ConverterNotFoundException.class)
+	public void nothingInCommon() throws Exception {
+		List<Object> resources = new ArrayList<Object>();
+		resources.add(new ClassPathResource("test"));
+		resources.add(3);
+		TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
+		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
+	}
+
+	public List<Resource> resources;
+
+	public static  abstract class BaseResource implements Resource {
+
+		public InputStream getInputStream() throws IOException {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public boolean exists() {
+			// TODO Auto-generated method stub
+			return false;
+		}
+
+		public boolean isReadable() {
+			// TODO Auto-generated method stub
+			return false;
+		}
+
+		public boolean isOpen() {
+			// TODO Auto-generated method stub
+			return false;
+		}
+
+		public URL getURL() throws IOException {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public URI getURI() throws IOException {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public File getFile() throws IOException {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public long contentLength() throws IOException {
+			// TODO Auto-generated method stub
+			return 0;
+		}
+
+		public long lastModified() throws IOException {
+			// TODO Auto-generated method stub
+			return 0;
+		}
+
+		public Resource createRelative(String relativePath) throws IOException {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public String getFilename() {
+			// TODO Auto-generated method stub
+			return null;
+		}
+
+		public String getDescription() {
+			// TODO Auto-generated method stub
+			return null;
+		}
+		
+	}
+	
+	public static class TestResource extends BaseResource {
+		
+	}
+}
+

org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java

@@ -51,6 +51,18 @@ public class MapToMapConverterTests {
 		assertEquals(map, conversionService.convert(map, Map.class));
 	}
 
+	@Test
+	public void scalarMapNotGenericSource() throws Exception {
+		Map<String, String> map = new HashMap<String, String>();
+		map.put("1", "9");
+		map.put("2", "37");
+		TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("notGenericMapSource"));
+		TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget"));		
+		assertFalse(conversionService.canConvert(sourceType, targetType));
+	}
+	
+	public Map notGenericMapSource;
+	
 	@Test
 	public void collectionMap() throws Exception {
 		Map<String, List<String>> map = new HashMap<String, List<String>>();
@@ -115,7 +127,7 @@ public class MapToMapConverterTests {
 		Map<String, String> map = new HashMap<String, String>();
 		TypeDescriptor sourceType = TypeDescriptor.forObject(map);
 		TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget"));
-		assertTrue(conversionService.canConvert(sourceType, targetType));
+		//assertTrue(conversionService.canConvert(sourceType, targetType));
 		assertEquals(map, conversionService.convert(map, sourceType, targetType));
 	}
 

org.springframework.core/src/test/java/org/springframework/core/convert/support/Spr7728Tests.java

@@ -13,14 +13,12 @@ public class Spr7728Tests
 {
     private CollectionToCollectionConverter theConverter;
     private Vector<String> theSrcVector;
-    private TypeDescriptor theSrcType;
     private TypeDescriptor theTargetType;
 
     @Before
     public void setup()
     {
         theSrcVector = new Vector<String>();
-        theSrcType = TypeDescriptor.forObject(theSrcVector);
         theTargetType = TypeDescriptor.forObject(new ArrayList());
         theConverter = new CollectionToCollectionConverter(new GenericConversionService());
     }
@@ -30,7 +28,6 @@ public class Spr7728Tests
         throws Exception
     {
         theSrcVector.add("Element");
-
         testCollectionConversionToArrayList(theSrcVector);
     }
 
@@ -43,8 +40,7 @@ public class Spr7728Tests
 
     private void testCollectionConversionToArrayList(Collection<String> aSource)
     {
-        Object myConverted = theConverter.convert(aSource, theSrcType, theTargetType);
-
+        Object myConverted = theConverter.convert(aSource, TypeDescriptor.forObject(aSource), theTargetType);
         Assert.assertTrue(myConverted instanceof ArrayList<?>);
         Assert.assertEquals(aSource.size(), ((ArrayList<?>) myConverted).size());
     }