optimized debug logging in case of non-convertible collection (SPR-8499)

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4663 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java

@@ -20,6 +20,7 @@ import java.beans.PropertyEditor;
 import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.Map;
@@ -461,10 +462,20 @@ class TypeConverterDelegate {
 	protected Collection convertToTypedCollection(
 			Collection original, String propertyName, Class requiredType, TypeDescriptor typeDescriptor) {
 
-		boolean originalAllowed = requiredType.isInstance(original);
-		if (!originalAllowed && !Collection.class.isAssignableFrom(requiredType)) {
+		if (!Collection.class.isAssignableFrom(requiredType)) {
 			return original;
 		}
+
+		boolean approximable = CollectionFactory.isApproximableCollectionType(requiredType);
+		if (!approximable && !canCreateCopy(requiredType)) {
+			if (logger.isDebugEnabled()) {
+				logger.debug("Custom Collection type [" + original.getClass().getName() +
+						"] does not allow for creating a copy - injecting original Collection as-is");
+			}
+			return original;
+		}
+
+		boolean originalAllowed = requiredType.isInstance(original);
 		typeDescriptor = typeDescriptor.narrow(original);
 		TypeDescriptor elementType = typeDescriptor.getElementTypeDescriptor();
 		if (elementType == null && originalAllowed &&
@@ -486,14 +497,14 @@ class TypeConverterDelegate {
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Cannot access Collection of type [" + original.getClass().getName() +
-						"] - injecting original Collection as-is", ex);
+						"] - injecting original Collection as-is: " + ex);
 			}
 			return original;
 		}
 
 		Collection convertedCopy;
 		try {
-			if (CollectionFactory.isApproximableCollectionType(requiredType)) {
+			if (approximable) {
 				convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
 			}
 			else {
@@ -503,7 +514,7 @@ class TypeConverterDelegate {
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Cannot create copy of Collection type [" + original.getClass().getName() +
-						"] - injecting original Collection as-is", ex);
+						"] - injecting original Collection as-is: " + ex);
 			}
 			return original;
 		}
@@ -512,15 +523,15 @@ class TypeConverterDelegate {
 		for (; it.hasNext(); i++) {
 			Object element = it.next();
 			String indexedPropertyName = buildIndexedPropertyName(propertyName, i);
-			Object convertedElement = convertIfNecessary(
-					indexedPropertyName, null, element, elementType != null ? elementType.getType() : null , typeDescriptor.getElementTypeDescriptor());
+			Object convertedElement = convertIfNecessary(indexedPropertyName, null, element,
+					(elementType != null ? elementType.getType() : null) , typeDescriptor.getElementTypeDescriptor());
 			try {
 				convertedCopy.add(convertedElement);
 			}
 			catch (Throwable ex) {
 				if (logger.isDebugEnabled()) {
 					logger.debug("Collection type [" + original.getClass().getName() +
-							"] seems to be read-only - injecting original Collection as-is", ex);
+							"] seems to be read-only - injecting original Collection as-is: " + ex);
 				}
 				return original;
 			}
@@ -533,10 +544,20 @@ class TypeConverterDelegate {
 	protected Map convertToTypedMap(
 			Map original, String propertyName, Class requiredType, TypeDescriptor typeDescriptor) {
 
-		boolean originalAllowed = requiredType.isInstance(original);
-		if (!originalAllowed && !Map.class.isAssignableFrom(requiredType)) {
+		if (!Map.class.isAssignableFrom(requiredType)) {
 			return original;
 		}
+
+		boolean approximable = CollectionFactory.isApproximableMapType(requiredType);
+		if (!approximable && !canCreateCopy(requiredType)) {
+			if (logger.isDebugEnabled()) {
+				logger.debug("Custom Map type [" + original.getClass().getName() +
+						"] does not allow for creating a copy - injecting original Map as-is");
+			}
+			return original;
+		}
+
+		boolean originalAllowed = requiredType.isInstance(original);
 		typeDescriptor = typeDescriptor.narrow(original);
 		TypeDescriptor keyType = typeDescriptor.getMapKeyTypeDescriptor();
 		TypeDescriptor valueType = typeDescriptor.getMapValueTypeDescriptor();
@@ -559,14 +580,14 @@ class TypeConverterDelegate {
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Cannot access Map of type [" + original.getClass().getName() +
-						"] - injecting original Map as-is", ex);
+						"] - injecting original Map as-is: " + ex);
 			}
 			return original;
 		}
 
 		Map convertedCopy;
 		try {
-			if (CollectionFactory.isApproximableMapType(requiredType)) {
+			if (approximable) {
 				convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
 			}
 			else {
@@ -576,7 +597,7 @@ class TypeConverterDelegate {
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Cannot create copy of Map type [" + original.getClass().getName() +
-						"] - injecting original Map as-is", ex);
+						"] - injecting original Map as-is: " + ex);
 			}
 			return original;
 		}
@@ -586,15 +607,17 @@ class TypeConverterDelegate {
 			Object key = entry.getKey();
 			Object value = entry.getValue();
 			String keyedPropertyName = buildKeyedPropertyName(propertyName, key);
-			Object convertedKey = convertIfNecessary(keyedPropertyName, null, key, keyType != null ? keyType.getType() : null, typeDescriptor.getMapKeyTypeDescriptor());
-			Object convertedValue = convertIfNecessary(keyedPropertyName, null, value, valueType!= null ? valueType.getType() : null, typeDescriptor.getMapValueTypeDescriptor());
+			Object convertedKey = convertIfNecessary(keyedPropertyName, null, key,
+					(keyType != null ? keyType.getType() : null), typeDescriptor.getMapKeyTypeDescriptor());
+			Object convertedValue = convertIfNecessary(keyedPropertyName, null, value,
+					(valueType!= null ? valueType.getType() : null), typeDescriptor.getMapValueTypeDescriptor());
 			try {
 				convertedCopy.put(convertedKey, convertedValue);
 			}
 			catch (Throwable ex) {
 				if (logger.isDebugEnabled()) {
 					logger.debug("Map type [" + original.getClass().getName() +
-							"] seems to be read-only - injecting original Map as-is", ex);
+							"] seems to be read-only - injecting original Map as-is: " + ex);
 				}
 				return original;
 			}
@@ -615,4 +638,9 @@ class TypeConverterDelegate {
 				null);
 	}
 
+	private boolean canCreateCopy(Class requiredType) {
+		return (!requiredType.isInterface() && !Modifier.isAbstract(requiredType.getModifiers()) &&
+				Modifier.isPublic(requiredType.getModifiers()) && ClassUtils.hasConstructor(requiredType));
+	}
+
 }