From 4d6a5849f70288b5f5c19030b1fd8f087a6cb90b Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Sun, 22 May 2011 19:10:40 +0000 Subject: [PATCH] SPR-8364 --- .../core/convert/TypeDescriptor.java | 54 +++++-- .../convert/support/MapToMapConverter.java | 27 ++-- .../GenericConversionServiceTests.java | 29 ---- .../support/MapToMapConverterTests.java | 145 ++++++++++++++++++ 4 files changed, 203 insertions(+), 52 deletions(-) create mode 100644 org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index d2b2efd5511..0d20faea4e1 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java @@ -343,24 +343,33 @@ public class TypeDescriptor { } /** - * Create a copy of this nested type descriptor and apply the specific type information from the indexed object. + * Create a copy of this nested type descriptor and apply the specific type information from the indexed value, if necessary. * Used to support collection and map indexing scenarios, where the indexer has a reference to the indexed type descriptor but needs to ensure its type actually represents the indexed object type. - * This is necessary to support type conversion during index object binding operations. + * This is necessary to support type conversion during collection and map binding operations where generic information may not be available. */ public TypeDescriptor applyIndexedObject(Object object) { if (object == null) { return this; } - // TODO preserve binding context with returned copy - // TODO fall back to generic info if collection is empty - if (object instanceof Collection) { - return new TypeDescriptor(object.getClass(), CollectionUtils.findCommonElementType((Collection) object)); + if (isCollection() && Object.class.equals(getElementType())) { + Collection collection = (Collection) object; + if (collection.size() > 0) { + return new TypeDescriptor(object.getClass(), CollectionUtils.findCommonElementType((Collection) object), methodParameter, field, fieldNestingLevel, annotations); + } else { + return this; + } } - else if (object instanceof Map) { - return new TypeDescriptor(object.getClass(), CollectionUtils.findCommonElementType(((Map) object).keySet()), CollectionUtils.findCommonElementType(((Map) object).values())); + else if (isMap() && Object.class.equals(getMapKeyType()) && Object.class.equals(getMapValueType())) { + Map map = (Map) object; + if (map.size() > 0) { + return new TypeDescriptor(object.getClass(), CollectionUtils.findCommonElementType(((Map) object).keySet()), + CollectionUtils.findCommonElementType(((Map) object).values()), methodParameter, field, fieldNestingLevel, annotations); + } else { + return this; + } } else { - return valueOf(object.getClass()); + return this; } } @@ -487,6 +496,7 @@ public class TypeDescriptor { } } + @SuppressWarnings("unchecked") private Class resolveCollectionElementType() { if (this.methodParameter != null) { return GenericCollectionTypeResolver.getCollectionParameterType(this.methodParameter); @@ -495,10 +505,11 @@ public class TypeDescriptor { return GenericCollectionTypeResolver.getCollectionFieldType(this.field, this.fieldNestingLevel); } else { - return GenericCollectionTypeResolver.getCollectionType((Class) this.type); + return GenericCollectionTypeResolver.getCollectionType((Class>) this.type); } } + @SuppressWarnings("unchecked") private Class resolveMapKeyType() { if (this.methodParameter != null) { return GenericCollectionTypeResolver.getMapKeyParameterType(this.methodParameter); @@ -507,10 +518,11 @@ public class TypeDescriptor { return GenericCollectionTypeResolver.getMapKeyFieldType(this.field, this.fieldNestingLevel); } else { - return GenericCollectionTypeResolver.getMapKeyType((Class) this.type); + return GenericCollectionTypeResolver.getMapKeyType((Class>) this.type); } } + @SuppressWarnings("unchecked") private Class resolveMapValueType() { if (this.methodParameter != null) { return GenericCollectionTypeResolver.getMapValueParameterType(this.methodParameter); @@ -519,7 +531,7 @@ public class TypeDescriptor { return GenericCollectionTypeResolver.getMapValueFieldType(this.field, this.fieldNestingLevel); } else { - return GenericCollectionTypeResolver.getMapValueType((Class) this.type); + return GenericCollectionTypeResolver.getMapValueType((Class>) this.type); } } @@ -550,7 +562,7 @@ public class TypeDescriptor { } this.elementType = TypeDescriptor.valueOf(elementType); } - + private TypeDescriptor(Class mapType, Class keyType, Class valueType) { this.type = mapType; if (keyType == null) { @@ -563,6 +575,22 @@ public class TypeDescriptor { this.mapValueType = TypeDescriptor.valueOf(valueType); } + private TypeDescriptor(Class collectionType, Class elementType, MethodParameter methodParameter, Field field, int fieldNestingLevel, Annotation[] annotations) { + this(collectionType, elementType); + this.methodParameter = methodParameter; + this.field = field; + this.fieldNestingLevel = fieldNestingLevel; + this.annotations = annotations; + } + + private TypeDescriptor(Class mapType, Class keyType, Class valueType, MethodParameter methodParameter, Field field, int fieldNestingLevel, Annotation[] annotations) { + this(mapType, keyType, valueType); + this.methodParameter = methodParameter; + this.field = field; + this.fieldNestingLevel = fieldNestingLevel; + this.annotations = annotations; + } + private TypeDescriptor(Class nestedType, Field field, int nestingLevel) { this.type = nestedType; this.field = field; 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 f8db9c9b0cb..1c71cb88448 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 @@ -66,17 +66,24 @@ final class MapToMapConverter implements ConditionalGenericConverter { if (source == null) { return null; } - Map sourceMap = (Map) source; - Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); - for (Object entry : sourceMap.entrySet()) { - Map.Entry sourceMapEntry = (Map.Entry) entry; - Object sourceKey = sourceMapEntry.getKey(); - Object sourceValue = sourceMapEntry.getValue(); - Object targetKey = this.conversionService.convert(sourceKey, sourceType.getMapKeyTypeDescriptor().applyIndexedObject(sourceKey), targetType.getMapKeyTypeDescriptor()); - Object targetValue = this.conversionService.convert(sourceValue, sourceType.getMapValueTypeDescriptor().applyIndexedObject(sourceValue), targetType.getMapValueTypeDescriptor()); - targetMap.put(targetKey, targetValue); + Map sourceMap = (Map) source; + Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); + TypeDescriptor targetKeyType = targetType.getMapKeyTypeDescriptor(); + TypeDescriptor targetValueType = targetType.getMapValueTypeDescriptor(); + if (Object.class.equals(targetKeyType.getType()) && Object.class.equals(targetValueType.getType())) { + for (Map.Entry entry : sourceMap.entrySet()) { + targetMap.put(entry.getKey(), entry.getValue()); + } + } else { + for (Map.Entry entry : sourceMap.entrySet()) { + Object sourceKey = entry.getKey(); + Object sourceValue = entry.getValue(); + Object targetKey = this.conversionService.convert(sourceKey, sourceType.getMapKeyTypeDescriptor().applyIndexedObject(sourceKey), targetType.getMapKeyTypeDescriptor()); + Object targetValue = this.conversionService.convert(sourceValue, sourceType.getMapValueTypeDescriptor().applyIndexedObject(sourceValue), targetType.getMapValueTypeDescriptor()); + targetMap.put(targetKey, targetValue); + } } return targetMap; } -} +} \ No newline at end of file diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java index 94e830fd3df..d7b8e3689d9 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java @@ -412,35 +412,6 @@ public class GenericConversionServiceTests { assertNull(conversionService.convert(list, sourceType, targetType)); } - @Test - public void emptyMapToMap() throws Exception { - conversionService.addConverter(new MapToMapConverter(conversionService)); - conversionService.addConverterFactory(new StringToNumberConverterFactory()); - Map map = new HashMap(); - TypeDescriptor sourceType = TypeDescriptor.forObject(map); - TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget")); - assertTrue(conversionService.canConvert(sourceType, targetType)); - assertEquals(map, conversionService.convert(map, sourceType, targetType)); - } - - public Map emptyMapTarget; - - @Test - public void emptyMapToMapDifferentTargetType() throws Exception { - conversionService.addConverter(new MapToMapConverter(conversionService)); - conversionService.addConverterFactory(new StringToNumberConverterFactory()); - Map map = new HashMap(); - TypeDescriptor sourceType = TypeDescriptor.forObject(map); - TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapDifferentTarget")); - assertTrue(conversionService.canConvert(sourceType, targetType)); - @SuppressWarnings("unchecked") - LinkedHashMap result = (LinkedHashMap) conversionService.convert(map, sourceType, targetType); - assertEquals(map, result); - assertEquals(LinkedHashMap.class, result.getClass()); - } - - public LinkedHashMap emptyMapDifferentTarget; - private interface MyBaseInterface { } 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 new file mode 100644 index 00000000000..7876f7169f8 --- /dev/null +++ b/org.springframework.core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java @@ -0,0 +1,145 @@ +package org.springframework.core.convert.support; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.core.convert.TypeDescriptor; + +public class MapToMapConverterTests { + + private GenericConversionService conversionService = new GenericConversionService(); + + @Before + public void setUp() { + conversionService.addConverter(new MapToMapConverter(conversionService)); + } + + @Test + public void scalarMap() throws Exception { + Map map = new HashMap(); + map.put("1", "9"); + map.put("2", "37"); + TypeDescriptor sourceType = TypeDescriptor.forObject(map); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget")); + assertFalse(conversionService.canConvert(sourceType, targetType)); + conversionService.addConverterFactory(new StringToNumberConverterFactory()); + assertTrue(conversionService.canConvert(sourceType, targetType)); + @SuppressWarnings("unchecked") + Map result = (Map) conversionService.convert(map, sourceType, targetType); + assertFalse(map.equals(result)); + assertEquals((Integer) 9, result.get(1)); + assertEquals((Integer) 37, result.get(2)); + } + + public Map scalarMapTarget; + + @Test + public void scalarMapNotGenericTarget() throws Exception { + Map map = new HashMap(); + map.put("1", "9"); + map.put("2", "37"); + assertTrue(conversionService.canConvert(Map.class, Map.class)); + assertEquals(map, conversionService.convert(map, Map.class)); + } + + @Test + public void collectionMap() throws Exception { + Map> map = new HashMap>(); + map.put("1", Arrays.asList("9", "12")); + map.put("2", Arrays.asList("37", "23")); + TypeDescriptor sourceType = TypeDescriptor.forObject(map); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("collectionMapTarget")); + assertFalse(conversionService.canConvert(sourceType, targetType)); + conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); + conversionService.addConverterFactory(new StringToNumberConverterFactory()); + assertTrue(conversionService.canConvert(sourceType, targetType)); + @SuppressWarnings("unchecked") + Map> result = (Map>) conversionService.convert(map, sourceType, targetType); + assertFalse(map.equals(result)); + assertEquals(Arrays.asList(9, 12), result.get(1)); + assertEquals(Arrays.asList(37, 23), result.get(2)); + } + + public Map> collectionMapTarget; + + public Map> sourceCollectionMapTarget; + + @Test + public void collectionMapSourceTarget() throws Exception { + Map> map = new HashMap>(); + map.put("1", Arrays.asList("9", "12")); + map.put("2", Arrays.asList("37", "23")); + TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("sourceCollectionMapTarget")); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("collectionMapTarget")); + assertFalse(conversionService.canConvert(sourceType, targetType)); + conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); + conversionService.addConverterFactory(new StringToNumberConverterFactory()); + assertTrue(conversionService.canConvert(sourceType, targetType)); + @SuppressWarnings("unchecked") + Map> result = (Map>) conversionService.convert(map, sourceType, targetType); + assertFalse(map.equals(result)); + assertEquals(Arrays.asList(9, 12), result.get(1)); + assertEquals(Arrays.asList(37, 23), result.get(2)); + } + + @Test + public void collectionMapNotGenericTarget() throws Exception { + Map> map = new HashMap>(); + 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)); + } + + @Test + public void collectionMapNotGenericTargetCollectionToObjectInteraction() throws Exception { + Map> map = new HashMap>(); + map.put("1", Arrays.asList("9", "12")); + map.put("2", Arrays.asList("37", "23")); + conversionService.addConverter(new CollectionToObjectConverter(conversionService)); + assertTrue(conversionService.canConvert(Map.class, Map.class)); + assertEquals(map, conversionService.convert(map, Map.class)); + } + + @Test + public void emptyMap() throws Exception { + Map map = new HashMap(); + TypeDescriptor sourceType = TypeDescriptor.forObject(map); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapTarget")); + assertTrue(conversionService.canConvert(sourceType, targetType)); + assertEquals(map, conversionService.convert(map, sourceType, targetType)); + } + + public Map emptyMapTarget; + + @Test + public void emptyMapNoTargetGenericInfo() throws Exception { + Map map = new HashMap(); + assertTrue(conversionService.canConvert(Map.class, Map.class)); + assertEquals(map, conversionService.convert(map, Map.class)); + } + + @Test + public void emptyMapDifferentTargetImplType() throws Exception { + Map map = new HashMap(); + TypeDescriptor sourceType = TypeDescriptor.forObject(map); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("emptyMapDifferentTarget")); + assertTrue(conversionService.canConvert(sourceType, targetType)); + @SuppressWarnings("unchecked") + LinkedHashMap result = (LinkedHashMap) conversionService.convert(map, sourceType, targetType); + assertEquals(map, result); + assertEquals(LinkedHashMap.class, result.getClass()); + } + + public LinkedHashMap emptyMapDifferentTarget; + +}