ConversionService is able to deal with empty collections and nested collections (fixed regression; SPR-7289, SPR-7293); ConversionService properly handles nested Resource arrays in Map values (fixed regression; SPR-7295); ConversionService does not accidentally use copy constructor for same type (SPR-7304)

Author: jhoeller

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

@@ -39,9 +39,12 @@
  */
 public class TypeDescriptor {
 
-	/** Constant defining an 'unknown type' TypeDescriptor */
+	/** Constant defining a TypeDescriptor for a <code>null</code> value */
 	public static final TypeDescriptor NULL = new TypeDescriptor();
 
+	/** Constant defining a TypeDescriptor for 'unknown type' */
+	public static final TypeDescriptor UNKNOWN = new TypeDescriptor(Object.class);
+
 	private static final Map<Class<?>, TypeDescriptor> typeDescriptorCache = new HashMap<Class<?>, TypeDescriptor>();
 
 	private static final Annotation[] EMPTY_ANNOTATION_ARRAY = new Annotation[0];
@@ -140,7 +143,7 @@ private TypeDescriptor() {
 	 * Create a new descriptor for the type of the given value.
 	 * <p>Use this constructor when a conversion point comes from a source such as a Map or
 	 * Collection, where no additional context is available but elements can be introspected.
-	 * @param type the actual type to wrap
+	 * @param value the value to determine the actual type from
 	 */
 	private TypeDescriptor(Object value) {
 		Assert.notNull(value, "Value must not be null");
@@ -263,7 +266,7 @@ public synchronized TypeDescriptor getElementTypeDescriptor() {
 	 */
 	public TypeDescriptor getElementTypeDescriptor(Object element) {
 		TypeDescriptor elementType = getElementTypeDescriptor();
-		return (elementType != TypeDescriptor.NULL ? elementType : forObject(element));
+		return (elementType != TypeDescriptor.UNKNOWN ? elementType : forObject(element));
 	}
 
 	/**
@@ -306,7 +309,7 @@ public synchronized TypeDescriptor getMapKeyTypeDescriptor() {
 	 */
 	public TypeDescriptor getMapKeyTypeDescriptor(Object key) {
 		TypeDescriptor keyType = getMapKeyTypeDescriptor();
-		return keyType != TypeDescriptor.NULL ? keyType : TypeDescriptor.forObject(key);
+		return (keyType != TypeDescriptor.UNKNOWN ? keyType : TypeDescriptor.forObject(key));
 	}
 
 	/**
@@ -335,7 +338,7 @@ public synchronized TypeDescriptor getMapValueTypeDescriptor() {
 	 */
 	public TypeDescriptor getMapValueTypeDescriptor(Object value) {
 		TypeDescriptor valueType = getMapValueTypeDescriptor();
-		return (valueType != TypeDescriptor.NULL ? valueType : TypeDescriptor.forObject(value));
+		return (valueType != TypeDescriptor.UNKNOWN ? valueType : TypeDescriptor.forObject(value));
 	}
 
 	/**
@@ -390,11 +393,8 @@ else if (isMap() && targetType.isMap()) {
 	 * @return the type descriptor
 	 */
 	public TypeDescriptor forElementType(Class<?> elementType) {
-		if (getType().equals(elementType)) {
-			return this;
-		}
-		else if (elementType == null) {
-			return TypeDescriptor.NULL;
+		if (elementType == null) {
+			return TypeDescriptor.UNKNOWN;
 		}
 		else if (this.methodParameter != null) {
 			return new TypeDescriptor(this.methodParameter, elementType);
@@ -408,29 +408,29 @@ else if (this.field != null) {
 	}
 
 	public boolean equals(Object obj) {
-		if (!(obj instanceof TypeDescriptor)) {
-			return false;
-		}
-		TypeDescriptor td = (TypeDescriptor) obj;
-		if (this == td) {
+		if (this == obj) {
 			return true;
 		}
+		if (!(obj instanceof TypeDescriptor) || obj == TypeDescriptor.NULL) {
+			return false;
+		}
+		TypeDescriptor other = (TypeDescriptor) obj;
 		boolean annotatedTypeEquals =
-				getType().equals(td.getType()) && ObjectUtils.nullSafeEquals(getAnnotations(), td.getAnnotations());
+				getType().equals(other.getType()) && ObjectUtils.nullSafeEquals(getAnnotations(), other.getAnnotations());
 		if (isCollection()) {
-			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getElementType(), td.getElementType());
+			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getElementType(), other.getElementType());
 		}
 		else if (isMap()) {
-			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getMapKeyType(), td.getMapKeyType()) &&
-					ObjectUtils.nullSafeEquals(getMapValueType(), td.getMapValueType());
+			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getMapKeyType(), other.getMapKeyType()) &&
+					ObjectUtils.nullSafeEquals(getMapValueType(), other.getMapValueType());
 		}
 		else {
 			return annotatedTypeEquals;
 		}
 	}
 
 	public int hashCode() {
-		return getType().hashCode();
+		return (this == TypeDescriptor.NULL ? 0 : getType().hashCode());
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -58,9 +58,14 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 			return null;
 		}
 		Collection<?> sourceCollection = (Collection<?>) source;
+		if (sourceCollection.isEmpty()) {
+			return sourceCollection;
+		}
 		Collection target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size());
 		for (Object sourceElement : sourceCollection) {
-			Object targetElement = this.conversionService.convert(sourceElement, sourceType.getElementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor(sourceElement));
+			Object targetElement = this.conversionService.convert(sourceElement,
+					sourceType.getElementTypeDescriptor(sourceElement),
+					targetType.getElementTypeDescriptor(sourceElement));
 			target.add(targetElement);
 		}
 		return target;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -62,7 +62,8 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 			if (i > 0) {
 				string.append(DELIMITER);
 			}
-			Object targetElement = this.conversionService.convert(sourceElement, sourceType.getElementTypeDescriptor(sourceElement), targetType);
+			Object targetElement = this.conversionService.convert(
+					sourceElement, sourceType.getElementTypeDescriptor(sourceElement), targetType);
 			string.append(targetElement);
 			i++;
 		}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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,7 +16,6 @@
 
 package org.springframework.core.convert.support;
 
-import java.util.Collection;
 import java.util.Map;
 
 import org.springframework.core.convert.ConversionFailedException;
@@ -41,15 +40,6 @@ public static Object invokeConverter(GenericConverter converter, Object source,
 		}
 	}
 
-	public static TypeDescriptor getElementType(Collection<?> collection) {
-		for (Object element : collection) {
-			if (element != null) {
-				return TypeDescriptor.valueOf(element.getClass());
-			}
-		}
-		return TypeDescriptor.NULL;
-	}
-	
 	public static TypeDescriptor[] getMapEntryTypes(Map<?, ?> sourceMap) {
 		Class<?> keyType = null;
 		Class<?> valueType = null;

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

@@ -244,7 +244,7 @@ protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescripto
 			if (logger.isTraceEnabled()) {
 				logger.trace("Matched cached converter " + converter);
 			}
-			return converter != NO_MATCH ? converter : null; 
+			return (converter != NO_MATCH ? converter : null);
 		}
 		else {
 			converter = findConverterForClassPair(sourceType, targetType);
@@ -394,6 +394,7 @@ private Map<Class<?>, MatchableConverters> getTargetConvertersForSource(Class<?>
 
 	private GenericConverter getMatchingConverterForTarget(TypeDescriptor sourceType, TypeDescriptor targetType,
 			Map<Class<?>, MatchableConverters> converters) {
+
 		Class<?> targetObjectType = targetType.getObjectType();
 		if (targetObjectType.isInterface()) {
 			LinkedList<Class<?>> classQueue = new LinkedList<Class<?>>();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -59,13 +59,20 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 			return null;
 		}
 		Map<?, ?> sourceMap = (Map<?, ?>) source;
+		if (sourceMap.isEmpty()) {
+			return sourceMap;
+		}
 		Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size());
 		for (Object entry : sourceMap.entrySet()) {
 			Map.Entry sourceMapEntry = (Map.Entry) entry;
 			Object sourceKey = sourceMapEntry.getKey();
 			Object sourceValue = sourceMapEntry.getValue();
-			Object targetKey = this.conversionService.convert(sourceKey, sourceType.getMapKeyTypeDescriptor(sourceKey), targetType.getMapKeyTypeDescriptor(sourceKey));
-			Object targetValue = this.conversionService.convert(sourceValue, sourceType.getMapValueTypeDescriptor(sourceValue), targetType.getMapValueTypeDescriptor(sourceValue));
+			Object targetKey = this.conversionService.convert(sourceKey,
+					sourceType.getMapKeyTypeDescriptor(sourceKey),
+					targetType.getMapKeyTypeDescriptor(sourceKey));
+			Object targetValue = this.conversionService.convert(sourceValue,
+					sourceType.getMapValueTypeDescriptor(sourceValue),
+					targetType.getMapValueTypeDescriptor(sourceValue));
 			targetMap.put(targetKey, targetValue);
 		}
 		return targetMap;

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

@@ -42,9 +42,11 @@
  * @since 3.0
  */
 final class ObjectToObjectConverter implements ConditionalGenericConverter {
-	
+
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return hasValueOfMethodOrConstructor(targetType.getObjectType(), sourceType.getObjectType());
+		Class source = sourceType.getObjectType();
+		Class target = targetType.getObjectType();
+		return (!source.equals(target) && hasValueOfMethodOrConstructor(target, source));
 	}
 
 	public Set<ConvertiblePair> getConvertibleTypes() {

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

@@ -17,6 +17,7 @@
 package org.springframework.core.convert.support;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -225,6 +226,57 @@ public void testWildcardMap() throws Exception {
 		assertEquals(input, converted);
 	}
 
+	@Test
+	public void testListOfList() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		List<List<String>> list = Collections.singletonList(Collections.singletonList("Foo"));
+		assertNotNull(service.convert(list, String.class));
+	}
+
+	@Test
+	public void testEmptyList() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		List list = Collections.emptyList();
+		List result = service.convert(list, List.class);
+		assertSame(list, result);
+		result = service.convert(list, list.getClass());
+		assertSame(list, result);
+	}
+
+	@Test
+	public void testEmptyMap() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		Map map = Collections.emptyMap();
+		Map result = service.convert(map, Map.class);
+		assertSame(map, result);
+		result = service.convert(map, map.getClass());
+		assertSame(map, result);
+	}
+
+	@Test
+	public void testStringToString() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		String value = "myValue";
+		String result = service.convert(value, String.class);
+		assertSame(value, result);
+	}
+
+	@Test
+	public void testStringToObject() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		String value = "myValue";
+		Object result = service.convert(value, Object.class);
+		assertSame(value, result);
+	}
+
+	@Test
+	public void testIgnoreCopyConstructor() {
+		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		WithCopyConstructor value = new WithCopyConstructor();
+		Object result = service.convert(value, WithCopyConstructor.class);
+		assertSame(value, result);
+	}
+
 	@Test
 	public void testPerformance1() {
 		GenericConversionService conversionService = ConversionServiceFactory.createDefaultConversionService();
@@ -296,6 +348,7 @@ public void testPerformance3() throws Exception {
 
 	public static Map<String, Integer> map;
 
+
 	private interface MyBaseInterface {
 
 	}
@@ -319,6 +372,16 @@ public String convert(MyBaseInterface source) {
 	}
 
 
+	public static class WithCopyConstructor {
+
+		public WithCopyConstructor() {
+		}
+
+		public WithCopyConstructor(WithCopyConstructor value) {
+		}
+	}
+
+
 	public static Map<String, ?> wildcardMap;
 
 }