From 5c67dbf42466ee984c48c9b04e58d4039d614848 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Mon, 23 May 2011 05:21:02 +0000 Subject: [PATCH] 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 --- .../convert/ConversionFailedException.java | 3 +- .../convert/ConverterNotFoundException.java | 3 +- .../core/convert/TypeDescriptor.java | 91 +++++++++--- .../support/CollectionToArrayConverter.java | 6 +- .../CollectionToCollectionConverter.java | 3 - .../support/CollectionToObjectConverter.java | 6 +- .../convert/support/MapToMapConverter.java | 9 -- .../CollectionToCollectionConverterTests.java | 140 ++++++++++++++++++ .../support/MapToMapConverterTests.java | 14 +- .../core/convert/support/Spr7728Tests.java | 6 +- 10 files changed, 231 insertions(+), 50 deletions(-) create mode 100644 org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/ConversionFailedException.java b/org.springframework.core/src/main/java/org/springframework/core/convert/ConversionFailedException.java index f1880fea86f..63f4678f58f 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/ConversionFailedException.java +++ b/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; diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java b/org.springframework.core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java index e7e9fd5c745..fcfc6f574ab 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java +++ b/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; } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index edb8600e2a2..9d1e490e591 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/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> classQueue = new LinkedList>(); + 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> 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; + } + + } + } \ No newline at end of file diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java index 36e084ad951..e26bae837f7 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java +++ b/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) { diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java index f59edfc0331..c97da5c74ea 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java +++ b/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); } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java index d5785b2df56..c97b4c9f685 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java +++ b/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) { diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java index 5748ac60da9..ba1f81a1f47 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java +++ b/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()); } diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java new file mode 100644 index 00000000000..51260ea6616 --- /dev/null +++ b/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 resources = new ArrayList(); + 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 resources = new ArrayList(); + 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 resources = new ArrayList(); + 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 resources = new ArrayList(); + 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 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 { + + } +} + diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java index 7876f7169f8..d70e7d4ea6d 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java +++ b/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 map = new HashMap(); + 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> map = new HashMap>(); @@ -115,7 +127,7 @@ public class MapToMapConverterTests { Map map = new HashMap(); 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)); } diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/Spr7728Tests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/Spr7728Tests.java index 54aadf3cafd..ab1ea31e160 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/Spr7728Tests.java +++ b/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 theSrcVector; - private TypeDescriptor theSrcType; private TypeDescriptor theTargetType; @Before public void setup() { theSrcVector = new Vector(); - 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 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()); }