Skip to content

Commit d317b63

Browse files
jhoellerjhoeller
committed
CollectionToCollectionConverter avoids collection copy for untyped collection when simply returning source anyway
Also uses addAll instead of iteration over untyped collection now, supporting optimized addAll in target collection type, and avoids repeated getElementTypeDescriptor calls. Issue: SPR-11479 (cherry picked from commit bea94d5)
1 parent b1bcc5d commit d317b63

File tree

2 files changed

+79
-55
lines changed

2 files changed

+79
-55
lines changed

‎spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java

+18-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -34,16 +34,19 @@
3434
* parameterized type to the target collection's parameterized type if necessary.
3535
*
3636
* @author Keith Donald
37+
* @author Juergen Hoeller
3738
* @since 3.0
3839
*/
3940
final class CollectionToCollectionConverter implements ConditionalGenericConverter {
4041

4142
private final ConversionService conversionService;
4243

44+
4345
public CollectionToCollectionConverter(ConversionService conversionService) {
4446
this.conversionService = conversionService;
4547
}
4648

49+
4750
public Set<ConvertiblePair> getConvertibleTypes() {
4851
return Collections.singleton(new ConvertiblePair(Collection.class, Collection.class));
4952
}
@@ -58,27 +61,34 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
5861
if (source == null) {
5962
return null;
6063
}
61-
boolean copyRequired = !targetType.getType().isInstance(source);
6264
Collection<?> sourceCollection = (Collection<?>) source;
65+
66+
// Shortcut if possible...
67+
boolean copyRequired = !targetType.getType().isInstance(source);
6368
if (!copyRequired && sourceCollection.isEmpty()) {
64-
return sourceCollection;
69+
return source;
6570
}
71+
TypeDescriptor elementDesc = targetType.getElementTypeDescriptor();
72+
if (elementDesc == null && !copyRequired) {
73+
return source;
74+
}
75+
76+
// At this point, we need a collection copy in any case, even if just for finding out about element copies...
6677
Collection<Object> target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size());
67-
if (targetType.getElementTypeDescriptor() == null) {
68-
for (Object element : sourceCollection) {
69-
target.add(element);
70-
}
78+
if (elementDesc == null) {
79+
target.addAll(sourceCollection);
7180
}
7281
else {
7382
for (Object sourceElement : sourceCollection) {
7483
Object targetElement = this.conversionService.convert(sourceElement,
75-
sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor());
84+
sourceType.elementTypeDescriptor(sourceElement), elementDesc);
7685
target.add(targetElement);
7786
if (sourceElement != targetElement) {
7887
copyRequired = true;
7988
}
8089
}
8190
}
91+
8292
return (copyRequired ? target : source);
8393
}
8494

‎spring-core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java

+61-47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -44,16 +44,19 @@
4444
/**
4545
* @author Keith Donald
4646
* @author Juergen Hoeller
47+
* @author Stephane Nicoll
4748
*/
4849
public class CollectionToCollectionConverterTests {
4950

5051
private GenericConversionService conversionService = new GenericConversionService();
5152

53+
5254
@Before
5355
public void setUp() {
5456
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
5557
}
5658

59+
5760
@Test
5861
public void scalarList() throws Exception {
5962
List<String> list = new ArrayList<String>();
@@ -77,8 +80,6 @@ public void scalarList() throws Exception {
7780
assertEquals(37, result.get(1));
7881
}
7982

80-
public ArrayList<Integer> scalarListTarget;
81-
8283
@Test
8384
public void emptyListToList() throws Exception {
8485
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
@@ -90,8 +91,6 @@ public void emptyListToList() throws Exception {
9091
assertEquals(list, conversionService.convert(list, sourceType, targetType));
9192
}
9293

93-
public List<Integer> emptyListTarget;
94-
9594
@Test
9695
public void emptyListToListDifferentTargetType() throws Exception {
9796
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
@@ -106,8 +105,6 @@ public void emptyListToListDifferentTargetType() throws Exception {
106105
assertTrue(result.isEmpty());
107106
}
108107

109-
public LinkedList<Integer> emptyListDifferentTarget;
110-
111108
@Test
112109
public void collectionToObjectInteraction() throws Exception {
113110
List<List<String>> list = new ArrayList<List<String>>();
@@ -149,8 +146,6 @@ public void objectToCollection() throws Exception {
149146
assertEquals((Integer) 23, result.get(1).get(1).get(0));
150147
}
151148

152-
public List<List<List<Integer>>> objectToCollection;
153-
154149
@Test
155150
@SuppressWarnings("unchecked")
156151
public void stringToCollection() throws Exception {
@@ -165,10 +160,46 @@ public void stringToCollection() throws Exception {
165160
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("objectToCollection"));
166161
assertTrue(conversionService.canConvert(sourceType, targetType));
167162
List<List<List<Integer>>> result = (List<List<List<Integer>>>) conversionService.convert(list, sourceType, targetType);
168-
assertEquals((Integer)9, result.get(0).get(0).get(0));
169-
assertEquals((Integer)12, result.get(0).get(0).get(1));
170-
assertEquals((Integer)37, result.get(1).get(0).get(0));
171-
assertEquals((Integer)23, result.get(1).get(0).get(1));
163+
assertEquals((Integer) 9, result.get(0).get(0).get(0));
164+
assertEquals((Integer) 12, result.get(0).get(0).get(1));
165+
assertEquals((Integer) 37, result.get(1).get(0).get(0));
166+
assertEquals((Integer) 23, result.get(1).get(0).get(1));
167+
}
168+
169+
@Test
170+
public void convertEmptyVector_shouldReturnEmptyArrayList() {
171+
Vector<String> vector = new Vector<String>();
172+
vector.add("Element");
173+
testCollectionConversionToArrayList(vector);
174+
}
175+
176+
@Test
177+
public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() {
178+
Vector<String> vector = new Vector<String>();
179+
vector.add("Element");
180+
testCollectionConversionToArrayList(vector);
181+
}
182+
183+
@Test
184+
public void testCollectionsEmptyList() throws Exception {
185+
CollectionToCollectionConverter converter = new CollectionToCollectionConverter(new GenericConversionService());
186+
TypeDescriptor type = new TypeDescriptor(getClass().getField("list"));
187+
converter.convert(list, type, TypeDescriptor.valueOf(Class.forName("java.util.Collections$EmptyList")));
188+
}
189+
190+
@SuppressWarnings("rawtypes")
191+
private void testCollectionConversionToArrayList(Collection<String> aSource) {
192+
Object myConverted = (new CollectionToCollectionConverter(new GenericConversionService())).convert(
193+
aSource, TypeDescriptor.forObject(aSource), TypeDescriptor.forObject(new ArrayList()));
194+
assertTrue(myConverted instanceof ArrayList<?>);
195+
assertEquals(aSource.size(), ((ArrayList<?>) myConverted).size());
196+
}
197+
198+
@Test
199+
public void listToCollectionNoCopyRequired() throws NoSuchFieldException {
200+
List<?> input = new ArrayList<String>(Arrays.asList("foo", "bar"));
201+
assertSame(input, conversionService.convert(input, TypeDescriptor.forObject(input),
202+
new TypeDescriptor(getClass().getField("wildCardCollection"))));
172203
}
173204

174205
@Test
@@ -210,8 +241,6 @@ public void elementTypesNotConvertible() throws Exception {
210241
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
211242
}
212243

213-
public List<String> strings;
214-
215244
@Test(expected=ConversionFailedException.class)
216245
public void nothingInCommon() throws Exception {
217246
List<Object> resources = new ArrayList<Object>();
@@ -221,8 +250,24 @@ public void nothingInCommon() throws Exception {
221250
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
222251
}
223252

253+
254+
public ArrayList<Integer> scalarListTarget;
255+
256+
public List<Integer> emptyListTarget;
257+
258+
public LinkedList<Integer> emptyListDifferentTarget;
259+
260+
public List<List<List<Integer>>> objectToCollection;
261+
262+
public List<String> strings;
263+
264+
public List list = Collections.emptyList();
265+
266+
public Collection<?> wildCardCollection = Collections.emptyList();
267+
224268
public List<Resource> resources;
225269

270+
226271
public static abstract class BaseResource implements Resource {
227272

228273
@Override
@@ -286,39 +331,8 @@ public String getDescription() {
286331
}
287332
}
288333

289-
public static class TestResource extends BaseResource {
290-
291-
}
292-
293-
@Test
294-
public void convertEmptyVector_shouldReturnEmptyArrayList() {
295-
Vector<String> vector = new Vector<String>();
296-
vector.add("Element");
297-
testCollectionConversionToArrayList(vector);
298-
}
299334

300-
@Test
301-
public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() {
302-
Vector<String> vector = new Vector<String>();
303-
vector.add("Element");
304-
testCollectionConversionToArrayList(vector);
305-
}
306-
307-
@Test
308-
public void testCollectionsEmptyList() throws Exception {
309-
CollectionToCollectionConverter converter = new CollectionToCollectionConverter(new GenericConversionService());
310-
TypeDescriptor type = new TypeDescriptor(getClass().getField("list"));
311-
converter.convert(list, type, TypeDescriptor.valueOf(Class.forName("java.util.Collections$EmptyList")));
312-
}
313-
314-
public List list = Collections.emptyList();
315-
316-
@SuppressWarnings("rawtypes")
317-
private void testCollectionConversionToArrayList(Collection<String> aSource) {
318-
Object myConverted = (new CollectionToCollectionConverter(new GenericConversionService())).convert(
319-
aSource, TypeDescriptor.forObject(aSource), TypeDescriptor.forObject(new ArrayList()));
320-
assertTrue(myConverted instanceof ArrayList<?>);
321-
assertEquals(aSource.size(), ((ArrayList<?>) myConverted).size());
335+
public static class TestResource extends BaseResource {
322336
}
323337

324338
}

0 commit comments

Comments
 (0)