try to create unknown collection implementation types via default constructor

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

@@ -195,28 +195,32 @@ class TypeConverterDelegate {
 					// Array required -> apply appropriate conversion of elements.
 					return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType());
 				}
-				else if (convertedValue instanceof Collection && CollectionFactory.isApproximableCollectionType(requiredType)) {
+				else if (convertedValue instanceof Collection) {
 					// Convert elements to target type, if determined.
-					convertedValue = convertToTypedCollection((Collection) convertedValue, propertyName, methodParam);
+					convertedValue = convertToTypedCollection(
+							(Collection) convertedValue, propertyName, requiredType, methodParam);
 				}
-				else if (convertedValue instanceof Map && CollectionFactory.isApproximableMapType(requiredType)) {
+				else if (convertedValue instanceof Map) {
 					// Convert keys and values to respective target type, if determined.
-					convertedValue = convertToTypedMap((Map) convertedValue, propertyName, methodParam);
+					convertedValue = convertToTypedMap(
+							(Map) convertedValue, propertyName, requiredType, methodParam);
 				}
 				else if (convertedValue instanceof String && !requiredType.isInstance(convertedValue)) {
-					try {
-						Constructor strCtor = requiredType.getConstructor(String.class);
-						return (T) BeanUtils.instantiateClass(strCtor, convertedValue);
-					}
-					catch (NoSuchMethodException ex) {
-						// proceed with field lookup
-						if (logger.isTraceEnabled()) {
-							logger.trace("No String constructor found on type [" + requiredType.getName() + "]", ex);
+					if (!requiredType.isInterface() && !requiredType.isEnum()) {
+						try {
+							Constructor strCtor = requiredType.getConstructor(String.class);
+							return (T) BeanUtils.instantiateClass(strCtor, convertedValue);
 						}
-					}
-					catch (Exception ex) {
-						if (logger.isTraceEnabled()) {
-							logger.trace("Construction via String failed for type [" + requiredType.getName() + "]", ex);
+						catch (NoSuchMethodException ex) {
+							// proceed with field lookup
+							if (logger.isTraceEnabled()) {
+								logger.trace("No String constructor found on type [" + requiredType.getName() + "]", ex);
+							}
+						}
+						catch (Exception ex) {
+							if (logger.isDebugEnabled()) {
+								logger.debug("Construction via String failed for type [" + requiredType.getName() + "]", ex);
+							}
 						}
 					}
 					String trimmedValue = ((String) convertedValue).trim();
@@ -431,18 +435,8 @@ class TypeConverterDelegate {
 
 	@SuppressWarnings("unchecked")
 	protected Collection convertToTypedCollection(
-			Collection original, String propertyName, MethodParameter methodParam) {
-
-		Class elementType = null;
-		if (methodParam != null) {
-			elementType = GenericCollectionTypeResolver.getCollectionParameterType(methodParam);
-		}
-		if (elementType == null &&
-				!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
-			return original;
-		}
+			Collection original, String propertyName, Class requiredType, MethodParameter methodParam) {
 
-		Collection convertedCopy;
 		Iterator it;
 		try {
 			it = original.iterator();
@@ -453,7 +447,6 @@ class TypeConverterDelegate {
 				}
 				return original;
 			}
-			convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
 		}
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
@@ -462,7 +455,34 @@ class TypeConverterDelegate {
 			}
 			return original;
 		}
-		boolean actuallyConverted = false;
+
+		Collection convertedCopy;
+		try {
+			if (CollectionFactory.isApproximableCollectionType(requiredType)) {
+				convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
+			}
+			else {
+				convertedCopy = (Collection) requiredType.newInstance();
+			}
+		}
+		catch (Throwable ex) {
+			if (logger.isDebugEnabled()) {
+				logger.debug("Cannot create copy of Collection type [" + original.getClass().getName() +
+						"] - injecting original Collection as-is", ex);
+			}
+			return original;
+		}
+
+		boolean originalAllowed = requiredType.isInstance(original);
+		Class elementType = null;
+		if (methodParam != null) {
+			elementType = GenericCollectionTypeResolver.getCollectionParameterType(methodParam);
+		}
+		if (elementType == null && originalAllowed &&
+				!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
+			return original;
+		}
+
 		int i = 0;
 		for (; it.hasNext(); i++) {
 			Object element = it.next();
@@ -476,25 +496,13 @@ class TypeConverterDelegate {
 				methodParam.decreaseNestingLevel();
 			}
 			convertedCopy.add(convertedElement);
-			actuallyConverted = actuallyConverted || (element != convertedElement);
+			originalAllowed = originalAllowed && (element == convertedElement);
 		}
-		return (actuallyConverted ? convertedCopy : original);
+		return (originalAllowed ? original : convertedCopy);
 	}
 
 	@SuppressWarnings("unchecked")
-	protected Map convertToTypedMap(Map original, String propertyName, MethodParameter methodParam) {
-		Class keyType = null;
-		Class valueType = null;
-		if (methodParam != null) {
-			keyType = GenericCollectionTypeResolver.getMapKeyParameterType(methodParam);
-			valueType = GenericCollectionTypeResolver.getMapValueParameterType(methodParam);
-		}
-		if (keyType == null && valueType == null &&
-				!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
-			return original;
-		}
-
-		Map convertedCopy;
+	protected Map convertToTypedMap(Map original, String propertyName, Class requiredType, MethodParameter methodParam) {
 		Iterator it;
 		try {
 			it = original.entrySet().iterator();
@@ -505,7 +513,6 @@ class TypeConverterDelegate {
 				}
 				return original;
 			}
-			convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
 		}
 		catch (Throwable ex) {
 			if (logger.isDebugEnabled()) {
@@ -514,7 +521,36 @@ class TypeConverterDelegate {
 			}
 			return original;
 		}
-		boolean actuallyConverted = false;
+
+		Map convertedCopy;
+		try {
+			if (CollectionFactory.isApproximableMapType(requiredType)) {
+				convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
+			}
+			else {
+				convertedCopy = (Map) requiredType.newInstance();
+			}
+		}
+		catch (Throwable ex) {
+			if (logger.isDebugEnabled()) {
+				logger.debug("Cannot create copy of Map type [" + original.getClass().getName() +
+						"] - injecting original Map as-is", ex);
+			}
+			return original;
+		}
+
+		boolean originalAllowed = requiredType.isInstance(original);
+		Class keyType = null;
+		Class valueType = null;
+		if (methodParam != null) {
+			keyType = GenericCollectionTypeResolver.getMapKeyParameterType(methodParam);
+			valueType = GenericCollectionTypeResolver.getMapValueParameterType(methodParam);
+		}
+		if (keyType == null && valueType == null && originalAllowed &&
+				!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
+			return original;
+		}
+
 		while (it.hasNext()) {
 			Map.Entry entry = (Map.Entry) it.next();
 			Object key = entry.getKey();
@@ -533,9 +569,9 @@ class TypeConverterDelegate {
 				methodParam.decreaseNestingLevel();
 			}
 			convertedCopy.put(convertedKey, convertedValue);
-			actuallyConverted = actuallyConverted || (key != convertedKey) || (value != convertedValue);
+			originalAllowed = originalAllowed && (key == convertedKey) && (value == convertedValue);
 		}
-		return (actuallyConverted ? convertedCopy : original);
+		return (originalAllowed ? original : convertedCopy);
 	}
 
 	private String buildIndexedPropertyName(String propertyName, int index) {

org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/XmlBeanCollectionTests.java

@@ -27,9 +27,12 @@ import java.util.Properties;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.IdentityHashMap;
+import java.util.HashSet;
+import java.util.concurrent.CopyOnWriteArraySet;
 
-import static org.junit.Assert.*;
 import org.junit.Test;
+import static org.junit.Assert.*;
 import test.beans.TestBean;
 
 import org.springframework.beans.factory.BeanCreationException;
@@ -120,6 +123,7 @@ public class XmlBeanCollectionTests {
 		TestBean jen = (TestBean) this.beanFactory.getBean("pJenny");
 		TestBean dave = (TestBean) this.beanFactory.getBean("pDavid");
 		TestBean rod = (TestBean) this.beanFactory.getBean("pRod");
+
 		Object[] friends = rod.getFriends().toArray();
 		assertTrue(friends.length == 2);
 		assertTrue("First friend must be jen, not " + friends[0],
@@ -127,6 +131,7 @@ public class XmlBeanCollectionTests {
 		assertTrue("Jen not same instance", friends[0] != jen);
 		assertTrue(friends[1].toString().equals(dave.toString()));
 		assertTrue("Dave not same instance", friends[1] != dave);
+		assertEquals("Jen", dave.getSpouse().getName());
 
 		TestBean rod2 = (TestBean) this.beanFactory.getBean("pRod");
 		Object[] friends2 = rod2.getFriends().toArray();
@@ -273,6 +278,25 @@ public class XmlBeanCollectionTests {
 		assertEquals(null, it.next());
 	}
 
+	@Test
+	public void testPopulatedConcurrentSet() throws Exception {
+		HasMap hasMap = (HasMap) this.beanFactory.getBean("concurrentSet");
+		assertTrue(hasMap.getConcurrentSet().size() == 3);
+		assertTrue(hasMap.getConcurrentSet().contains("bar"));
+		TestBean jenny = (TestBean) this.beanFactory.getBean("jenny");
+		assertTrue(hasMap.getConcurrentSet().contains(jenny));
+		assertTrue(hasMap.getConcurrentSet().contains(null));
+	}
+
+	@Test
+	public void testPopulatedIdentityMap() throws Exception {
+		HasMap hasMap = (HasMap) this.beanFactory.getBean("identityMap");
+		assertTrue(hasMap.getIdentityMap().size() == 2);
+		HashSet set = new HashSet(hasMap.getIdentityMap().keySet());
+		assertTrue(set.contains("foo"));
+		assertTrue(set.contains("jenny"));
+	}
+
 	@Test
 	public void testEmptyProps() throws Exception {
 		HasMap hasMap = (HasMap) this.beanFactory.getBean("emptyProps");
@@ -399,6 +423,7 @@ public class XmlBeanCollectionTests {
 		assertTrue(set.contains("TWO"));
 	}
 
+
 	public static class MapAndSet {
 
 		private Object obj;
@@ -415,7 +440,6 @@ public class XmlBeanCollectionTests {
 			return obj;
 		}
 	}
-	
 }
 
 
@@ -429,8 +453,12 @@ class HasMap {
 	
 	private Map map;
 
+	private IdentityHashMap identityMap;
+
 	private Set set;
 
+	private CopyOnWriteArraySet concurrentSet;
+
 	private Properties props;
 	
 	private Object[] objectArray;
@@ -439,9 +467,6 @@ class HasMap {
 	
 	private Integer[] intArray;
 
-	private HasMap() {
-	}
-
 	public Map getMap() {
 		return map;
 	}
@@ -450,6 +475,14 @@ class HasMap {
 		this.map = map;
 	}
 
+	public IdentityHashMap getIdentityMap() {
+		return identityMap;
+	}
+
+	public void setIdentityMap(IdentityHashMap identityMap) {
+		this.identityMap = identityMap;
+	}
+
 	public Set getSet() {
 		return set;
 	}
@@ -458,6 +491,14 @@ class HasMap {
 		this.set = set;
 	}
 
+	public CopyOnWriteArraySet getConcurrentSet() {
+		return concurrentSet;
+	}
+
+	public void setConcurrentSet(CopyOnWriteArraySet concurrentSet) {
+		this.concurrentSet = concurrentSet;
+	}
+
 	public Properties getProps() {
 		return props;
 	}

org.springframework.beans/src/test/java/test/beans/TestBean.java

@@ -196,14 +196,18 @@ public class TestBean implements BeanNameAware, BeanFactoryAware, ITestBean, IOt
 		this.jedi = jedi;
 	}
 
-	public ITestBean getSpouse() {
-		return (spouses != null ? spouses[0] : null);
+	public void setSpouse(ITestBean spouse) {
+		this.spouses = new ITestBean[] {spouse};
 	}
 
-	public void setSpouse(ITestBean spouse) {
+	public void setActualSpouse(TestBean spouse) {
 		this.spouses = new ITestBean[] {spouse};
 	}
 
+	public ITestBean getSpouse() {
+		return (spouses != null ? spouses[0] : null);
+	}
+
 	public ITestBean[] getSpouses() {
 		return spouses;
 	}

org.springframework.beans/src/test/resources/org/springframework/beans/factory/xml/collections.xml

@@ -47,6 +47,7 @@
 	<bean id="pDavid" class="test.beans.TestBean" scope="prototype">
 		<property name="name"><value>David</value></property>
 		<property name="age"><value>27</value></property>
+		<property name="actualSpouse" value="Jen"/>
 	</bean>
 
 	<bean id="pRod" class="test.beans.TestBean" scope="prototype">
@@ -210,7 +211,6 @@
 		</property>
 	</bean>
 
-
 	<bean id="set" class="org.springframework.beans.factory.xml.HasMap">
 		<property name="set">
 			<set>
@@ -221,6 +221,25 @@
 		</property>
 	</bean>
 
+	<bean id="concurrentSet" class="org.springframework.beans.factory.xml.HasMap">
+		<property name="concurrentSet">
+			<set>
+				<value>bar</value>
+				<ref local="jenny"/>
+				<null/>
+			</set>
+		</property>
+	</bean>
+
+	<bean id="identityMap" class="org.springframework.beans.factory.xml.HasMap">
+		<property name="identityMap">
+			<map>
+				<entry key="foo" value="bar"/>
+				<entry key="jenny" value-ref="pJenny"/>
+			</map>
+		</property>
+	</bean>
+
 	<bean id="emptyProps" class="org.springframework.beans.factory.xml.HasMap">
 		<property name="props">
 			<props>