SPR-8714 Do not create copy in map-to-map and collection-to-collection conversion if not necessary

This commit is contained in:
Rossen Stoyanchev 2011-10-06 14:17:31 +00:00
parent 00a726b098
commit cb5954ed02
4 changed files with 21 additions and 12 deletions

View File

@ -57,6 +57,7 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert
if (source == null) { if (source == null) {
return null; return null;
} }
boolean isCopyRequired = !targetType.getType().isInstance(source);
Collection<?> sourceCollection = (Collection<?>) source; Collection<?> sourceCollection = (Collection<?>) source;
Collection<Object> target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size()); Collection<Object> target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size());
if (targetType.getElementTypeDescriptor() == null) { if (targetType.getElementTypeDescriptor() == null) {
@ -68,9 +69,12 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert
for (Object sourceElement : sourceCollection) { for (Object sourceElement : sourceCollection) {
Object targetElement = this.conversionService.convert(sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor()); Object targetElement = this.conversionService.convert(sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor());
target.add(targetElement); target.add(targetElement);
if (sourceElement != targetElement) {
isCopyRequired = true;
}
} }
} }
return target; return isCopyRequired ? target : source;
} }
} }

View File

@ -57,6 +57,7 @@ final class MapToMapConverter implements ConditionalGenericConverter {
if (source == null) { if (source == null) {
return null; return null;
} }
boolean isCopyRequired = !targetType.getType().isInstance(source);
Map<Object, Object> sourceMap = (Map<Object, Object>) source; Map<Object, Object> sourceMap = (Map<Object, Object>) source;
Map<Object, Object> targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); Map<Object, Object> targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size());
for (Map.Entry<Object, Object> entry : sourceMap.entrySet()) { for (Map.Entry<Object, Object> entry : sourceMap.entrySet()) {
@ -65,8 +66,11 @@ final class MapToMapConverter implements ConditionalGenericConverter {
Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor()); Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor());
Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor()); Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor());
targetMap.put(targetKey, targetValue); targetMap.put(targetKey, targetValue);
if (sourceKey != targetKey || sourceValue != targetValue) {
isCopyRequired = true;
}
} }
return targetMap; return isCopyRequired ? targetMap : sourceMap;
} }
// internal helpers // internal helpers

View File

@ -112,7 +112,7 @@ public class CollectionToCollectionConverterTests {
list.add(Arrays.asList("37", "23")); list.add(Arrays.asList("37", "23"));
conversionService.addConverter(new CollectionToObjectConverter(conversionService)); conversionService.addConverter(new CollectionToObjectConverter(conversionService));
assertTrue(conversionService.canConvert(List.class, List.class)); assertTrue(conversionService.canConvert(List.class, List.class));
assertEquals(list, conversionService.convert(list, List.class)); assertSame(list, conversionService.convert(list, List.class));
} }
@Test @Test
@ -175,7 +175,7 @@ public class CollectionToCollectionConverterTests {
resources.add(new FileSystemResource("test")); resources.add(new FileSystemResource("test"));
resources.add(new TestResource()); resources.add(new TestResource());
TypeDescriptor sourceType = TypeDescriptor.forObject(resources); TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources")))); assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
} }
@Test @Test
@ -186,7 +186,7 @@ public class CollectionToCollectionConverterTests {
resources.add(new FileSystemResource("test")); resources.add(new FileSystemResource("test"));
resources.add(new TestResource()); resources.add(new TestResource());
TypeDescriptor sourceType = TypeDescriptor.forObject(resources); TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources")))); assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
} }
@Test @Test
@ -195,7 +195,7 @@ public class CollectionToCollectionConverterTests {
resources.add(null); resources.add(null);
resources.add(null); resources.add(null);
TypeDescriptor sourceType = TypeDescriptor.forObject(resources); TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources")))); assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
} }
@Test(expected=ConverterNotFoundException.class) @Test(expected=ConverterNotFoundException.class)

View File

@ -2,6 +2,7 @@ package org.springframework.core.convert.support;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
@ -56,7 +57,7 @@ public class MapToMapConverterTests {
map.put("1", "9"); map.put("1", "9");
map.put("2", "37"); map.put("2", "37");
assertTrue(conversionService.canConvert(Map.class, Map.class)); assertTrue(conversionService.canConvert(Map.class, Map.class));
assertEquals(map, conversionService.convert(map, Map.class)); assertSame(map, conversionService.convert(map, Map.class));
} }
@Test @Test
@ -140,7 +141,7 @@ public class MapToMapConverterTests {
map.put("1", Arrays.asList("9", "12")); map.put("1", Arrays.asList("9", "12"));
map.put("2", Arrays.asList("37", "23")); map.put("2", Arrays.asList("37", "23"));
assertTrue(conversionService.canConvert(Map.class, Map.class)); assertTrue(conversionService.canConvert(Map.class, Map.class));
assertEquals(map, conversionService.convert(map, Map.class)); assertSame(map, conversionService.convert(map, Map.class));
} }
@Test @Test
@ -151,7 +152,7 @@ public class MapToMapConverterTests {
conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
conversionService.addConverter(new CollectionToObjectConverter(conversionService)); conversionService.addConverter(new CollectionToObjectConverter(conversionService));
assertTrue(conversionService.canConvert(Map.class, Map.class)); assertTrue(conversionService.canConvert(Map.class, Map.class));
assertEquals(map, conversionService.convert(map, Map.class)); assertSame(map, conversionService.convert(map, Map.class));
} }
@Test @Test
@ -160,7 +161,7 @@ public class MapToMapConverterTests {
TypeDescriptor sourceType = TypeDescriptor.forObject(map); TypeDescriptor sourceType = TypeDescriptor.forObject(map);
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget")); TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget"));
assertTrue(conversionService.canConvert(sourceType, targetType)); assertTrue(conversionService.canConvert(sourceType, targetType));
assertEquals(map, conversionService.convert(map, sourceType, targetType)); assertSame(map, conversionService.convert(map, sourceType, targetType));
} }
public Map<String, String> emptyMapTarget; public Map<String, String> emptyMapTarget;
@ -169,7 +170,7 @@ public class MapToMapConverterTests {
public void emptyMapNoTargetGenericInfo() throws Exception { public void emptyMapNoTargetGenericInfo() throws Exception {
Map<String, String> map = new HashMap<String, String>(); Map<String, String> map = new HashMap<String, String>();
assertTrue(conversionService.canConvert(Map.class, Map.class)); assertTrue(conversionService.canConvert(Map.class, Map.class));
assertEquals(map, conversionService.convert(map, Map.class)); assertSame(map, conversionService.convert(map, Map.class));
} }
@Test @Test
@ -185,5 +186,5 @@ public class MapToMapConverterTests {
} }
public LinkedHashMap<String, String> emptyMapDifferentTarget; public LinkedHashMap<String, String> emptyMapDifferentTarget;
} }