fixed detection of element type in case of nested collections (SPR-7569)

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

@@ -76,6 +76,8 @@ public class TypeDescriptor {
 
 	private Field field;
 
+	private int fieldNestingLevel = 1;
+
 	private Object value;
 
 	private TypeDescriptor elementType;
@@ -133,6 +135,19 @@ public class TypeDescriptor {
 		this.type = type;
 	}
 
+	/**
+	 * Create a new type descriptor for a field.
+	 * Use this constructor when a target conversion point originates from a field.
+	 * @param field the field to wrap
+	 * @param type the specific type to expose (may be an array/collection element)
+	 */
+	private TypeDescriptor(Field field, int nestingLevel, Class<?> type) {
+		Assert.notNull(field, "Field must not be null");
+		this.field = field;
+		this.fieldNestingLevel = nestingLevel;
+		this.type = type;
+	}
+
 	/**
 	 * Internal constructor for a NULL descriptor.
 	 */
@@ -397,10 +412,12 @@ public class TypeDescriptor {
 			return TypeDescriptor.UNKNOWN;
 		}
 		else if (this.methodParameter != null) {
-			return new TypeDescriptor(this.methodParameter, elementType);
+			MethodParameter nested = new MethodParameter(this.methodParameter);
+			nested.increaseNestingLevel();
+			return new TypeDescriptor(nested, elementType);
 		}
 		else if (this.field != null) {
-			return new TypeDescriptor(this.field, elementType);
+			return new TypeDescriptor(this.field, this.fieldNestingLevel + 1, elementType);
 		}
 		else {
 			return TypeDescriptor.valueOf(elementType);
@@ -434,7 +451,7 @@ public class TypeDescriptor {
 	}
 
 	/**
-	 * A textual representation of the type descriptor (eg. Map<String,Foo>) for use in messages
+	 * A textual representation of the type descriptor (eg. Map<String,Foo>) for use in messages.
 	 */
 	public String asString() {
 		return toString();
@@ -442,28 +459,22 @@ public class TypeDescriptor {
 
 	public String toString() {
 		if (this == TypeDescriptor.NULL) {
-			return "[TypeDescriptor.NULL]";
+			return "null";
 		}
 		else {
 			StringBuilder builder = new StringBuilder();
-			builder.append("[TypeDescriptor ");
 			Annotation[] anns = getAnnotations();
 			for (Annotation ann : anns) {
 				builder.append("@").append(ann.annotationType().getName()).append(' ');
 			}
 			builder.append(ClassUtils.getQualifiedName(getType()));
 			if (isMap()) {
-				Class<?> mapKeyType = getMapKeyType();
-				Class<?> valueKeyType = getMapValueType();
-				builder.append("<").append(mapKeyType != null ? ClassUtils.getQualifiedName(mapKeyType) : "?");
-				builder.append(", ").append(valueKeyType != null ? ClassUtils.getQualifiedName(valueKeyType) : "?");
-				builder.append(">");
+				builder.append("<").append(getMapKeyTypeDescriptor());
+				builder.append(", ").append(getMapValueTypeDescriptor()).append(">");
 			}
 			else if (isCollection()) {
-				Class<?> elementType = getElementType();
-				builder.append("<").append(elementType != null ? ClassUtils.getQualifiedName(elementType) : "?").append(">");
+				builder.append("<").append(getElementTypeDescriptor()).append(">");
 			}
-			builder.append("]");
 			return builder.toString();
 		}
 	}
@@ -486,7 +497,7 @@ public class TypeDescriptor {
 	@SuppressWarnings("unchecked")
 	private Class<?> resolveCollectionElementType() {
 		if (this.field != null) {
-			return GenericCollectionTypeResolver.getCollectionFieldType(this.field);
+			return GenericCollectionTypeResolver.getCollectionFieldType(this.field, this.fieldNestingLevel);
 		}
 		else if (this.methodParameter != null) {
 			return GenericCollectionTypeResolver.getCollectionParameterType(this.methodParameter);
@@ -497,7 +508,10 @@ public class TypeDescriptor {
 				return elementType;
 			}
 		}
-        return (this.type != null ? GenericCollectionTypeResolver.getCollectionType((Class<? extends Collection>) this.type) : null);
+        else if (this.type != null) {
+			return GenericCollectionTypeResolver.getCollectionType((Class<? extends Collection>) this.type);
+		}
+		return null;
 	}
 
 	@SuppressWarnings("unchecked")
@@ -514,7 +528,10 @@ public class TypeDescriptor {
 				return keyType;
 			}
 		}
-		return (this.type != null && isMap() ? GenericCollectionTypeResolver.getMapKeyType((Class<? extends Map>) this.type) : null);
+		else if (this.type != null && isMap()) {
+			return GenericCollectionTypeResolver.getMapKeyType((Class<? extends Map>) this.type);
+		}
+		return null;
 	}
 
 	@SuppressWarnings("unchecked")
@@ -531,7 +548,10 @@ public class TypeDescriptor {
 				return valueType;
 			}
 		}
-		return (isMap() && this.type != null ? GenericCollectionTypeResolver.getMapValueType((Class<? extends Map>) this.type) : null);
+		else if (this.type != null && isMap()) {
+			return GenericCollectionTypeResolver.getMapValueType((Class<? extends Map>) this.type);
+		}
+		return null;
 	}
 
 	private Annotation[] resolveAnnotations() {

org.springframework.core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java

@@ -16,16 +16,16 @@
 
 package org.springframework.core.convert;
 
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertTrue;
-import static org.junit.Assert.assertFalse;
-
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
 import org.junit.Test;
 
 /**
@@ -33,44 +33,75 @@ import org.junit.Test;
  */
 public class TypeDescriptorTests {
 
-	List<String> listOfString;
-	int[] intArray;
-	List<String>[] arrayOfListOfString;
+	public List<String> listOfString;
+
+	public List<List<String>> listOfListOfString = new ArrayList<List<String>>();
+
+	public List<List> listOfListOfUnknown = new ArrayList<List>();
+
+	public int[] intArray;
+
+	public List<String>[] arrayOfListOfString;
+
+	public List<Integer> listField = new ArrayList<Integer>();
+
+	public Map<String, Integer> mapField = new HashMap<String, Integer>();
+
 
 	@Test
-	public void listDescriptors() throws Exception {
+	public void listDescriptor() throws Exception {
 		TypeDescriptor typeDescriptor = new TypeDescriptor(TypeDescriptorTests.class.getDeclaredField("listOfString"));
 		assertFalse(typeDescriptor.isArray());
-		assertEquals(List.class,typeDescriptor.getType());
-		assertEquals(String.class,typeDescriptor.getElementType());
+		assertEquals(List.class, typeDescriptor.getType());
+		assertEquals(String.class, typeDescriptor.getElementType());
 		// TODO caught shorten these names but it is OK that they are fully qualified for now
-		assertEquals("[TypeDescriptor java.util.List<java.lang.String>]",typeDescriptor.asString());
+		assertEquals("java.util.List<java.lang.String>", typeDescriptor.asString());
+	}
+
+	@Test
+	public void listOfListOfStringDescriptor() throws Exception {
+		TypeDescriptor typeDescriptor = new TypeDescriptor(TypeDescriptorTests.class.getDeclaredField("listOfListOfString"));
+		assertFalse(typeDescriptor.isArray());
+		assertEquals(List.class, typeDescriptor.getType());
+		assertEquals(List.class, typeDescriptor.getElementType());
+		assertEquals(String.class, typeDescriptor.getElementTypeDescriptor().getElementType());
+		assertEquals("java.util.List<java.util.List<java.lang.String>>", typeDescriptor.asString());
 	}
-	
+
 	@Test
-	public void arrayTypeDescriptors() throws Exception {
+	public void listOfListOfUnknownDescriptor() throws Exception {
+		TypeDescriptor typeDescriptor = new TypeDescriptor(TypeDescriptorTests.class.getDeclaredField("listOfListOfUnknown"));
+		assertFalse(typeDescriptor.isArray());
+		assertEquals(List.class, typeDescriptor.getType());
+		assertEquals(List.class, typeDescriptor.getElementType());
+		assertEquals(Object.class, typeDescriptor.getElementTypeDescriptor().getElementType());
+		assertEquals("java.util.List<java.util.List<java.lang.Object>>", typeDescriptor.asString());
+	}
+
+	@Test
+	public void arrayTypeDescriptor() throws Exception {
 		TypeDescriptor typeDescriptor = new TypeDescriptor(TypeDescriptorTests.class.getDeclaredField("intArray"));
 		assertTrue(typeDescriptor.isArray());
 		assertEquals(Integer.TYPE,typeDescriptor.getElementType());
-		assertEquals("[TypeDescriptor int[]]",typeDescriptor.asString());
+		assertEquals("int[]",typeDescriptor.asString());
 	}
 
 	@Test
-	public void buildingArrayTypeDescriptors() throws Exception {
+	public void buildingArrayTypeDescriptor() throws Exception {
 		TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(int[].class);
 		assertTrue(typeDescriptor.isArray());
-		assertEquals(Integer.TYPE,typeDescriptor.getElementType());
+		assertEquals(Integer.TYPE ,typeDescriptor.getElementType());
 	}
-	
+
 	@Test
-	public void complexTypeDescriptors() throws Exception {
+	public void complexTypeDescriptor() throws Exception {
 		TypeDescriptor typeDescriptor = new TypeDescriptor(TypeDescriptorTests.class.getDeclaredField("arrayOfListOfString"));
 		assertTrue(typeDescriptor.isArray());
 		assertEquals(List.class,typeDescriptor.getElementType());
 		// TODO asc notice that the type of the list elements is lost: typeDescriptor.getElementType() should return a TypeDescriptor
-		assertEquals("[TypeDescriptor java.util.List[]]",typeDescriptor.asString());
+		assertEquals("java.util.List[]",typeDescriptor.asString());
 	}
-	
+
 	@Test
 	public void testEquals() throws Exception {
 		TypeDescriptor t1 = TypeDescriptor.valueOf(String.class);
@@ -94,9 +125,5 @@ public class TypeDescriptorTests {
 		TypeDescriptor t12 = new TypeDescriptor(getClass().getField("mapField"));
 		assertEquals(t11, t12);
 	}
-	
-	public List<Integer> listField = new ArrayList<Integer>();
-	
-	public Map<String, Integer> mapField = new HashMap<String, Integer>();
 
 }