diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 3e69ee4421..f2267fcf3c 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -431,14 +431,6 @@ public class GenericConversionService implements ConfigurableConversionService { */ private static class Converters { - private static final Set> IGNORED_CLASSES; - static { - Set> ignored = new HashSet>(); - ignored.add(Object.class); - ignored.add(Object[].class); - IGNORED_CLASSES = Collections.unmodifiableSet(ignored); - } - private final Set globalConverters = new LinkedHashSet(); @@ -483,12 +475,13 @@ public class GenericConversionService implements ConfigurableConversionService { */ public GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) { // Search the full type hierarchy - List sourceCandidates = getTypeHierarchy(sourceType); - List targetCandidates = getTypeHierarchy(targetType); - for (TypeDescriptor sourceCandidate : sourceCandidates) { - for (TypeDescriptor targetCandidate : targetCandidates) { + List> sourceCandidates = getClassHierarchy(sourceType.getType()); + List> targetCandidates = getClassHierarchy(targetType.getType()); + for (Class sourceCandidate : sourceCandidates) { + for (Class targetCandidate : targetCandidates) { + ConvertiblePair convertiblePair = new ConvertiblePair(sourceCandidate, targetCandidate); GenericConverter converter = getRegisteredConverter( - sourceType, targetType, sourceCandidate, targetCandidate); + sourceType, targetType, convertiblePair); if(converter != null) { return converter; } @@ -497,12 +490,11 @@ public class GenericConversionService implements ConfigurableConversionService { return null; } - private GenericConverter getRegisteredConverter(TypeDescriptor sourceType, TypeDescriptor targetType, - TypeDescriptor sourceCandidate, TypeDescriptor targetCandidate) { + private GenericConverter getRegisteredConverter(TypeDescriptor sourceType, + TypeDescriptor targetType, ConvertiblePair convertiblePair) { // Check specifically registered converters - ConvertersForPair convertersForPair = converters.get(new ConvertiblePair( - sourceCandidate.getType(), targetCandidate.getType())); + ConvertersForPair convertersForPair = converters.get(convertiblePair); GenericConverter converter = convertersForPair == null ? null : convertersForPair.getConverter(sourceType, targetType); if (converter != null) { @@ -512,7 +504,7 @@ public class GenericConversionService implements ConfigurableConversionService { // Check ConditionalGenericConverter that match all types for (GenericConverter globalConverter : this.globalConverters) { if (((ConditionalConverter)globalConverter).matches( - sourceCandidate, targetCandidate)) { + sourceType, targetType)) { return globalConverter; } } @@ -526,44 +518,38 @@ public class GenericConversionService implements ConfigurableConversionService { * @return an ordered list of all classes that the given type extends or * implements. */ - private List getTypeHierarchy(TypeDescriptor type) { - if(type.isPrimitive()) { - type = TypeDescriptor.valueOf(type.getObjectType()); - } - Set typeHierarchy = new LinkedHashSet(); - collectTypeHierarchy(typeHierarchy, type); - if(type.isArray()) { - typeHierarchy.add(TypeDescriptor.valueOf(Object[].class)); - } - typeHierarchy.add(TypeDescriptor.valueOf(Object.class)); - return new ArrayList(typeHierarchy); - } - - private void collectTypeHierarchy(Set typeHierarchy, - TypeDescriptor type) { - if(type != null && !IGNORED_CLASSES.contains(type.getType())) { - if(typeHierarchy.add(type)) { - Class superclass = type.getType().getSuperclass(); - if (type.isArray()) { - superclass = ClassUtils.resolvePrimitiveIfNecessary(superclass); - } - collectTypeHierarchy(typeHierarchy, createRelated(type, superclass)); - - for (Class implementsInterface : type.getType().getInterfaces()) { - collectTypeHierarchy(typeHierarchy, createRelated(type, implementsInterface)); - } + private List> getClassHierarchy(Class type) { + List> hierarchy = new ArrayList>(20); + Set> visited = new HashSet>(20); + addToClassHierarchy(0, ClassUtils.resolvePrimitiveIfNecessary(type), false, hierarchy, visited); + boolean array = type.isArray(); + int i = 0; + while (i < hierarchy.size()) { + Class candidate = hierarchy.get(i); + candidate = (array ? candidate.getComponentType() + : ClassUtils.resolvePrimitiveIfNecessary(candidate)); + Class superclass = candidate.getSuperclass(); + if (candidate.getSuperclass() != null && superclass != Object.class) { + addToClassHierarchy(i + 1, candidate.getSuperclass(), array, hierarchy, visited); } + for (Class implementedInterface : candidate.getInterfaces()) { + addToClassHierarchy(hierarchy.size(), implementedInterface, array, hierarchy, visited); + } + i++; } + addToClassHierarchy(hierarchy.size(), Object.class, array, hierarchy, visited); + addToClassHierarchy(hierarchy.size(), Object.class, false, hierarchy, visited); + return hierarchy; } - private TypeDescriptor createRelated(TypeDescriptor type, Class relatedType) { - if (relatedType == null && type.isArray()) { - relatedType = Array.newInstance(relatedType, 0).getClass(); + private void addToClassHierarchy(int index, Class type, boolean asArray, + List> hierarchy, Set> visited) { + if(asArray) { + type = Array.newInstance(type, 0).getClass(); } - if(!type.getType().equals(relatedType)) { - return type.upcast(relatedType); + if(visited.add(type)) { + hierarchy.add(index, type); } - return null; } @Override diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java index e5d7e27d3a..e1fdb3dee0 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java @@ -16,11 +16,7 @@ package org.springframework.core.convert.support; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertTrue; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; +import static org.junit.Assert.*; import java.awt.Color; import java.math.BigDecimal; @@ -506,6 +502,21 @@ public class DefaultConversionTests { assertEquals(source, result); } + @Test + public void convertCollectionToObjectWithCustomConverter() throws Exception { + List source = new ArrayList(); + source.add("A"); + source.add("B"); + conversionService.addConverter(new Converter() { + @Override + public ListWrapper convert(List source) { + return new ListWrapper(source); + } + }); + ListWrapper result = conversionService.convert(source, ListWrapper.class); + assertSame(source, result.getList()); + } + @Test public void convertObjectToCollection() { List result = (List) conversionService.convert(3L, List.class); @@ -777,4 +788,17 @@ public class DefaultConversionTests { } } + private static class ListWrapper { + + private List list; + + public ListWrapper(List list) { + this.list = list; + } + + public List getList() { + return list; + } + } + } diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java index fc2af6d87d..b5ede248ed 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -48,6 +47,7 @@ import org.springframework.core.io.Resource; import org.springframework.util.StopWatch; import org.springframework.util.StringUtils; +import static org.hamcrest.Matchers.greaterThan; import static org.junit.Assert.*; /** @@ -696,14 +696,11 @@ public class GenericConversionServiceTests { MyConditionalGenericConverter converter = new MyConditionalGenericConverter(); conversionService.addConverter(converter); assertEquals((Integer) 3, conversionService.convert(3, Integer.class)); + assertThat(converter.getSourceTypes().size(), greaterThan(2)); Iterator iterator = converter.getSourceTypes().iterator(); - assertEquals(Integer.class, iterator.next().getType()); - assertEquals(Number.class, iterator.next().getType()); - TypeDescriptor last = null; - while (iterator.hasNext()) { - last = iterator.next(); + while(iterator.hasNext()) { + assertEquals(Integer.class, iterator.next().getType()); } - assertEquals(Object.class, last.getType()); } @Test @@ -784,7 +781,7 @@ public class GenericConversionServiceTests { private static class MyConditionalGenericConverter implements GenericConverter, ConditionalConverter { - private Set sourceTypes = new LinkedHashSet(); + private List sourceTypes = new ArrayList(); public Set getConvertibleTypes() { return null; @@ -800,7 +797,7 @@ public class GenericConversionServiceTests { return null; } - public Set getSourceTypes() { + public List getSourceTypes() { return sourceTypes; } }