map-to-map tests

org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java

@@ -36,10 +36,10 @@ import org.springframework.util.Assert;
  */
 public class TypeDescriptor {
 
-    /**
+	/**
-     * constant value typeDescriptor for the type of a null value
+	 * constant value typeDescriptor for the type of a null value
-     */	
+	 */
-	public final static TypeDescriptor NULL_TYPE_DESCRIPTOR = new TypeDescriptor((Class<?>)null);
+	public final static TypeDescriptor NULL_TYPE_DESCRIPTOR = new TypeDescriptor((Class<?>) null);
 
 	private MethodParameter methodParameter;
 
@@ -48,19 +48,19 @@ public class TypeDescriptor {
 	private Annotation[] cachedFieldAnnotations;
 
 	private Class<?> type;
-	
+
 	/**
-	 * Creates a new descriptor for the given type.
+	 * Creates a new descriptor for the given type. Use this constructor when a bound value comes from a source such as
-	 * Use this constructor when a bound value comes from a source such as a Map or collection, where no additional binding metadata is available.
+	 * a Map or collection, where no additional binding metadata is available.
 	 * @param type the actual type
 	 */
 	public TypeDescriptor(Class<?> type) {
 		this.type = type;
 	}
-	
+
 	/**
-	 * Create a new descriptor for a method or constructor parameter.
+	 * Create a new descriptor for a method or constructor parameter. Use this constructor when a bound value originates
-	 * Use this constructor when a bound value originates from a method parameter, such as a setter method argument.
+	 * from a method parameter, such as a setter method argument.
 	 * @param methodParameter the MethodParameter to wrap
 	 */
 	public TypeDescriptor(MethodParameter methodParameter) {
@@ -69,8 +69,7 @@ public class TypeDescriptor {
 	}
 
 	/**
-	 * Create a new descriptor for a field.
+	 * Create a new descriptor for a field. Use this constructor when a bound value originates from a field.
-	 * Use this constructor when a bound value originates from a field.
 	 * @param field the field to wrap
 	 */
 	public TypeDescriptor(Field field) {
@@ -88,7 +87,7 @@ public class TypeDescriptor {
 			return type;
 		} else if (field != null) {
 			return field.getType();
-		} else if (methodParameter!=null) {
+		} else if (methodParameter != null) {
 			return methodParameter.getParameterType();
 		} else {
 			return null;
@@ -101,6 +100,9 @@ public class TypeDescriptor {
 	 */
 	public Class<?> getWrapperTypeIfPrimitive() {
 		Class<?> type = getType();
+		if (type == null) {
+			return null;
+		}
 		if (type.isPrimitive()) {
 			if (type.equals(int.class)) {
 				return Integer.class;
@@ -125,12 +127,17 @@ public class TypeDescriptor {
 			return type;
 		}
 	}
-	
+
 	/**
 	 * Returns the name of this type; the fully qualified classname.
 	 */
 	public String getName() {
-		return getType().getName();
+		Class<?> type = getType();
+		if (type != null) {
+			return getType().getName();
+		} else {
+			return null;
+		}
 	}
 
 	/**
@@ -138,26 +145,23 @@ public class TypeDescriptor {
 	 */
 	public boolean isArray() {
 		Class<?> type = getType();
-		return (type==null?false:type.isArray());
+		if (type != null) {
+			return type.isArray();
+		} else {
+			return false;
+		}
 	}
 
 	/**
 	 * Is this type a {@link Collection} type?
 	 */
 	public boolean isCollection() {
-		return Collection.class.isAssignableFrom(getType());
+		return isTypeAssignableTo(Collection.class);
 	}
 
 	/**
-	 * Is this type a {@link Map} type?
+	 * If this type is an array type or {@link Collection} type, returns the underlying element type. Returns null if
-	 */
+	 * the type is neither an array or collection.
-	public boolean isMap() {
-		return Map.class.isAssignableFrom(getType());
-	}
-
-	/**
-	 * If this type is an array type or {@link Collection} type, returns the underlying element type.
-	 * Returns null if the type is neither an array or collection.
 	 */
 	public Class<?> getElementType() {
 		if (isArray()) {
@@ -168,15 +172,34 @@ public class TypeDescriptor {
 			return null;
 		}
 	}
+
+	/**
+	 * Is this type a {@link Map} type?
+	 */
+	public boolean isMap() {
+		return isTypeAssignableTo(Map.class);
+	}
 	
+	/**
+	 * Is this descriptor for a map where the key type and value type are known? 
+	 */
+	public boolean isMapEntryTypeKnown() {
+		return isMap() && getMapKeyType() != null && getMapValueType() != null;
+	}
+
 	/**
 	 * Determine the generic key type of the wrapped Map parameter/field, if any.
 	 * 
 	 * @return the generic type, or <code>null</code> if none
 	 */
 	public Class<?> getMapKeyType() {
-		return (field != null ? GenericCollectionTypeResolver.getMapKeyFieldType(field) : GenericCollectionTypeResolver
+		if (field != null) {
-				.getMapKeyParameterType(methodParameter));
+			return GenericCollectionTypeResolver.getMapKeyFieldType(field);
+		} else if (methodParameter != null) {
+			return GenericCollectionTypeResolver.getMapKeyParameterType(methodParameter);
+		} else {
+			return null;
+		}
 	}
 
 	/**
@@ -185,8 +208,13 @@ public class TypeDescriptor {
 	 * @return the generic type, or <code>null</code> if none
 	 */
 	public Class<?> getMapValueType() {
-		return (field != null ? GenericCollectionTypeResolver.getMapValueFieldType(field)
+		if (field != null) {
-				: GenericCollectionTypeResolver.getMapValueParameterType(methodParameter));
+			return GenericCollectionTypeResolver.getMapValueFieldType(field);
+		} else if (methodParameter != null) {
+			return GenericCollectionTypeResolver.getMapValueParameterType(methodParameter);
+		} else {
+			return null;
+		}
 	}
 
 	/**
@@ -198,8 +226,10 @@ public class TypeDescriptor {
 				cachedFieldAnnotations = field.getAnnotations();
 			}
 			return cachedFieldAnnotations;
-		} else {
+		} else if (methodParameter != null) {
 			return methodParameter.getParameterAnnotations();
+		} else {
+			return null;
 		}
 	}
 
@@ -229,14 +259,24 @@ public class TypeDescriptor {
 	 * Returns true if this type is an abstract class.
 	 */
 	public boolean isAbstractClass() {
-		return !getType().isInterface() && Modifier.isAbstract(getType().getModifiers());
+		Class<?> type = getType();
+		if (type != null) {
+			return !getType().isInterface() && Modifier.isAbstract(getType().getModifiers());			
+		} else {
+			return false;
+		}
 	}
 
 	/**
 	 * Is the obj an instance of this type?
 	 */
 	public boolean isInstance(Object obj) {
-		return getType().isInstance(obj);
+		Class<?> type = getType();
+		if (type != null) {
+			return getType().isInstance(obj);
+		} else {
+			return false;
+		}
 	}
 
 	/**
@@ -247,7 +287,6 @@ public class TypeDescriptor {
 	public boolean isAssignableTo(TypeDescriptor targetType) {
 		return targetType.getType().isAssignableFrom(getType());
 	}
-	
 
 	/**
 	 * Creates a new type descriptor for the given class.
@@ -265,13 +304,13 @@ public class TypeDescriptor {
 	 * @return the type descriptor
 	 */
 	public static TypeDescriptor forObject(Object object) {
-		if (object==null) {
+		if (object == null) {
 			return NULL_TYPE_DESCRIPTOR;
 		} else {
 			return valueOf(object.getClass());
 		}
 	}
-	
+
 	/**
 	 * @return a textual representation of the type descriptor (eg. Map<String,Foo>) for use in messages
 	 */
@@ -288,13 +327,13 @@ public class TypeDescriptor {
 			stringValue.append(clazz.getName());
 			if (isCollection()) {
 				Class<?> collectionType = getCollectionElementType();
-				if (collectionType!=null) {
+				if (collectionType != null) {
 					stringValue.append("<").append(collectionType.getName()).append(">");
 				}
 			} else if (isMap()) {
 				Class<?> keyType = getMapKeyType();
 				Class<?> valType = getMapValueType();
-				if (keyType!=null && valType!=null) {
+				if (keyType != null && valType != null) {
 					stringValue.append("<").append(keyType.getName()).append(",");
 					stringValue.append(valType).append(">");
 				}
@@ -303,20 +342,29 @@ public class TypeDescriptor {
 		return stringValue.toString();
 	}
 
-
 	// internal helpers
-	
+
 	private Class<?> getArrayComponentType() {
 		return getType().getComponentType();
 	}
-	
+
+	@SuppressWarnings("unchecked")
 	private Class<?> getCollectionElementType() {
 		if (type != null) {
 			return GenericCollectionTypeResolver.getCollectionType((Class<? extends Collection>) type);
 		} else if (field != null) {
 			return GenericCollectionTypeResolver.getCollectionFieldType(field);
 		} else {
-			return  GenericCollectionTypeResolver.getCollectionParameterType(methodParameter);
+			return GenericCollectionTypeResolver.getCollectionParameterType(methodParameter);
+		}
+	}
+	
+	private boolean isTypeAssignableTo(Class<?> clazz) {
+		Class<?> type = getType();
+		if (type != null) {
+			return clazz.isAssignableFrom(type);
+		} else {
+			return false;
 		}
 	}
 

org.springframework.core/src/main/java/org/springframework/core/convert/service/MapToMap.java

@@ -23,36 +23,58 @@ import java.util.TreeMap;
 
 import org.springframework.core.convert.ConversionExecutionException;
 import org.springframework.core.convert.ConversionExecutor;
+import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.TypeDescriptor;
 
+/**
+ * Converts from one map to another map, with support for converting individual map elements based on generic type information.
+ * @author Keith Donald
+ */
 class MapToMap implements ConversionExecutor {
 
 	private TypeDescriptor sourceType;
 
 	private TypeDescriptor targetType;
 
-	private GenericConversionService conversionService;
+	private ConversionService conversionService;
 
-	public MapToMap(TypeDescriptor sourceType, TypeDescriptor targetType, GenericConversionService conversionService) {
+	private EntryConverter entryConverter;
+
+	/**
+	 * Creates a new map-to-map converter
+	 * @param sourceType the source map type
+	 * @param targetType the target map type
+	 * @param conversionService the conversion service
+	 */
+	public MapToMap(TypeDescriptor sourceType, TypeDescriptor targetType, ConversionService conversionService) {
 		this.sourceType = sourceType;
 		this.targetType = targetType;
 		this.conversionService = conversionService;
+		this.entryConverter = createEntryConverter();
+	}
+
+	private EntryConverter createEntryConverter() {
+		if (sourceType.isMapEntryTypeKnown() && targetType.isMapEntryTypeKnown()) {
+			ConversionExecutor keyConverter = conversionService.getConversionExecutor(sourceType.getMapKeyType(),
+					TypeDescriptor.valueOf(targetType.getMapKeyType()));
+			ConversionExecutor valueConverter = conversionService.getConversionExecutor(sourceType.getMapValueType(),
+					TypeDescriptor.valueOf(targetType.getMapValueType()));
+			return new EntryConverter(keyConverter, valueConverter);
+		} else {
+			return EntryConverter.NO_OP_INSTANCE;
+		}
 	}
 
 	@SuppressWarnings("unchecked")
 	public Object execute(Object source) throws ConversionExecutionException {
 		try {
-			// TODO shouldn't do all this if generic info is null - should cache executor after first iteration?
 			Map map = (Map) source;
 			Map targetMap = (Map) getImpl(targetType.getType()).newInstance();
+			EntryConverter converter = getEntryConverter(map);
 			Iterator<Map.Entry<?, ?>> it = map.entrySet().iterator();
 			while (it.hasNext()) {
 				Map.Entry entry = it.next();
-				Object key = entry.getKey();
+				targetMap.put(converter.convertKey(entry.getKey()), converter.convertValue(entry.getValue()));
-				Object value = entry.getValue();
-				key = conversionService.executeConversion(key, TypeDescriptor.valueOf(targetType.getMapKeyType()));
-				value = conversionService.executeConversion(value, TypeDescriptor.valueOf(targetType.getMapValueType()));
-				targetMap.put(key, value);
 			}
 			return targetMap;
 		} catch (Exception e) {
@@ -60,6 +82,37 @@ class MapToMap implements ConversionExecutor {
 		}
 	}
 
+	private EntryConverter getEntryConverter(Map<?, ?> map) {
+		EntryConverter entryConverter = this.entryConverter;
+		if (entryConverter == EntryConverter.NO_OP_INSTANCE) {
+			Class<?> targetKeyType = targetType.getMapKeyType();
+			Class<?> targetValueType = targetType.getMapValueType();
+			if (targetKeyType != null && targetValueType != null) {
+				ConversionExecutor keyConverter = null;
+				ConversionExecutor valueConverter = null;
+				Iterator<?> it = map.entrySet().iterator();
+				while (it.hasNext()) {
+					Map.Entry<?, ?> entry = (Map.Entry<?, ?>) it.next();
+					Object key = entry.getKey();
+					Object value = entry.getValue();
+					if (keyConverter == null && key != null) {
+						keyConverter = conversionService.getConversionExecutor(key.getClass(), TypeDescriptor
+								.valueOf(targetKeyType));
+					}
+					if (valueConverter == null && value != null) {
+						valueConverter = conversionService.getConversionExecutor(value.getClass(), TypeDescriptor
+								.valueOf(targetValueType));
+					}
+					if (keyConverter != null && valueConverter != null) {
+						break;
+					}
+				}
+				entryConverter = new EntryConverter(keyConverter, valueConverter);
+			}
+		}
+		return entryConverter;
+	}
+
 	static Class<?> getImpl(Class<?> targetClass) {
 		if (targetClass.isInterface()) {
 			if (Map.class.equals(targetClass)) {
@@ -74,4 +127,39 @@ class MapToMap implements ConversionExecutor {
 		}
 	}
 
+	private static class EntryConverter {
+
+		public static final EntryConverter NO_OP_INSTANCE = new EntryConverter();
+
+		private ConversionExecutor keyConverter;
+
+		private ConversionExecutor valueConverter;
+
+		private EntryConverter() {
+
+		}
+
+		public EntryConverter(ConversionExecutor keyConverter, ConversionExecutor valueConverter) {
+			this.keyConverter = keyConverter;
+			this.valueConverter = valueConverter;
+		}
+
+		public Object convertKey(Object key) {
+			if (keyConverter != null) {
+				return keyConverter.execute(key);
+			} else {
+				return key;
+			}
+		}
+
+		public Object convertValue(Object value) {
+			if (valueConverter != null) {
+				return valueConverter.execute(value);
+			} else {
+				return value;
+			}
+		}
+
+	}
+
 }

org.springframework.core/src/test/java/org/springframework/core/convert/service/MapToMapTests.java

@@ -0,0 +1,57 @@
+package org.springframework.core.convert.service;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+import org.springframework.core.convert.TypeDescriptor;
+
+public class MapToMapTests {
+
+	@Test
+	public void testMapToMapConversion() throws Exception {
+		DefaultConversionService service = new DefaultConversionService();
+		MapToMap c = new MapToMap(new TypeDescriptor(getClass().getField("source")),
+				new TypeDescriptor(getClass().getField("bindTarget")), service);
+		source.put("1", "BAR");
+		source.put("2", "BAZ");
+		Map result = (Map) c.execute(source);
+		assertEquals(FooEnum.BAR, result.get(1));
+		assertEquals(FooEnum.BAZ, result.get(2));
+	}
+
+	@Test
+	public void testMapToMapConversionNoGenericInfoOnSource() throws Exception {
+		DefaultConversionService service = new DefaultConversionService();
+		MapToMap c = new MapToMap(TypeDescriptor.valueOf(Map.class),
+				new TypeDescriptor(getClass().getField("bindTarget")), service);
+		source.put("1", "BAR");
+		source.put("2", "BAZ");
+		Map result = (Map) c.execute(source);
+		assertEquals(FooEnum.BAR, result.get(1));
+		assertEquals(FooEnum.BAZ, result.get(2));
+	}
+	
+	@Test
+	public void testMapToMapConversionNoGenericInfo() throws Exception {
+		DefaultConversionService service = new DefaultConversionService();
+		MapToMap c = new MapToMap(TypeDescriptor.valueOf(Map.class),
+				TypeDescriptor.valueOf(Map.class), service);
+		source.put("1", "BAR");
+		source.put("2", "BAZ");
+		Map result = (Map) c.execute(source);
+		assertEquals("BAR", result.get("1"));
+		assertEquals("BAZ", result.get("2"));
+	}
+
+
+	public Map<String, String> source = new HashMap<String, String>();
+	public Map<Integer, FooEnum> bindTarget = new HashMap<Integer, FooEnum>();
+	
+	public static enum FooEnum {
+		BAR, BAZ;
+	}
+
+}