implemented collection/map converter conditional matching checks; updated SpEL to reflect this behavior

Keith Donald · Keith Donald · commit 1e39b0bbbc37 · 2011-06-07T20:00:28.000Z
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java
@@ -45,7 +45,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		return this.helperConverter.matches(sourceType, targetType);
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {		
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToCollectionConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToCollectionConverter.java
@@ -49,7 +49,23 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (targetType.getElementTypeDescriptor() == null) {
+			// yes
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType.getElementTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getElementTypeDescriptor().getType().isAssignableFrom(targetType.getElementTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	@SuppressWarnings("unchecked")
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToObjectConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToObjectConverter.java
@@ -45,7 +45,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		return this.helperConverter.matches(sourceType, targetType);
 	}
 	
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToStringConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ArrayToStringConverter.java
@@ -45,7 +45,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		return this.helperConverter.matches(sourceType, targetType);
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToArrayConverter.java
@@ -49,7 +49,23 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (sourceType.getElementTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType.getElementTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getElementTypeDescriptor().getType().isAssignableFrom(targetType.getElementTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
@@ -66,4 +82,4 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 		return array;
 	}
 
-}
+}
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java
@@ -49,7 +49,27 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (targetType.getElementTypeDescriptor() == null) {
+			// yes
+			return true;
+		}
+		if (sourceType.getElementTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType.getElementTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getElementTypeDescriptor().getType().isAssignableFrom(targetType.getElementTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	@SuppressWarnings("unchecked")
@@ -72,4 +92,4 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 		return target;
 	}
 
-}
+}
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToObjectConverter.java
@@ -43,7 +43,23 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (sourceType.getElementTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType);
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getElementTypeDescriptor().getType().isAssignableFrom(targetType.getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java
@@ -45,7 +45,23 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (sourceType.getElementTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getElementTypeDescriptor(), targetType);
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getElementTypeDescriptor().getType().isAssignableFrom(targetType.getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
@@ -69,4 +85,4 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 		return sb.toString();
 	}
 
-}
+}
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java
@@ -49,7 +49,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		return canConvertKey(sourceType, targetType) && canConvertValue(sourceType, targetType);
 	}
 	
 	@SuppressWarnings("unchecked")
@@ -70,6 +70,54 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 	}
 	
 	// internal helpers
+
+	private boolean canConvertKey(TypeDescriptor sourceType, TypeDescriptor targetType) {
+		if (targetType.getMapKeyTypeDescriptor() == null) {
+			// yes
+			return true;
+		}
+		if (sourceType.getMapKeyTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getMapKeyTypeDescriptor(), targetType.getMapKeyTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getMapKeyTypeDescriptor().getType().isAssignableFrom(targetType.getMapKeyTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
+	}
+	
+	private boolean canConvertValue(TypeDescriptor sourceType, TypeDescriptor targetType) {
+		if (targetType.getMapValueTypeDescriptor() == null) {
+			// yes
+			return true;
+		}
+		if (sourceType.getMapValueTypeDescriptor() == null) {
+			// maybe
+			return true;
+		}
+		boolean canConvert = conversionService.canConvert(sourceType.getMapValueTypeDescriptor(), targetType.getMapValueTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getMapValueTypeDescriptor().getType().isAssignableFrom(targetType.getMapValueTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
+	}
 	
 	private Object convertKey(Object sourceKey, TypeDescriptor sourceType, TypeDescriptor targetType) {
 		if (targetType == null) {
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToArrayConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToArrayConverter.java
@@ -44,7 +44,19 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		boolean canConvert = conversionService.canConvert(sourceType, targetType.getElementTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getType().isAssignableFrom(targetType.getElementTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToCollectionConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToCollectionConverter.java
@@ -46,7 +46,23 @@ public Set<ConvertiblePair> getConvertibleTypes() {
 	}
 
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		return true;
+		if (targetType.getElementTypeDescriptor() == null) {
+			// yes
+			return true;
+		}		
+		boolean canConvert = conversionService.canConvert(sourceType, targetType.getElementTypeDescriptor());
+		if (canConvert) {
+			// yes
+			return true;
+		} else {
+			if (sourceType.getType().isAssignableFrom(targetType.getElementTypeDescriptor().getType())) {
+				// maybe;
+				return true;
+			} else {
+				// no;
+				return false;
+			}
+		}
 	}
 
 	@SuppressWarnings("unchecked")
@@ -64,4 +80,4 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 		return target;
 	}
 
-}
+}
diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java
@@ -175,16 +175,16 @@ public void allNulls() throws Exception {
 		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
 	}
 
-	@Test
-	public void allNullsNotConvertible() throws Exception {
+	@Test(expected=ConverterNotFoundException.class)
+	public void elementTypesNotConvertible() throws Exception {
 		List<Resource> resources = new ArrayList<Resource>();
 		resources.add(null);
 		resources.add(null);
-		TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("allNullsNotConvertible")); 
+		TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("strings")); 
 		assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
 	}
 	
-	public List<String> allNullsNotConvertible;
+	public List<String> strings;
 
 	@Test(expected=ConversionFailedException.class)
 	public void nothingInCommon() throws Exception {
diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java
@@ -225,11 +225,11 @@ public void convertObjectToPrimitiveViaConverterFactory() {
 	@Test
 	public void genericConverterDelegatingBackToConversionServiceConverterNotFound() {
 		conversionService.addConverter(new ObjectToArrayConverter(conversionService));
-		assertTrue(conversionService.canConvert(String.class, Integer[].class));
+		assertFalse(conversionService.canConvert(String.class, Integer[].class));
 		try {
 			conversionService.convert("3,4,5", Integer[].class);
-		} catch (ConversionFailedException e) {
-			assertTrue(e.getCause() instanceof ConverterNotFoundException);
+			fail("should have failed");
+		} catch (ConverterNotFoundException e) {
 		}
 	}
 
diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java
@@ -3,6 +3,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.Arrays;
 import java.util.HashMap;
@@ -114,12 +115,13 @@ public void collectionMapSourceTarget() throws Exception {
 		map.put("2", Arrays.asList("37", "23"));
 		TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("sourceCollectionMapTarget"));
 		TypeDescriptor targetType = new TypeDescriptor(getClass().getField("collectionMapTarget"));
-		assertTrue(conversionService.canConvert(sourceType, targetType));
+		assertFalse(conversionService.canConvert(sourceType, targetType));
 		try {
 			conversionService.convert(map, sourceType, targetType);
-		} catch (ConversionFailedException e) {
-			assertTrue(e.getCause() instanceof ConverterNotFoundException);
-		}				
+			fail("Should have failed");
+		} catch (ConverterNotFoundException e) {
+			
+		}
 		conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
 		conversionService.addConverterFactory(new StringToNumberConverterFactory());
 		assertTrue(conversionService.canConvert(sourceType, targetType));
diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java
@@ -17,6 +17,7 @@
 package org.springframework.expression.spel.ast;
 
 import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -123,23 +124,20 @@ private TypedValue createNewInstance(ExpressionState state) throws EvaluationExc
 				// In the first case we should not retry, in the second case we should see if there is a
 				// better suited method.
 
-				// To determine which situation it is, the AccessException will contain a cause - this
-				// will be the exception thrown by the reflective invocation. Inside this exception there
-				// may or may not be a root cause. If there is a root cause it is a user created exception.
-				// If there is no root cause it was a reflective invocation problem.
-
-				Throwable causeOfAccessException = ae.getCause();
-				Throwable rootCause = (causeOfAccessException == null ? null : causeOfAccessException.getCause());
-				if (rootCause != null) {
+				// To determine which situation it is, the AccessException will contain a cause.
+				// If the cause is an InvocationTargetException, a user exception was thrown inside the constructor.
+				// Otherwise the constructor could not be invoked.
+				if (ae.getCause() instanceof InvocationTargetException) {
 					// User exception was the root cause - exit now
+					Throwable rootCause = ae.getCause().getCause();
 					if (rootCause instanceof RuntimeException) {
 						throw (RuntimeException) rootCause;
 					} else {
 						String typename = (String) this.children[0].getValueInternal(state).getValue();
 						throw new SpelEvaluationException(getStartPosition(), rootCause,
 								SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM, typename, FormatHelper
 										.formatMethodForMessage("", argumentTypes));
-					}
+					}					
 				}
 
 				// at this point we know it wasn't a user problem so worth a retry if a better candidate can be found
diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java
diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java
diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorExecutor.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorExecutor.java
diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java
diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java
diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java
diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Fruit.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Fruit.java

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {`
`48`		`- return true;`
	`48`	`+ return this.helperConverter.matches(sourceType, targetType);`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {`