root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Fix GenericConversionService search algorithm 

Previously the algorithm used by GenericConversionService to find
converters incorrectly searched for interfaces working up from the
base class. This caused particular problems with custom List
converters as as the Collection interface would be considered before
the List interface giving CollectionToObjectConverter precedence
over the custom converter.

The updated algorithm restores the class search order to behave in the
same way as Spring 3.1.

Issue: SPR-10116
Backport-Issue: SPR-10117
Backport-Commit: aa914497dc
This commit is contained in:
Phillip Webb 2012-12-22 10:24:33 -08:00
parent a30ee0164a
commit 1abb7f66a7
3 changed files with 71 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java
View File

@ -431,14 +431,6 @@ public class GenericConversionService implements ConfigurableConversionService {
*/ */
private static class Converters { private static class Converters {
private static final Set<Class<?>> IGNORED_CLASSES;
static {
Set<Class<?>> ignored = new HashSet<Class<?>>();
ignored.add(Object.class);
ignored.add(Object[].class);
IGNORED_CLASSES = Collections.unmodifiableSet(ignored);
}
private final Set<GenericConverter> globalConverters = private final Set<GenericConverter> globalConverters =
new LinkedHashSet<GenericConverter>(); new LinkedHashSet<GenericConverter>();
@ -483,12 +475,13 @@ public class GenericConversionService implements ConfigurableConversionService {
*/ */
public GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) { public GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) {
// Search the full type hierarchy // Search the full type hierarchy
List<TypeDescriptor> sourceCandidates = getTypeHierarchy(sourceType); List<Class<?>> sourceCandidates = getClassHierarchy(sourceType.getType());
List<TypeDescriptor> targetCandidates = getTypeHierarchy(targetType); List<Class<?>> targetCandidates = getClassHierarchy(targetType.getType());
for (TypeDescriptor sourceCandidate : sourceCandidates) { for (Class<?> sourceCandidate : sourceCandidates) {
for (TypeDescriptor targetCandidate : targetCandidates) { for (Class<?> targetCandidate : targetCandidates) {
ConvertiblePair convertiblePair = new ConvertiblePair(sourceCandidate, targetCandidate);
GenericConverter converter = getRegisteredConverter( GenericConverter converter = getRegisteredConverter(
sourceType, targetType, sourceCandidate, targetCandidate); sourceType, targetType, convertiblePair);
if(converter != null) { if(converter != null) {
return converter; return converter;
} }
@ -497,12 +490,11 @@ public class GenericConversionService implements ConfigurableConversionService {
return null; return null;
} }
private GenericConverter getRegisteredConverter(TypeDescriptor sourceType, TypeDescriptor targetType, private GenericConverter getRegisteredConverter(TypeDescriptor sourceType,
TypeDescriptor sourceCandidate, TypeDescriptor targetCandidate) { TypeDescriptor targetType, ConvertiblePair convertiblePair) {
// Check specifically registered converters // Check specifically registered converters
ConvertersForPair convertersForPair = converters.get(new ConvertiblePair( ConvertersForPair convertersForPair = converters.get(convertiblePair);
sourceCandidate.getType(), targetCandidate.getType()));
GenericConverter converter = convertersForPair == null ? null GenericConverter converter = convertersForPair == null ? null
: convertersForPair.getConverter(sourceType, targetType); : convertersForPair.getConverter(sourceType, targetType);
if (converter != null) { if (converter != null) {
@ -512,7 +504,7 @@ public class GenericConversionService implements ConfigurableConversionService {
// Check ConditionalGenericConverter that match all types // Check ConditionalGenericConverter that match all types
for (GenericConverter globalConverter : this.globalConverters) { for (GenericConverter globalConverter : this.globalConverters) {
if (((ConditionalConverter)globalConverter).matches( if (((ConditionalConverter)globalConverter).matches(
sourceCandidate, targetCandidate)) { sourceType, targetType)) {
return globalConverter; return globalConverter;
} }
} }
@ -526,45 +518,39 @@ public class GenericConversionService implements ConfigurableConversionService {
* @return an ordered list of all classes that the given type extends or * @return an ordered list of all classes that the given type extends or
* implements. * implements.
*/ */
private List<TypeDescriptor> getTypeHierarchy(TypeDescriptor type) { private List<Class<?>> getClassHierarchy(Class<?> type) {
if(type.isPrimitive()) { List<Class<?>> hierarchy = new ArrayList<Class<?>>(20);
type = TypeDescriptor.valueOf(type.getObjectType()); Set<Class<?>> visited = new HashSet<Class<?>>(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);
} }
Set<TypeDescriptor> typeHierarchy = new LinkedHashSet<TypeDescriptor>(); for (Class<?> implementedInterface : candidate.getInterfaces()) {
collectTypeHierarchy(typeHierarchy, type); addToClassHierarchy(hierarchy.size(), implementedInterface, array, hierarchy, visited);
if(type.isArray()) {
typeHierarchy.add(TypeDescriptor.valueOf(Object[].class));
} }
typeHierarchy.add(TypeDescriptor.valueOf(Object.class)); i++;
return new ArrayList<TypeDescriptor>(typeHierarchy); }
addToClassHierarchy(hierarchy.size(), Object.class, array, hierarchy, visited);
addToClassHierarchy(hierarchy.size(), Object.class, false, hierarchy, visited);
return hierarchy;
} }
private void collectTypeHierarchy(Set<TypeDescriptor> typeHierarchy, private void addToClassHierarchy(int index, Class<?> type, boolean asArray,
TypeDescriptor type) { List<Class<?>> hierarchy, Set<Class<?>> visited) {
if(type != null && !IGNORED_CLASSES.contains(type.getType())) { if(asArray) {
if(typeHierarchy.add(type)) { type = Array.newInstance(type, 0).getClass();
Class<?> superclass = type.getType().getSuperclass();
if (type.isArray()) {
superclass = ClassUtils.resolvePrimitiveIfNecessary(superclass);
} }
collectTypeHierarchy(typeHierarchy, createRelated(type, superclass)); if(visited.add(type)) {
hierarchy.add(index, type);
for (Class<?> implementsInterface : type.getType().getInterfaces()) {
collectTypeHierarchy(typeHierarchy, createRelated(type, implementsInterface));
} }
} }
}
}
private TypeDescriptor createRelated(TypeDescriptor type, Class<?> relatedType) {
if (relatedType == null && type.isArray()) {
relatedType = Array.newInstance(relatedType, 0).getClass();
}
if(!type.getType().equals(relatedType)) {
return type.upcast(relatedType);
}
return null;
}
@Override @Override
public String toString() { public String toString() {

34
spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java
View File

@ -16,11 +16,7 @@
package org.springframework.core.convert.support; package org.springframework.core.convert.support;
import static junit.framework.Assert.assertEquals; import static org.junit.Assert.*;
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 java.awt.Color; import java.awt.Color;
import java.math.BigDecimal; import java.math.BigDecimal;
@ -506,6 +502,21 @@ public class DefaultConversionTests {
assertEquals(source, result); assertEquals(source, result);
} }
@Test
public void convertCollectionToObjectWithCustomConverter() throws Exception {
List<String> source = new ArrayList<String>();
source.add("A");
source.add("B");
conversionService.addConverter(new Converter<List, ListWrapper>() {
@Override
public ListWrapper convert(List source) {
return new ListWrapper(source);
}
});
ListWrapper result = conversionService.convert(source, ListWrapper.class);
assertSame(source, result.getList());
}
@Test @Test
public void convertObjectToCollection() { public void convertObjectToCollection() {
List<String> result = (List<String>) conversionService.convert(3L, List.class); List<String> result = (List<String>) 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;
}
}
} }

13
spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java
View File

@ -27,7 +27,6 @@ import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@ -48,6 +47,7 @@ import org.springframework.core.io.Resource;
import org.springframework.util.StopWatch; import org.springframework.util.StopWatch;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.*; import static org.junit.Assert.*;
/** /**
@ -696,14 +696,11 @@ public class GenericConversionServiceTests {
MyConditionalGenericConverter converter = new MyConditionalGenericConverter(); MyConditionalGenericConverter converter = new MyConditionalGenericConverter();
conversionService.addConverter(converter); conversionService.addConverter(converter);
assertEquals((Integer) 3, conversionService.convert(3, Integer.class)); assertEquals((Integer) 3, conversionService.convert(3, Integer.class));
assertThat(converter.getSourceTypes().size(), greaterThan(2));
Iterator<TypeDescriptor> iterator = converter.getSourceTypes().iterator(); Iterator<TypeDescriptor> iterator = converter.getSourceTypes().iterator();
while(iterator.hasNext()) {
assertEquals(Integer.class, iterator.next().getType()); assertEquals(Integer.class, iterator.next().getType());
assertEquals(Number.class, iterator.next().getType());
TypeDescriptor last = null;
while (iterator.hasNext()) {
last = iterator.next();
} }
assertEquals(Object.class, last.getType());
} }
@Test @Test
@ -784,7 +781,7 @@ public class GenericConversionServiceTests {
private static class MyConditionalGenericConverter implements GenericConverter, private static class MyConditionalGenericConverter implements GenericConverter,
ConditionalConverter { ConditionalConverter {
private Set<TypeDescriptor> sourceTypes = new LinkedHashSet<TypeDescriptor>(); private List<TypeDescriptor> sourceTypes = new ArrayList<TypeDescriptor>();
public Set<ConvertiblePair> getConvertibleTypes() { public Set<ConvertiblePair> getConvertibleTypes() {
return null; return null;
@ -800,7 +797,7 @@ public class GenericConversionServiceTests {
return null; return null;
} }
public Set<TypeDescriptor> getSourceTypes() { public List<TypeDescriptor> getSourceTypes() {
return sourceTypes; return sourceTypes;
} }
} }