Skip to content

Commit cb5954e

Browse files
committed
SPR-8714 Do not create copy in map-to-map and collection-to-collection conversion if not necessary
1 parent 00a726b commit cb5954e

File tree

4 files changed

+21
-12
lines changed

4 files changed

+21
-12
lines changed

org.springframework.core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
5757
if (source == null) {
5858
return null;
5959
}
60+
boolean isCopyRequired = !targetType.getType().isInstance(source);
6061
Collection<?> sourceCollection = (Collection<?>) source;
6162
Collection<Object> target = CollectionFactory.createCollection(targetType.getType(), sourceCollection.size());
6263
if (targetType.getElementTypeDescriptor() == null) {
@@ -68,9 +69,12 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
6869
for (Object sourceElement : sourceCollection) {
6970
Object targetElement = this.conversionService.convert(sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType.getElementTypeDescriptor());
7071
target.add(targetElement);
72+
if (sourceElement != targetElement) {
73+
isCopyRequired = true;
74+
}
7175
}
7276
}
73-
return target;
77+
return isCopyRequired ? target : source;
7478
}
7579

7680
}

org.springframework.core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
5757
if (source == null) {
5858
return null;
5959
}
60+
boolean isCopyRequired = !targetType.getType().isInstance(source);
6061
Map<Object, Object> sourceMap = (Map<Object, Object>) source;
6162
Map<Object, Object> targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size());
6263
for (Map.Entry<Object, Object> entry : sourceMap.entrySet()) {
@@ -65,8 +66,11 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
6566
Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor());
6667
Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor());
6768
targetMap.put(targetKey, targetValue);
69+
if (sourceKey != targetKey || sourceValue != targetValue) {
70+
isCopyRequired = true;
71+
}
6872
}
69-
return targetMap;
73+
return isCopyRequired ? targetMap : sourceMap;
7074
}
7175

7276
// internal helpers

org.springframework.core/src/test/java/org/springframework/core/convert/support/CollectionToCollectionConverterTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void collectionToObjectInteraction() throws Exception {
112112
list.add(Arrays.asList("37", "23"));
113113
conversionService.addConverter(new CollectionToObjectConverter(conversionService));
114114
assertTrue(conversionService.canConvert(List.class, List.class));
115-
assertEquals(list, conversionService.convert(list, List.class));
115+
assertSame(list, conversionService.convert(list, List.class));
116116
}
117117

118118
@Test
@@ -175,7 +175,7 @@ public void differentImpls() throws Exception {
175175
resources.add(new FileSystemResource("test"));
176176
resources.add(new TestResource());
177177
TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
178-
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
178+
assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
179179
}
180180

181181
@Test
@@ -186,7 +186,7 @@ public void mixedInNulls() throws Exception {
186186
resources.add(new FileSystemResource("test"));
187187
resources.add(new TestResource());
188188
TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
189-
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
189+
assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
190190
}
191191

192192
@Test
@@ -195,7 +195,7 @@ public void allNulls() throws Exception {
195195
resources.add(null);
196196
resources.add(null);
197197
TypeDescriptor sourceType = TypeDescriptor.forObject(resources);
198-
assertEquals(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
198+
assertSame(resources, conversionService.convert(resources, sourceType, new TypeDescriptor(getClass().getField("resources"))));
199199
}
200200

201201
@Test(expected=ConverterNotFoundException.class)

org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.junit.Assert.assertEquals;
44
import static org.junit.Assert.assertFalse;
5+
import static org.junit.Assert.assertSame;
56
import static org.junit.Assert.assertTrue;
67
import static org.junit.Assert.fail;
78

@@ -56,7 +57,7 @@ public void scalarMapNotGenericTarget() throws Exception {
5657
map.put("1", "9");
5758
map.put("2", "37");
5859
assertTrue(conversionService.canConvert(Map.class, Map.class));
59-
assertEquals(map, conversionService.convert(map, Map.class));
60+
assertSame(map, conversionService.convert(map, Map.class));
6061
}
6162

6263
@Test
@@ -140,7 +141,7 @@ public void collectionMapNotGenericTarget() throws Exception {
140141
map.put("1", Arrays.asList("9", "12"));
141142
map.put("2", Arrays.asList("37", "23"));
142143
assertTrue(conversionService.canConvert(Map.class, Map.class));
143-
assertEquals(map, conversionService.convert(map, Map.class));
144+
assertSame(map, conversionService.convert(map, Map.class));
144145
}
145146

146147
@Test
@@ -151,7 +152,7 @@ public void collectionMapNotGenericTargetCollectionToObjectInteraction() throws
151152
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
152153
conversionService.addConverter(new CollectionToObjectConverter(conversionService));
153154
assertTrue(conversionService.canConvert(Map.class, Map.class));
154-
assertEquals(map, conversionService.convert(map, Map.class));
155+
assertSame(map, conversionService.convert(map, Map.class));
155156
}
156157

157158
@Test
@@ -160,7 +161,7 @@ public void emptyMap() throws Exception {
160161
TypeDescriptor sourceType = TypeDescriptor.forObject(map);
161162
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget"));
162163
assertTrue(conversionService.canConvert(sourceType, targetType));
163-
assertEquals(map, conversionService.convert(map, sourceType, targetType));
164+
assertSame(map, conversionService.convert(map, sourceType, targetType));
164165
}
165166

166167
public Map<String, String> emptyMapTarget;
@@ -169,7 +170,7 @@ public void emptyMap() throws Exception {
169170
public void emptyMapNoTargetGenericInfo() throws Exception {
170171
Map<String, String> map = new HashMap<String, String>();
171172
assertTrue(conversionService.canConvert(Map.class, Map.class));
172-
assertEquals(map, conversionService.convert(map, Map.class));
173+
assertSame(map, conversionService.convert(map, Map.class));
173174
}
174175

175176
@Test
@@ -185,5 +186,5 @@ public void emptyMapDifferentTargetImplType() throws Exception {
185186
}
186187

187188
public LinkedHashMap<String, String> emptyMapDifferentTarget;
188-
189+
189190
}

0 commit comments

Comments
 (0)