diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java index ced0e15bfe8..8906a452996 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/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) { + Collection original, String propertyName, Class requiredType, MethodParameter methodParam) { - Class elementType = null; - if (methodParam != null) { - elementType = GenericCollectionTypeResolver.getCollectionParameterType(methodParam); - } - if (elementType == null && - !this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) { - return original; - } - - 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) { diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/XmlBeanCollectionTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/XmlBeanCollectionTests.java index 53dfea90736..00e3b20b959 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/XmlBeanCollectionTests.java +++ b/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; } diff --git a/org.springframework.beans/src/test/java/test/beans/TestBean.java b/org.springframework.beans/src/test/java/test/beans/TestBean.java index 97e7ad694a0..d1f191549c7 100644 --- a/org.springframework.beans/src/test/java/test/beans/TestBean.java +++ b/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 setActualSpouse(TestBean spouse) { + this.spouses = new ITestBean[] {spouse}; + } + + public ITestBean getSpouse() { + return (spouses != null ? spouses[0] : null); + } + public ITestBean[] getSpouses() { return spouses; } diff --git a/org.springframework.beans/src/test/resources/org/springframework/beans/factory/xml/collections.xml b/org.springframework.beans/src/test/resources/org/springframework/beans/factory/xml/collections.xml index da5a046946e..2aa3ba5dee3 100644 --- a/org.springframework.beans/src/test/resources/org/springframework/beans/factory/xml/collections.xml +++ b/org.springframework.beans/src/test/resources/org/springframework/beans/factory/xml/collections.xml @@ -47,6 +47,7 @@ David 27 + @@ -210,7 +211,6 @@ - @@ -221,6 +221,25 @@ + + + + bar + + + + + + + + + + + + + + +