diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java index 09ca1f8ad5a..a8071eae58a 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java @@ -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 @@ import org.springframework.core.convert.converter.ConditionalGenericConverter; * 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 getConvertibleTypes() { return Collections.singleton(new ConvertiblePair(Collection.class, Collection.class)); @@ -60,27 +63,34 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert 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 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); } diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java index ddb99a220b1..00bf054a66d 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java @@ -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 @@ import static org.junit.Assert.*; /** * @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 list = new ArrayList(); @@ -77,8 +80,6 @@ public class CollectionToCollectionConverterTests { assertEquals(37, result.get(1)); } - public ArrayList scalarListTarget; - @Test public void emptyListToList() throws Exception { conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); @@ -90,8 +91,6 @@ public class CollectionToCollectionConverterTests { assertEquals(list, conversionService.convert(list, sourceType, targetType)); } - public List emptyListTarget; - @Test public void emptyListToListDifferentTargetType() throws Exception { conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); @@ -106,8 +105,6 @@ public class CollectionToCollectionConverterTests { assertTrue(result.isEmpty()); } - public LinkedList emptyListDifferentTarget; - @Test public void collectionToObjectInteraction() throws Exception { List> list = new ArrayList>(); @@ -149,8 +146,6 @@ public class CollectionToCollectionConverterTests { assertEquals((Integer) 23, result.get(1).get(1).get(0)); } - public List>> objectToCollection; - @Test @SuppressWarnings("unchecked") public void stringToCollection() throws Exception { @@ -165,10 +160,46 @@ public class CollectionToCollectionConverterTests { TypeDescriptor targetType = new TypeDescriptor(getClass().getField("objectToCollection")); assertTrue(conversionService.canConvert(sourceType, targetType)); List>> result = (List>>) 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 vector = new Vector(); + vector.add("Element"); + testCollectionConversionToArrayList(vector); + } + + @Test + public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() { + Vector vector = new Vector(); + 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 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(Arrays.asList("foo", "bar")); + assertSame(input, conversionService.convert(input, TypeDescriptor.forObject(input), + new TypeDescriptor(getClass().getField("wildCardCollection")))); } @Test @@ -210,8 +241,6 @@ public class CollectionToCollectionConverterTests { assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources")))); } - public List strings; - @Test(expected=ConversionFailedException.class) public void nothingInCommon() throws Exception { List resources = new ArrayList(); @@ -221,8 +250,24 @@ public class CollectionToCollectionConverterTests { assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources")))); } + + public ArrayList scalarListTarget; + + public List emptyListTarget; + + public LinkedList emptyListDifferentTarget; + + public List>> objectToCollection; + + public List strings; + + public List list = Collections.emptyList(); + + public Collection wildCardCollection = Collections.emptyList(); + public List resources; + public static abstract class BaseResource implements Resource { @Override @@ -286,39 +331,8 @@ public class CollectionToCollectionConverterTests { } } - public static class TestResource extends BaseResource { - - } - - @Test - public void convertEmptyVector_shouldReturnEmptyArrayList() { - Vector vector = new Vector(); - vector.add("Element"); - testCollectionConversionToArrayList(vector); - } - @Test - public void convertNonEmptyVector_shouldReturnNonEmptyArrayList() { - Vector vector = new Vector(); - 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 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 { } }