Skip to content

Commit 9a48f3f

Browse files
jhoellerjhoeller
committed
try to create unknown collection implementation types via default constructor
1 parent b118aae commit 9a48f3f

File tree

4 files changed

+157
-57
lines changed

4 files changed

+157
-57
lines changed

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

Lines changed: 84 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -195,28 +195,32 @@ else if (requiredType.isArray()) {
195195
// Array required -> apply appropriate conversion of elements.
196196
return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType());
197197
}
198-
else if (convertedValue instanceof Collection && CollectionFactory.isApproximableCollectionType(requiredType)) {
198+
else if (convertedValue instanceof Collection) {
199199
// Convert elements to target type, if determined.
200-
convertedValue = convertToTypedCollection((Collection) convertedValue, propertyName, methodParam);
200+
convertedValue = convertToTypedCollection(
201+
(Collection) convertedValue, propertyName, requiredType, methodParam);
201202
}
202-
else if (convertedValue instanceof Map && CollectionFactory.isApproximableMapType(requiredType)) {
203+
else if (convertedValue instanceof Map) {
203204
// Convert keys and values to respective target type, if determined.
204-
convertedValue = convertToTypedMap((Map) convertedValue, propertyName, methodParam);
205+
convertedValue = convertToTypedMap(
206+
(Map) convertedValue, propertyName, requiredType, methodParam);
205207
}
206208
else if (convertedValue instanceof String && !requiredType.isInstance(convertedValue)) {
207-
try {
208-
Constructor strCtor = requiredType.getConstructor(String.class);
209-
return (T) BeanUtils.instantiateClass(strCtor, convertedValue);
210-
}
211-
catch (NoSuchMethodException ex) {
212-
// proceed with field lookup
213-
if (logger.isTraceEnabled()) {
214-
logger.trace("No String constructor found on type [" + requiredType.getName() + "]", ex);
209+
if (!requiredType.isInterface() && !requiredType.isEnum()) {
210+
try {
211+
Constructor strCtor = requiredType.getConstructor(String.class);
212+
return (T) BeanUtils.instantiateClass(strCtor, convertedValue);
215213
}
216-
}
217-
catch (Exception ex) {
218-
if (logger.isTraceEnabled()) {
219-
logger.trace("Construction via String failed for type [" + requiredType.getName() + "]", ex);
214+
catch (NoSuchMethodException ex) {
215+
// proceed with field lookup
216+
if (logger.isTraceEnabled()) {
217+
logger.trace("No String constructor found on type [" + requiredType.getName() + "]", ex);
218+
}
219+
}
220+
catch (Exception ex) {
221+
if (logger.isDebugEnabled()) {
222+
logger.debug("Construction via String failed for type [" + requiredType.getName() + "]", ex);
223+
}
220224
}
221225
}
222226
String trimmedValue = ((String) convertedValue).trim();
@@ -431,18 +435,8 @@ else if (input.getClass().isArray()) {
431435

432436
@SuppressWarnings("unchecked")
433437
protected Collection convertToTypedCollection(
434-
Collection original, String propertyName, MethodParameter methodParam) {
435-
436-
Class elementType = null;
437-
if (methodParam != null) {
438-
elementType = GenericCollectionTypeResolver.getCollectionParameterType(methodParam);
439-
}
440-
if (elementType == null &&
441-
!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
442-
return original;
443-
}
438+
Collection original, String propertyName, Class requiredType, MethodParameter methodParam) {
444439

445-
Collection convertedCopy;
446440
Iterator it;
447441
try {
448442
it = original.iterator();
@@ -453,7 +447,6 @@ protected Collection convertToTypedCollection(
453447
}
454448
return original;
455449
}
456-
convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
457450
}
458451
catch (Throwable ex) {
459452
if (logger.isDebugEnabled()) {
@@ -462,7 +455,34 @@ protected Collection convertToTypedCollection(
462455
}
463456
return original;
464457
}
465-
boolean actuallyConverted = false;
458+
459+
Collection convertedCopy;
460+
try {
461+
if (CollectionFactory.isApproximableCollectionType(requiredType)) {
462+
convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
463+
}
464+
else {
465+
convertedCopy = (Collection) requiredType.newInstance();
466+
}
467+
}
468+
catch (Throwable ex) {
469+
if (logger.isDebugEnabled()) {
470+
logger.debug("Cannot create copy of Collection type [" + original.getClass().getName() +
471+
"] - injecting original Collection as-is", ex);
472+
}
473+
return original;
474+
}
475+
476+
boolean originalAllowed = requiredType.isInstance(original);
477+
Class elementType = null;
478+
if (methodParam != null) {
479+
elementType = GenericCollectionTypeResolver.getCollectionParameterType(methodParam);
480+
}
481+
if (elementType == null && originalAllowed &&
482+
!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
483+
return original;
484+
}
485+
466486
int i = 0;
467487
for (; it.hasNext(); i++) {
468488
Object element = it.next();
@@ -476,25 +496,13 @@ protected Collection convertToTypedCollection(
476496
methodParam.decreaseNestingLevel();
477497
}
478498
convertedCopy.add(convertedElement);
479-
actuallyConverted = actuallyConverted || (element != convertedElement);
499+
originalAllowed = originalAllowed && (element == convertedElement);
480500
}
481-
return (actuallyConverted ? convertedCopy : original);
501+
return (originalAllowed ? original : convertedCopy);
482502
}
483503

484504
@SuppressWarnings("unchecked")
485-
protected Map convertToTypedMap(Map original, String propertyName, MethodParameter methodParam) {
486-
Class keyType = null;
487-
Class valueType = null;
488-
if (methodParam != null) {
489-
keyType = GenericCollectionTypeResolver.getMapKeyParameterType(methodParam);
490-
valueType = GenericCollectionTypeResolver.getMapValueParameterType(methodParam);
491-
}
492-
if (keyType == null && valueType == null &&
493-
!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
494-
return original;
495-
}
496-
497-
Map convertedCopy;
505+
protected Map convertToTypedMap(Map original, String propertyName, Class requiredType, MethodParameter methodParam) {
498506
Iterator it;
499507
try {
500508
it = original.entrySet().iterator();
@@ -505,7 +513,6 @@ protected Map convertToTypedMap(Map original, String propertyName, MethodParamet
505513
}
506514
return original;
507515
}
508-
convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
509516
}
510517
catch (Throwable ex) {
511518
if (logger.isDebugEnabled()) {
@@ -514,7 +521,36 @@ protected Map convertToTypedMap(Map original, String propertyName, MethodParamet
514521
}
515522
return original;
516523
}
517-
boolean actuallyConverted = false;
524+
525+
Map convertedCopy;
526+
try {
527+
if (CollectionFactory.isApproximableMapType(requiredType)) {
528+
convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
529+
}
530+
else {
531+
convertedCopy = (Map) requiredType.newInstance();
532+
}
533+
}
534+
catch (Throwable ex) {
535+
if (logger.isDebugEnabled()) {
536+
logger.debug("Cannot create copy of Map type [" + original.getClass().getName() +
537+
"] - injecting original Map as-is", ex);
538+
}
539+
return original;
540+
}
541+
542+
boolean originalAllowed = requiredType.isInstance(original);
543+
Class keyType = null;
544+
Class valueType = null;
545+
if (methodParam != null) {
546+
keyType = GenericCollectionTypeResolver.getMapKeyParameterType(methodParam);
547+
valueType = GenericCollectionTypeResolver.getMapValueParameterType(methodParam);
548+
}
549+
if (keyType == null && valueType == null && originalAllowed &&
550+
!this.propertyEditorRegistry.hasCustomEditorForElement(null, propertyName)) {
551+
return original;
552+
}
553+
518554
while (it.hasNext()) {
519555
Map.Entry entry = (Map.Entry) it.next();
520556
Object key = entry.getKey();
@@ -533,9 +569,9 @@ protected Map convertToTypedMap(Map original, String propertyName, MethodParamet
533569
methodParam.decreaseNestingLevel();
534570
}
535571
convertedCopy.put(convertedKey, convertedValue);
536-
actuallyConverted = actuallyConverted || (key != convertedKey) || (value != convertedValue);
572+
originalAllowed = originalAllowed && (key == convertedKey) && (value == convertedValue);
537573
}
538-
return (actuallyConverted ? convertedCopy : original);
574+
return (originalAllowed ? original : convertedCopy);
539575
}
540576

541577
private String buildIndexedPropertyName(String propertyName, int index) {

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

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,12 @@
2727
import java.util.Set;
2828
import java.util.TreeMap;
2929
import java.util.TreeSet;
30+
import java.util.IdentityHashMap;
31+
import java.util.HashSet;
32+
import java.util.concurrent.CopyOnWriteArraySet;
3033

31-
import static org.junit.Assert.*;
3234
import org.junit.Test;
35+
import static org.junit.Assert.*;
3336
import test.beans.TestBean;
3437

3538
import org.springframework.beans.factory.BeanCreationException;
@@ -120,13 +123,15 @@ public void testRefSubelementsBuildCollectionWithPrototypes() throws Exception {
120123
TestBean jen = (TestBean) this.beanFactory.getBean("pJenny");
121124
TestBean dave = (TestBean) this.beanFactory.getBean("pDavid");
122125
TestBean rod = (TestBean) this.beanFactory.getBean("pRod");
126+
123127
Object[] friends = rod.getFriends().toArray();
124128
assertTrue(friends.length == 2);
125129
assertTrue("First friend must be jen, not " + friends[0],
126130
friends[0].toString().equals(jen.toString()));
127131
assertTrue("Jen not same instance", friends[0] != jen);
128132
assertTrue(friends[1].toString().equals(dave.toString()));
129133
assertTrue("Dave not same instance", friends[1] != dave);
134+
assertEquals("Jen", dave.getSpouse().getName());
130135

131136
TestBean rod2 = (TestBean) this.beanFactory.getBean("pRod");
132137
Object[] friends2 = rod2.getFriends().toArray();
@@ -273,6 +278,25 @@ public void testPopulatedSet() throws Exception {
273278
assertEquals(null, it.next());
274279
}
275280

281+
@Test
282+
public void testPopulatedConcurrentSet() throws Exception {
283+
HasMap hasMap = (HasMap) this.beanFactory.getBean("concurrentSet");
284+
assertTrue(hasMap.getConcurrentSet().size() == 3);
285+
assertTrue(hasMap.getConcurrentSet().contains("bar"));
286+
TestBean jenny = (TestBean) this.beanFactory.getBean("jenny");
287+
assertTrue(hasMap.getConcurrentSet().contains(jenny));
288+
assertTrue(hasMap.getConcurrentSet().contains(null));
289+
}
290+
291+
@Test
292+
public void testPopulatedIdentityMap() throws Exception {
293+
HasMap hasMap = (HasMap) this.beanFactory.getBean("identityMap");
294+
assertTrue(hasMap.getIdentityMap().size() == 2);
295+
HashSet set = new HashSet(hasMap.getIdentityMap().keySet());
296+
assertTrue(set.contains("foo"));
297+
assertTrue(set.contains("jenny"));
298+
}
299+
276300
@Test
277301
public void testEmptyProps() throws Exception {
278302
HasMap hasMap = (HasMap) this.beanFactory.getBean("emptyProps");
@@ -399,6 +423,7 @@ public void testEnumSetFactory() throws Exception {
399423
assertTrue(set.contains("TWO"));
400424
}
401425

426+
402427
public static class MapAndSet {
403428

404429
private Object obj;
@@ -415,7 +440,6 @@ public Object getObject() {
415440
return obj;
416441
}
417442
}
418-
419443
}
420444

421445

@@ -429,8 +453,12 @@ class HasMap {
429453

430454
private Map map;
431455

456+
private IdentityHashMap identityMap;
457+
432458
private Set set;
433459

460+
private CopyOnWriteArraySet concurrentSet;
461+
434462
private Properties props;
435463

436464
private Object[] objectArray;
@@ -439,9 +467,6 @@ class HasMap {
439467

440468
private Integer[] intArray;
441469

442-
private HasMap() {
443-
}
444-
445470
public Map getMap() {
446471
return map;
447472
}
@@ -450,6 +475,14 @@ public void setMap(Map map) {
450475
this.map = map;
451476
}
452477

478+
public IdentityHashMap getIdentityMap() {
479+
return identityMap;
480+
}
481+
482+
public void setIdentityMap(IdentityHashMap identityMap) {
483+
this.identityMap = identityMap;
484+
}
485+
453486
public Set getSet() {
454487
return set;
455488
}
@@ -458,6 +491,14 @@ public void setSet(Set set) {
458491
this.set = set;
459492
}
460493

494+
public CopyOnWriteArraySet getConcurrentSet() {
495+
return concurrentSet;
496+
}
497+
498+
public void setConcurrentSet(CopyOnWriteArraySet concurrentSet) {
499+
this.concurrentSet = concurrentSet;
500+
}
501+
461502
public Properties getProps() {
462503
return props;
463504
}

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,18 @@ public void setJedi(boolean jedi) {
196196
this.jedi = jedi;
197197
}
198198

199-
public ITestBean getSpouse() {
200-
return (spouses != null ? spouses[0] : null);
199+
public void setSpouse(ITestBean spouse) {
200+
this.spouses = new ITestBean[] {spouse};
201201
}
202202

203-
public void setSpouse(ITestBean spouse) {
203+
public void setActualSpouse(TestBean spouse) {
204204
this.spouses = new ITestBean[] {spouse};
205205
}
206206

207+
public ITestBean getSpouse() {
208+
return (spouses != null ? spouses[0] : null);
209+
}
210+
207211
public ITestBean[] getSpouses() {
208212
return spouses;
209213
}

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
<bean id="pDavid" class="test.beans.TestBean" scope="prototype">
4848
<property name="name"><value>David</value></property>
4949
<property name="age"><value>27</value></property>
50+
<property name="actualSpouse" value="Jen"/>
5051
</bean>
5152

5253
<bean id="pRod" class="test.beans.TestBean" scope="prototype">
@@ -210,7 +211,6 @@
210211
</property>
211212
</bean>
212213

213-
214214
<bean id="set" class="org.springframework.beans.factory.xml.HasMap">
215215
<property name="set">
216216
<set>
@@ -221,6 +221,25 @@
221221
</property>
222222
</bean>
223223

224+
<bean id="concurrentSet" class="org.springframework.beans.factory.xml.HasMap">
225+
<property name="concurrentSet">
226+
<set>
227+
<value>bar</value>
228+
<ref local="jenny"/>
229+
<null/>
230+
</set>
231+
</property>
232+
</bean>
233+
234+
<bean id="identityMap" class="org.springframework.beans.factory.xml.HasMap">
235+
<property name="identityMap">
236+
<map>
237+
<entry key="foo" value="bar"/>
238+
<entry key="jenny" value-ref="pJenny"/>
239+
</map>
240+
</property>
241+
</bean>
242+
224243
<bean id="emptyProps" class="org.springframework.beans.factory.xml.HasMap">
225244
<property name="props">
226245
<props>

0 commit comments

Comments
 (0)