GenericConversionService matches converters by full generic target type, allowing for the registration of multiple converters from the same source type to different collection types

Issue: SPR-11369

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

@@ -34,6 +34,7 @@ import org.springframework.core.SerializableTypeWrapper.FieldTypeProvider;
 import org.springframework.core.SerializableTypeWrapper.MethodParameterTypeProvider;
 import org.springframework.core.SerializableTypeWrapper.TypeProvider;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.ConcurrentReferenceHashMap;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
@@ -169,7 +170,7 @@ public final class ResolvableType implements Serializable {
 	 * type information or meta-data that alternative JVM languages may provide.
 	 */
 	public Object getSource() {
-		Object source = (this.typeProvider == null ? null : this.typeProvider.getSource());
+		Object source = (this.typeProvider != null ? this.typeProvider.getSource() : null);
 		return (source != null ? source : this.type);
 	}
 
@@ -244,7 +245,7 @@ public final class ResolvableType implements Serializable {
 
 		// We need an exact type match for generics
 		// List<CharSequence> is not assignable from List<String>
-		if (checkingGeneric ? !ourResolved.equals(typeResolved) : !ourResolved.isAssignableFrom(typeResolved)) {
+		if (checkingGeneric ? !ourResolved.equals(typeResolved) : !ClassUtils.isAssignable(ourResolved, typeResolved)) {
 			return false;
 		}
 

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

@@ -150,9 +150,9 @@ public class TypeDescriptor implements Serializable {
 	}
 
 	/**
-	 * Returns the underlying {@link ResolvableType}.
+	 * Return the underlying {@link ResolvableType}.
 	 */
-	protected ResolvableType getResolvableType() {
+	public ResolvableType getResolvableType() {
 		return this.resolvableType;
 	}
 

spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,11 +26,10 @@ import org.springframework.core.convert.TypeDescriptor;
  * <p>Often used to selectively match custom conversion logic based on the presence of a
  * field or class-level characteristic, such as an annotation or method. For example, when
  * converting from a String field to a Date field, an implementation might return
- *
  * {@code true} if the target field has also been annotated with {@code @DateTimeFormat}.
  *
- * <p>As another example, when converting from a String field to an {@code Account} field, an
+ * <p>As another example, when converting from a String field to an {@code Account} field,
- * implementation might return {@code true} if the target Account class defines a
+ * an implementation might return {@code true} if the target Account class defines a
  * {@code public static findAccount(String)} method.
  *
  * @author Phillip Webb
@@ -46,10 +45,10 @@ public interface ConditionalConverter {
 	/**
 	 * Should the conversion from {@code sourceType} to {@code targetType} currently under
 	 * consideration be selected?
-	 *
 	 * @param sourceType the type descriptor of the field we are converting from
 	 * @param targetType the type descriptor of the field we are converting to
 	 * @return true if conversion should be performed, false otherwise
 	 */
 	boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType);
+
 }

spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,11 @@
 
 package org.springframework.core.convert.converter;
 
+import java.util.Set;
+
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.util.Assert;
 
-import java.util.Set;
-
 /**
  * Generic converter interface for converting between two or more types.
  *
@@ -104,13 +104,17 @@ public interface GenericConverter {
 			}
 			ConvertiblePair other = (ConvertiblePair) obj;
 			return this.sourceType.equals(other.sourceType) && this.targetType.equals(other.targetType);
-
 		}
 
 		@Override
 		public int hashCode() {
 			return this.sourceType.hashCode() * 31 + this.targetType.hashCode();
 		}
+
+		@Override
+		public String toString() {
+			return this.sourceType.getName() + " -> " + this.targetType.getName();
+		}
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -81,16 +81,15 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 	@Override
 	public void addConverter(Converter<?, ?> converter) {
-		GenericConverter.ConvertiblePair typeInfo = getRequiredTypeInfo(converter, Converter.class);
+		ResolvableType[] typeInfo = getRequiredTypeInfo(converter, Converter.class);
 		Assert.notNull(typeInfo, "Unable to the determine sourceType <S> and targetType " +
 				"<T> which your Converter<S, T> converts between; declare these generic types.");
-		addConverter(new ConverterAdapter(typeInfo, converter));
+		addConverter(new ConverterAdapter(converter, typeInfo[0], typeInfo[1]));
 	}
 
 	@Override
 	public void addConverter(Class<?> sourceType, Class<?> targetType, Converter<?, ?> converter) {
-		GenericConverter.ConvertiblePair typeInfo = new GenericConverter.ConvertiblePair(sourceType, targetType);
+		addConverter(new ConverterAdapter(converter, ResolvableType.forClass(sourceType), ResolvableType.forClass(targetType)));
-		addConverter(new ConverterAdapter(typeInfo, converter));
 	}
 
 	@Override
@@ -101,13 +100,11 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 	@Override
 	public void addConverterFactory(ConverterFactory<?, ?> converterFactory) {
-		GenericConverter.ConvertiblePair typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class);
+		ResolvableType[] typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class);
-		if (typeInfo == null) {
+		Assert.notNull("Unable to the determine sourceType <S> and targetRangeType R which your " +
-			throw new IllegalArgumentException("Unable to the determine sourceType <S> and " +
+				"ConverterFactory<S, R> converts between; declare these generic types.");
-					"targetRangeType R which your ConverterFactory<S, R> converts between; " +
+		addConverter(new ConverterFactoryAdapter(converterFactory,
-					"declare these generic types.");
+				new ConvertiblePair(typeInfo[0].resolve(), typeInfo[1].resolve())));
-		}
-		addConverter(new ConverterFactoryAdapter(typeInfo, converterFactory));
 	}
 
 	@Override
@@ -120,15 +117,14 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 	@Override
 	public boolean canConvert(Class<?> sourceType, Class<?> targetType) {
-		Assert.notNull(targetType, "The targetType to convert to cannot be null");
+		Assert.notNull(targetType, "targetType to convert to cannot be null");
-		return canConvert(sourceType != null ?
+		return canConvert((sourceType != null ? TypeDescriptor.valueOf(sourceType) : null),
-				TypeDescriptor.valueOf(sourceType) : null,
 				TypeDescriptor.valueOf(targetType));
 	}
 
 	@Override
 	public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		Assert.notNull(targetType,"The targetType to convert to cannot be null");
+		Assert.notNull(targetType, "targetType to convert to cannot be null");
 		if (sourceType == null) {
 			return true;
 		}
@@ -266,7 +262,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 	// internal helpers
 
-	private GenericConverter.ConvertiblePair getRequiredTypeInfo(Object converter, Class<?> genericIfc) {
+	private ResolvableType[] getRequiredTypeInfo(Object converter, Class<?> genericIfc) {
 		ResolvableType resolvableType = ResolvableType.forClass(converter.getClass()).as(genericIfc);
 		ResolvableType[] generics = resolvableType.getGenerics();
 		if (generics.length < 2) {
@@ -277,7 +273,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 		if (sourceType == null || targetType == null) {
 			return null;
 		}
-		return new GenericConverter.ConvertiblePair(sourceType, targetType);
+		return generics;
 	}
 
 	private void invalidateCache() {
@@ -316,17 +312,18 @@ public class GenericConversionService implements ConfigurableConversionService {
 	@SuppressWarnings("unchecked")
 	private final class ConverterAdapter implements ConditionalGenericConverter {
 
-		private final ConvertiblePair typeInfo;
-
 		private final Converter<Object, Object> converter;
 
+		private final ConvertiblePair typeInfo;
 
-		public ConverterAdapter(ConvertiblePair typeInfo, Converter<?, ?> converter) {
+		private final ResolvableType targetType;
+
+		public ConverterAdapter(Converter<?, ?> converter, ResolvableType sourceType, ResolvableType targetType) {
 			this.converter = (Converter<Object, Object>) converter;
-			this.typeInfo = typeInfo;
+			this.typeInfo = new ConvertiblePair(sourceType.resolve(Object.class), targetType.resolve(Object.class));
+			this.targetType = targetType;
 		}
 
-
 		@Override
 		public Set<ConvertiblePair> getConvertibleTypes() {
 			return Collections.singleton(this.typeInfo);
@@ -334,8 +331,13 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 		@Override
 		public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-			if (!this.typeInfo.getTargetType().equals(targetType.getObjectType())) {
+			ResolvableType rt = targetType.getResolvableType();
-				return false;
+			if (!rt.isAssignableFrom(this.targetType)) {
+				// Generic type structure not fully assignable -> try lenient fallback if
+				// unresolvable generics remain, just requiring the raw type to match then
+				if (!rt.hasUnresolvableGenerics() || !this.typeInfo.getTargetType().equals(targetType.getObjectType())) {
+					return false;
+				}
 			}
 			if (this.converter instanceof ConditionalConverter) {
 				return ((ConditionalConverter) this.converter).matches(sourceType, targetType);
@@ -353,9 +355,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 		@Override
 		public String toString() {
-			return this.typeInfo.getSourceType().getName() + " -> " +
+			return this.typeInfo + " : " + this.converter.toString();
-					this.typeInfo.getTargetType().getName() + " : " +
-					this.converter.toString();
 		}
 	}
 
@@ -366,17 +366,15 @@ public class GenericConversionService implements ConfigurableConversionService {
 	@SuppressWarnings("unchecked")
 	private final class ConverterFactoryAdapter implements ConditionalGenericConverter {
 
-		private final ConvertiblePair typeInfo;
-
 		private final ConverterFactory<Object, Object> converterFactory;
 
+		private final ConvertiblePair typeInfo;
 
-		public ConverterFactoryAdapter(ConvertiblePair typeInfo, ConverterFactory<?, ?> converterFactory) {
+		public ConverterFactoryAdapter(ConverterFactory<?, ?> converterFactory, ConvertiblePair typeInfo) {
 			this.converterFactory = (ConverterFactory<Object, Object>) converterFactory;
 			this.typeInfo = typeInfo;
 		}
 
-
 		@Override
 		public Set<ConvertiblePair> getConvertibleTypes() {
 			return Collections.singleton(this.typeInfo);
@@ -407,9 +405,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 		@Override
 		public String toString() {
-			return this.typeInfo.getSourceType().getName() + " -> " +
+			return this.typeInfo + " : " + this.converterFactory.toString();
-					this.typeInfo.getTargetType().getName() + " : " +
-					this.converterFactory.toString();
 		}
 	}
 
@@ -423,13 +419,11 @@ public class GenericConversionService implements ConfigurableConversionService {
 
 		private final TypeDescriptor targetType;
 
-
 		public ConverterCacheKey(TypeDescriptor sourceType, TypeDescriptor targetType) {
 			this.sourceType = sourceType;
 			this.targetType = targetType;
 		}
 
-
 		@Override
 		public boolean equals(Object other) {
 			if (this == other) {
@@ -439,20 +433,20 @@ public class GenericConversionService implements ConfigurableConversionService {
 				return false;
 			}
 			ConverterCacheKey otherKey = (ConverterCacheKey) other;
-			return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType)
+			return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) &&
-				&& ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType);
+					ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType);
 		}
 
 		@Override
 		public int hashCode() {
-			return ObjectUtils.nullSafeHashCode(this.sourceType) * 29
+			return ObjectUtils.nullSafeHashCode(this.sourceType) * 29 +
-				+ ObjectUtils.nullSafeHashCode(this.targetType);
+					ObjectUtils.nullSafeHashCode(this.targetType);
 		}
 
 		@Override
 		public String toString() {
-			return "ConverterCacheKey [sourceType = " + this.sourceType
+			return "ConverterCacheKey [sourceType = " + this.sourceType +
-				+ ", targetType = " + this.targetType + "]";
+					", targetType = " + this.targetType + "]";
 		}
 	}
 
@@ -472,7 +466,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 			if (convertibleTypes == null) {
 				Assert.state(converter instanceof ConditionalConverter,
 						"Only conditional converters may return null convertible types");
-				globalConverters.add(converter);
+				this.globalConverters.add(converter);
 			}
 			else {
 				for (ConvertiblePair convertiblePair : convertibleTypes) {
@@ -496,9 +490,9 @@ public class GenericConversionService implements ConfigurableConversionService {
 		}
 
 		/**
-		 * Find a {@link GenericConverter} given a source and target type.  This method will
+		 * Find a {@link GenericConverter} given a source and target type.
-		 * attempt to match all possible converters by working though the class and interface
+		 * <p>This method will attempt to match all possible converters by working
-		 * hierarchy of the types.
+		 * through the class and interface hierarchy of the types.
 		 * @param sourceType the source type
 		 * @param targetType the target type
 		 * @return a {@link GenericConverter} or <tt>null</tt>
@@ -530,22 +524,19 @@ public class GenericConversionService implements ConfigurableConversionService {
 					return converter;
 				}
 			}
-
 			// Check ConditionalGenericConverter that match all types
 			for (GenericConverter globalConverter : this.globalConverters) {
 				if (((ConditionalConverter)globalConverter).matches(sourceType, targetType)) {
 					return globalConverter;
 				}
 			}
-
 			return null;
 		}
 
 		/**
 		 * Returns an ordered class hierarchy for the given type.
 		 * @param type the type
-		 * @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<Class<?>> getClassHierarchy(Class<?> type) {
 			List<Class<?>> hierarchy = new ArrayList<Class<?>>(20);
@@ -555,8 +546,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 			int i = 0;
 			while (i < hierarchy.size()) {
 				Class<?> candidate = hierarchy.get(i);
-				candidate = (array ? candidate.getComponentType()
+				candidate = (array ? candidate.getComponentType() : ClassUtils.resolvePrimitiveIfNecessary(candidate));
-						: ClassUtils.resolvePrimitiveIfNecessary(candidate));
 				Class<?> superclass = candidate.getSuperclass();
 				if (candidate.getSuperclass() != null && superclass != Object.class) {
 					addToClassHierarchy(i + 1, candidate.getSuperclass(), array, hierarchy, visited);
@@ -584,7 +574,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 		@Override
 		public String toString() {
 			StringBuilder builder = new StringBuilder();
-			builder.append("ConversionService converters = ").append("\n");
+			builder.append("ConversionService converters =").append("\n");
 			for (String converterString : getConverterStrings()) {
 				builder.append("\t");
 				builder.append(converterString);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -682,7 +683,7 @@ public class GenericConversionServiceTests {
 	}
 
 	@Test
-	public void shouldNotSuportNullConvertibleTypesFromNonConditionalGenericConverter() {
+	public void shouldNotSupportNullConvertibleTypesFromNonConditionalGenericConverter() {
 		GenericConversionService conversionService = new GenericConversionService();
 		GenericConverter converter = new GenericConverter() {
 			@Override
@@ -766,6 +767,41 @@ public class GenericConversionServiceTests {
 		conversionService.convert(source, sourceType, targetType);
 	}
 
+	@Test
+	public void multipleCollectionTypesFromSameSourceType() throws Exception {
+		conversionService.addConverter(new MyStringToStringCollectionConverter());
+		conversionService.addConverter(new MyStringToIntegerCollectionConverter());
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection"))));
+		assertEquals(Collections.singleton(4),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("integerCollection"))));
+		assertEquals(Collections.singleton(4),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection"))));
+		assertEquals(Collections.singleton(4),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection"))));
+		assertEquals(Collections.singleton(4),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection"))));
+	}
+
+	@Test
+	public void adaptedCollectionTypesFromSameSourceType() throws Exception {
+		conversionService.addConverter(new MyStringToStringCollectionConverter());
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection"))));
+		assertEquals(Collections.singleton("testX"),
+				conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection"))));
+	}
+
 
 	@ExampleAnnotation
 	public String annotatedString;
@@ -872,4 +908,28 @@ public class GenericConversionServiceTests {
 		}
 	}
 
+	public static class MyStringToStringCollectionConverter implements Converter<String, Collection<String>> {
+
+		@Override
+		public Collection<String> convert(String source) {
+			return Collections.singleton(source + "X");
+		}
+	}
+
+	public static class MyStringToIntegerCollectionConverter implements Converter<String, Collection<Integer>> {
+
+		@Override
+		public Collection<Integer> convert(String source) {
+			return Collections.singleton(source.length());
+		}
+	}
+
+	public Collection<String> stringCollection;
+
+	public Collection<Integer> integerCollection;
+
+	public Collection rawCollection;
+
+	public Collection<?> genericCollection;
+
 }