Permalink
Browse files

CollectionToCollectionConverter avoids collection copy for untyped co…

…llection 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
  • Loading branch information...
1 parent 40b81fc commit bea94d5302e16f72a64377c052595a1976629dde @jhoeller jhoeller committed Mar 10, 2014
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -34,16 +34,19 @@
* parameterized type to the target collection's parameterized type if necessary.
*
* @author Keith Donald
+ * @author Juergen Hoeller
* @since 3.0
*/
final class CollectionToCollectionConverter implements ConditionalGenericConverter {
private final ConversionService conversionService;
+
public CollectionToCollectionConverter(ConversionService conversionService) {
this.conversionService = conversionService;
}
+
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return Collections.singleton(new ConvertiblePair(Collection.class, Collection.class));
@@ -60,27 +63,34 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
if (source == null) {
return null;
}
- boolean copyRequired = !targetType.getType().isInstance(source);
Collection<?> sourceCollection = (Collection<?>) source;
+
+ // Shortcut if possible...
+ boolean copyRequired = !targetType.getType().isInstance(source);
if (!copyRequired && sourceCollection.isEmpty()) {
- return sourceCollection;
+ return source;
}
+ TypeDescriptor elementDesc = targetType.getElementTypeDescriptor();
+ if (elementDesc == null && !copyRequired) {
+ return source;
+ }
+
+ // At this point, we need a collection copy in any case, even if just for finding out about element copies...
Collection<Object> target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size());
- if (targetType.getElementTypeDescriptor() == null) {
- for (Object element : sourceCollection) {
- target.add(element);
- }
+ if (elementDesc == null) {
+ target.addAll(sourceCollection);
}
else {
for (Object sourceElement : sourceCollection) {
Object targetElement = this.conversionService.convert(sourceElement,
- sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor());
+ sourceType.elementTypeDescriptor(sourceElement), elementDesc);
target.add(targetElement);
if (sourceElement != targetElement) {
copyRequired = true;
}
}
}
+
return (copyRequired ? target : source);
}
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -44,16 +44,19 @@
/**
* @author Keith Donald
* @author Juergen Hoeller
+ * @author Stephane Nicoll
*/
public class CollectionToCollectionConverterTests {
private GenericConversionService conversionService = new GenericConversionService();
+
@Before
public void setUp() {
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
}
+
@Test
public void scalarList() throws Exception {
List<String> list = new ArrayList<String>();
@@ -77,8 +80,6 @@ public void scalarList() throws Exception {
assertEquals(37, result.get(1));
}
- public ArrayList<Integer> scalarListTarget;
-
@Test
public void emptyListToList() throws Exception {
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
@@ -90,8 +91,6 @@ public void emptyListToList() throws Exception {
assertEquals(list, conversionService.convert(list, sourceType, targetType));
}
- public List<Integer> emptyListTarget;
-
@Test
public void emptyListToListDifferentTargetType() throws Exception {
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
@@ -106,8 +105,6 @@ public void emptyListToListDifferentTargetType() throws Exception {
assertTrue(result.isEmpty());
}
- public LinkedList<Integer> emptyListDifferentTarget;
-
@Test
public void collectionToObjectInteraction() throws Exception {
List<List<String>> list = new ArrayList<List<String>>();
@@ -149,8 +146,6 @@ public void objectToCollection() throws Exception {
assertEquals((Integer) 23, result.get(1).get(1).get(0));
}
- public List<List<List<Integer>>> objectToCollection;
-
@Test
@SuppressWarnings("unchecked")
public void stringToCollection() throws Exception {
@@ -165,10 +160,46 @@ public void stringToCollection() throws Exception {
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("objectToCollection"));
assertTrue(conversionService.canConvert(sourceType, targetType));
List<List<List<Integer>>> result = (List<List<List<Integer>>>) conversionService.convert(list, sourceType, targetType);
- assertEquals((Integer)9, result.get(0).get(0).get(0));
- assertEquals((Integer)12, result.get(0).get(0).get(1));
- assertEquals((Integer)37, result.get(1).get(0).get(0));
- assertEquals((Integer)23, result.get(1).get(0).get(1));
+ assertEquals((Integer) 9, result.get(0).get(0).get(0));
+ assertEquals((Integer) 12, result.get(0).get(0).get(1));
+ assertEquals((Integer) 37, result.get(1).get(0).get(0));
+ assertEquals((Integer) 23, result.get(1).get(0).get(1));
+ }
+
+ @Test
+ public void convertEmptyVector_shouldReturnEmptyArrayList() {
+ Vector<String> vector = new Vector<String>();
+ vector.add("Element");
+ testCollectionConversionToArrayList(vector);
+ }
+
+ @Test
+ public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() {
+ Vector<String> vector = new Vector<String>();
+ vector.add("Element");
+ testCollectionConversionToArrayList(vector);
+ }
+
+ @Test
+ public void testCollectionsEmptyList() throws Exception {
+ CollectionToCollectionConverter converter = new CollectionToCollectionConverter(new GenericConversionService());
+ TypeDescriptor type = new TypeDescriptor(getClass().getField("list"));
+ converter.convert(list, type, TypeDescriptor.valueOf(Class.forName("java.util.Collections$EmptyList")));
+ }
+
+ @SuppressWarnings("rawtypes")
+ private void testCollectionConversionToArrayList(Collection<String> aSource) {
+ Object myConverted = (new CollectionToCollectionConverter(new GenericConversionService())).convert(
+ aSource, TypeDescriptor.forObject(aSource), TypeDescriptor.forObject(new ArrayList()));
+ assertTrue(myConverted instanceof ArrayList<?>);
+ assertEquals(aSource.size(), ((ArrayList<?>) myConverted).size());
+ }
+
+ @Test
+ public void listToCollectionNoCopyRequired() throws NoSuchFieldException {
+ List<?> input = new ArrayList<String>(Arrays.asList("foo", "bar"));
+ assertSame(input, conversionService.convert(input, TypeDescriptor.forObject(input),
+ new TypeDescriptor(getClass().getField("wildCardCollection"))));
}
@Test
@@ -210,8 +241,6 @@ public void elementTypesNotConvertible() throws Exception {
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
}
- public List<String> strings;
-
@Test(expected=ConversionFailedException.class)
public void nothingInCommon() throws Exception {
List<Object> resources = new ArrayList<Object>();
@@ -221,8 +250,24 @@ public void nothingInCommon() throws Exception {
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
}
+
+ public ArrayList<Integer> scalarListTarget;
+
+ public List<Integer> emptyListTarget;
+
+ public LinkedList<Integer> emptyListDifferentTarget;
+
+ public List<List<List<Integer>>> objectToCollection;
+
+ public List<String> strings;
+
+ public List list = Collections.emptyList();
+
+ public Collection<?> wildCardCollection = Collections.emptyList();
+
public List<Resource> resources;
+
public static abstract class BaseResource implements Resource {
@Override
@@ -286,39 +331,8 @@ public String getDescription() {
}
}
- public static class TestResource extends BaseResource {
-
- }
-
- @Test
- public void convertEmptyVector_shouldReturnEmptyArrayList() {
- Vector<String> vector = new Vector<String>();
- vector.add("Element");
- testCollectionConversionToArrayList(vector);
- }
- @Test
- public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() {
- Vector<String> vector = new Vector<String>();
- vector.add("Element");
- testCollectionConversionToArrayList(vector);
- }
-
- @Test
- public void testCollectionsEmptyList() throws Exception {
- CollectionToCollectionConverter converter = new CollectionToCollectionConverter(new GenericConversionService());
- TypeDescriptor type = new TypeDescriptor(getClass().getField("list"));
- converter.convert(list, type, TypeDescriptor.valueOf(Class.forName("java.util.Collections$EmptyList")));
- }
-
- public List list = Collections.emptyList();
-
- @SuppressWarnings("rawtypes")
- private void testCollectionConversionToArrayList(Collection<String> aSource) {
- Object myConverted = (new CollectionToCollectionConverter(new GenericConversionService())).convert(
- aSource, TypeDescriptor.forObject(aSource), TypeDescriptor.forObject(new ArrayList()));
- assertTrue(myConverted instanceof ArrayList<?>);
- assertEquals(aSource.size(), ((ArrayList<?>) myConverted).size());
+ public static class TestResource extends BaseResource {
}
}

0 comments on commit bea94d5

Please sign in to comment.