Skip to content

Commit 3f1fb4e

Browse files
philwebbphilwebb
committed
Merge pull request #155 from philwebb/SPR-9284
* SPR-9284: Allow MapToMap conversion to work when the target map does not have a default constructor (as long as a new map copy is not required). Polish trailing whitespace
2 parents ea8b132 + 38c4393 commit 3f1fb4e

File tree

2 files changed

+67
-20
lines changed

2 files changed

+67
-20
lines changed

‎spring-core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,9 @@
1616

1717
package org.springframework.core.convert.support;
1818

19+
import java.util.ArrayList;
1920
import java.util.Collections;
21+
import java.util.List;
2022
import java.util.Map;
2123
import java.util.Set;
2224

@@ -51,7 +53,7 @@ public Set<ConvertiblePair> getConvertibleTypes() {
5153
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
5254
return canConvertKey(sourceType, targetType) && canConvertValue(sourceType, targetType);
5355
}
54-
56+
5557
@SuppressWarnings("unchecked")
5658
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
5759
if (source == null) {
@@ -62,32 +64,39 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
6264
if (!copyRequired && sourceMap.isEmpty()) {
6365
return sourceMap;
6466
}
65-
Map<Object, Object> targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size());
67+
List<MapEntry> targetEntries = new ArrayList<MapEntry>(sourceMap.size());
6668
for (Map.Entry<Object, Object> entry : sourceMap.entrySet()) {
6769
Object sourceKey = entry.getKey();
6870
Object sourceValue = entry.getValue();
6971
Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor());
7072
Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor());
71-
targetMap.put(targetKey, targetValue);
73+
targetEntries.add(new MapEntry(targetKey, targetValue));
7274
if (sourceKey != targetKey || sourceValue != targetValue) {
7375
copyRequired = true;
7476
}
7577
}
76-
return (copyRequired ? targetMap : sourceMap);
78+
if(!copyRequired) {
79+
return sourceMap;
80+
}
81+
Map<Object, Object> targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size());
82+
for (MapEntry entry : targetEntries) {
83+
entry.addToMap(targetMap);
84+
}
85+
return targetMap;
7786
}
78-
87+
7988
// internal helpers
8089

8190
private boolean canConvertKey(TypeDescriptor sourceType, TypeDescriptor targetType) {
8291
return ConversionUtils.canConvertElements(sourceType.getMapKeyTypeDescriptor(),
8392
targetType.getMapKeyTypeDescriptor(), this.conversionService);
8493
}
85-
94+
8695
private boolean canConvertValue(TypeDescriptor sourceType, TypeDescriptor targetType) {
8796
return ConversionUtils.canConvertElements(sourceType.getMapValueTypeDescriptor(),
8897
targetType.getMapValueTypeDescriptor(), this.conversionService);
8998
}
90-
99+
91100
private Object convertKey(Object sourceKey, TypeDescriptor sourceType, TypeDescriptor targetType) {
92101
if (targetType == null) {
93102
return sourceKey;
@@ -102,4 +111,19 @@ private Object convertValue(Object sourceValue, TypeDescriptor sourceType, TypeD
102111
return this.conversionService.convert(sourceValue, sourceType.getMapValueTypeDescriptor(sourceValue), targetType);
103112
}
104113

114+
private static class MapEntry {
115+
116+
private Object key;
117+
private Object value;
118+
119+
public MapEntry(Object key, Object value) {
120+
this.key = key;
121+
this.value = value;
122+
}
123+
124+
public void addToMap(Map<Object, Object> map) {
125+
map.put(key, value);
126+
}
127+
}
128+
105129
}

‎spring-core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.core.convert.support;
1818

1919
import java.util.Arrays;
20+
import java.util.Collections;
2021
import java.util.HashMap;
2122
import java.util.LinkedHashMap;
2223
import java.util.List;
@@ -55,7 +56,7 @@ public void scalarMap() throws Exception {
5556
}
5657
conversionService.addConverterFactory(new StringToNumberConverterFactory());
5758
assertTrue(conversionService.canConvert(sourceType, targetType));
58-
@SuppressWarnings("unchecked")
59+
@SuppressWarnings("unchecked")
5960
Map<Integer, Integer> result = (Map<Integer, Integer>) conversionService.convert(map, sourceType, targetType);
6061
assertFalse(map.equals(result));
6162
assertEquals((Integer) 9, result.get(1));
@@ -79,7 +80,7 @@ public void scalarMapNotGenericSourceField() throws Exception {
7980
map.put("1", "9");
8081
map.put("2", "37");
8182
TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("notGenericMapSource"));
82-
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget"));
83+
TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget"));
8384
assertTrue(conversionService.canConvert(sourceType, targetType));
8485
try {
8586
conversionService.convert(map, sourceType, targetType);
@@ -88,15 +89,15 @@ public void scalarMapNotGenericSourceField() throws Exception {
8889
}
8990
conversionService.addConverterFactory(new StringToNumberConverterFactory());
9091
assertTrue(conversionService.canConvert(sourceType, targetType));
91-
@SuppressWarnings("unchecked")
92+
@SuppressWarnings("unchecked")
9293
Map<Integer, Integer> result = (Map<Integer, Integer>) conversionService.convert(map, sourceType, targetType);
9394
assertFalse(map.equals(result));
9495
assertEquals((Integer) 9, result.get(1));
95-
assertEquals((Integer) 37, result.get(2));
96+
assertEquals((Integer) 37, result.get(2));
9697
}
97-
98+
9899
public Map notGenericMapSource;
99-
100+
100101
@Test
101102
public void collectionMap() throws Exception {
102103
Map<String, List<String>> map = new HashMap<String, List<String>>();
@@ -109,11 +110,11 @@ public void collectionMap() throws Exception {
109110
conversionService.convert(map, sourceType, targetType);
110111
} catch (ConversionFailedException e) {
111112
assertTrue(e.getCause() instanceof ConverterNotFoundException);
112-
}
113+
}
113114
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
114115
conversionService.addConverterFactory(new StringToNumberConverterFactory());
115116
assertTrue(conversionService.canConvert(sourceType, targetType));
116-
@SuppressWarnings("unchecked")
117+
@SuppressWarnings("unchecked")
117118
Map<Integer, List<Integer>> result = (Map<Integer, List<Integer>>) conversionService.convert(map, sourceType, targetType);
118119
assertFalse(map.equals(result));
119120
assertEquals(Arrays.asList(9, 12), result.get(1));
@@ -134,12 +135,12 @@ public void collectionMapSourceTarget() throws Exception {
134135
conversionService.convert(map, sourceType, targetType);
135136
fail("Should have failed");
136137
} catch (ConverterNotFoundException e) {
137-
138+
138139
}
139140
conversionService.addConverter(new CollectionToCollectionConverter(conversionService));
140141
conversionService.addConverterFactory(new StringToNumberConverterFactory());
141142
assertTrue(conversionService.canConvert(sourceType, targetType));
142-
@SuppressWarnings("unchecked")
143+
@SuppressWarnings("unchecked")
143144
Map<Integer, List<Integer>> result = (Map<Integer, List<Integer>>) conversionService.convert(map, sourceType, targetType);
144145
assertFalse(map.equals(result));
145146
assertEquals(Arrays.asList(9, 12), result.get(1));
@@ -167,7 +168,7 @@ public void collectionMapNotGenericTargetCollectionToObjectInteraction() throws
167168
assertTrue(conversionService.canConvert(Map.class, Map.class));
168169
assertSame(map, conversionService.convert(map, Map.class));
169170
}
170-
171+
171172
@Test
172173
public void emptyMap() throws Exception {
173174
Map<String, String> map = new HashMap<String, String>();
@@ -200,4 +201,26 @@ public void emptyMapDifferentTargetImplType() throws Exception {
200201

201202
public LinkedHashMap<String, String> emptyMapDifferentTarget;
202203

204+
@Test
205+
public void noDefaultConstructorCopyNotRequired() throws Exception {
206+
// SPR-9284
207+
NoDefaultConstructorMap<String, Integer> map = new NoDefaultConstructorMap<String,Integer>(
208+
Collections.<String, Integer> singletonMap("1", 1));
209+
TypeDescriptor sourceType = TypeDescriptor.map(NoDefaultConstructorMap.class,
210+
TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class));
211+
TypeDescriptor targetType = TypeDescriptor.map(NoDefaultConstructorMap.class,
212+
TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class));
213+
assertTrue(conversionService.canConvert(sourceType, targetType));
214+
@SuppressWarnings("unchecked")
215+
Map<String, Integer> result = (Map<String, Integer>) conversionService.convert(map, sourceType, targetType);
216+
assertEquals(map, result);
217+
assertEquals(NoDefaultConstructorMap.class, result.getClass());
218+
}
219+
220+
public static class NoDefaultConstructorMap<K, V> extends HashMap<K, V> {
221+
public NoDefaultConstructorMap(Map<? extends K, ? extends V> m) {
222+
super(m);
223+
}
224+
}
225+
203226
}

0 commit comments

Comments
 (0)