From cb5954ed02e748f31d3ad342f696fb4a702ebc66 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 6 Oct 2011 14:17:31 +0000 Subject: [PATCH] SPR-8714 Do not create copy in map-to-map and collection-to-collection conversion if not necessary --- .../support/CollectionToCollectionConverter.java | 6 +++++- .../core/convert/support/MapToMapConverter.java | 6 +++++- .../CollectionToCollectionConverterTests.java | 8 ++++---- .../convert/support/MapToMapConverterTests.java | 13 +++++++------ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java index 22af221e2ba..72e2480ace1 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java @@ -57,6 +57,7 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert if (source == null) { return null; } + boolean isCopyRequired = !targetType.getType().isInstance(source); Collection sourceCollection = (Collection) source; Collection target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size()); if (targetType.getElementTypeDescriptor() == null) { @@ -68,9 +69,12 @@ final class CollectionToCollectionConverter implements ConditionalGenericConvert for (Object sourceElement : sourceCollection) { Object targetElement = this.conversionService.convert(sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor()); target.add(targetElement); + if (sourceElement != targetElement) { + isCopyRequired = true; + } } } - return target; + return isCopyRequired ? target : source; } } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java index 624f5887a0d..284073ae4a1 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java @@ -57,6 +57,7 @@ final class MapToMapConverter implements ConditionalGenericConverter { if (source == null) { return null; } + boolean isCopyRequired = !targetType.getType().isInstance(source); Map sourceMap = (Map) source; Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); for (Map.Entry entry : sourceMap.entrySet()) { @@ -65,8 +66,11 @@ final class MapToMapConverter implements ConditionalGenericConverter { Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor()); Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor()); targetMap.put(targetKey, targetValue); + if (sourceKey != targetKey || sourceValue != targetValue) { + isCopyRequired = true; + } } - return targetMap; + return isCopyRequired ? targetMap : sourceMap; } // internal helpers diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java index 85061cdbd26..e45c18356d5 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java @@ -112,7 +112,7 @@ public class CollectionToCollectionConverterTests { list.add(Arrays.asList("37", "23")); conversionService.addConverter(new CollectionToObjectConverter(conversionService)); assertTrue(conversionService.canConvert(List.class, List.class)); - assertEquals(list, conversionService.convert(list, List.class)); + assertSame(list, conversionService.convert(list, List.class)); } @Test @@ -175,7 +175,7 @@ public class CollectionToCollectionConverterTests { resources.add(new FileSystemResource("test")); resources.add(new TestResource()); 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 @@ -186,7 +186,7 @@ public class CollectionToCollectionConverterTests { resources.add(new FileSystemResource("test")); resources.add(new TestResource()); 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 @@ -195,7 +195,7 @@ public class CollectionToCollectionConverterTests { resources.add(null); resources.add(null); 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) diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java index 73555dfe0c7..420da3531ac 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java @@ -2,6 +2,7 @@ package org.springframework.core.convert.support; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -56,7 +57,7 @@ public class MapToMapConverterTests { map.put("1", "9"); map.put("2", "37"); assertTrue(conversionService.canConvert(Map.class, Map.class)); - assertEquals(map, conversionService.convert(map, Map.class)); + assertSame(map, conversionService.convert(map, Map.class)); } @Test @@ -140,7 +141,7 @@ public class MapToMapConverterTests { map.put("1", Arrays.asList("9", "12")); map.put("2", Arrays.asList("37", "23")); assertTrue(conversionService.canConvert(Map.class, Map.class)); - assertEquals(map, conversionService.convert(map, Map.class)); + assertSame(map, conversionService.convert(map, Map.class)); } @Test @@ -151,7 +152,7 @@ public class MapToMapConverterTests { conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); conversionService.addConverter(new CollectionToObjectConverter(conversionService)); assertTrue(conversionService.canConvert(Map.class, Map.class)); - assertEquals(map, conversionService.convert(map, Map.class)); + assertSame(map, conversionService.convert(map, Map.class)); } @Test @@ -160,7 +161,7 @@ public class MapToMapConverterTests { TypeDescriptor sourceType = TypeDescriptor.forObject(map); TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget")); assertTrue(conversionService.canConvert(sourceType, targetType)); - assertEquals(map, conversionService.convert(map, sourceType, targetType)); + assertSame(map, conversionService.convert(map, sourceType, targetType)); } public Map emptyMapTarget; @@ -169,7 +170,7 @@ public class MapToMapConverterTests { public void emptyMapNoTargetGenericInfo() throws Exception { Map map = new HashMap(); assertTrue(conversionService.canConvert(Map.class, Map.class)); - assertEquals(map, conversionService.convert(map, Map.class)); + assertSame(map, conversionService.convert(map, Map.class)); } @Test @@ -185,5 +186,5 @@ public class MapToMapConverterTests { } public LinkedHashMap emptyMapDifferentTarget; - + }