diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java index e8923e3ec6..c2dc4bfc20 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/MapToMapConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,9 @@ package org.springframework.core.convert.support; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; @@ -51,7 +53,7 @@ final class MapToMapConverter implements ConditionalGenericConverter { public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { return canConvertKey(sourceType, targetType) && canConvertValue(sourceType, targetType); } - + @SuppressWarnings("unchecked") public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { if (source == null) { @@ -62,32 +64,39 @@ final class MapToMapConverter implements ConditionalGenericConverter { if (!copyRequired && sourceMap.isEmpty()) { return sourceMap; } - Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); + List targetEntries = new ArrayList(sourceMap.size()); for (Map.Entry entry : sourceMap.entrySet()) { Object sourceKey = entry.getKey(); Object sourceValue = entry.getValue(); Object targetKey = convertKey(sourceKey, sourceType, targetType.getMapKeyTypeDescriptor()); Object targetValue = convertValue(sourceValue, sourceType, targetType.getMapValueTypeDescriptor()); - targetMap.put(targetKey, targetValue); + targetEntries.add(new MapEntry(targetKey, targetValue)); if (sourceKey != targetKey || sourceValue != targetValue) { copyRequired = true; } } - return (copyRequired ? targetMap : sourceMap); + if(!copyRequired) { + return sourceMap; + } + Map targetMap = CollectionFactory.createMap(targetType.getType(), sourceMap.size()); + for (MapEntry entry : targetEntries) { + entry.addToMap(targetMap); + } + return targetMap; } - + // internal helpers private boolean canConvertKey(TypeDescriptor sourceType, TypeDescriptor targetType) { return ConversionUtils.canConvertElements(sourceType.getMapKeyTypeDescriptor(), targetType.getMapKeyTypeDescriptor(), this.conversionService); } - + private boolean canConvertValue(TypeDescriptor sourceType, TypeDescriptor targetType) { return ConversionUtils.canConvertElements(sourceType.getMapValueTypeDescriptor(), targetType.getMapValueTypeDescriptor(), this.conversionService); } - + private Object convertKey(Object sourceKey, TypeDescriptor sourceType, TypeDescriptor targetType) { if (targetType == null) { return sourceKey; @@ -102,4 +111,19 @@ final class MapToMapConverter implements ConditionalGenericConverter { return this.conversionService.convert(sourceValue, sourceType.getMapValueTypeDescriptor(sourceValue), targetType); } + private static class MapEntry { + + private Object key; + private Object value; + + public MapEntry(Object key, Object value) { + this.key = key; + this.value = value; + } + + public void addToMap(Map map) { + map.put(key, value); + } + } + } diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java index afb525cb5b..635382e7c7 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/MapToMapConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package org.springframework.core.convert.support; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -55,7 +56,7 @@ public class MapToMapConverterTests { } conversionService.addConverterFactory(new StringToNumberConverterFactory()); assertTrue(conversionService.canConvert(sourceType, targetType)); - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") Map result = (Map) conversionService.convert(map, sourceType, targetType); assertFalse(map.equals(result)); assertEquals((Integer) 9, result.get(1)); @@ -79,7 +80,7 @@ public class MapToMapConverterTests { map.put("1", "9"); map.put("2", "37"); TypeDescriptor sourceType = new TypeDescriptor(getClass().getField("notGenericMapSource")); - TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget")); + TypeDescriptor targetType = new TypeDescriptor(getClass().getField("scalarMapTarget")); assertTrue(conversionService.canConvert(sourceType, targetType)); try { conversionService.convert(map, sourceType, targetType); @@ -88,15 +89,15 @@ public class MapToMapConverterTests { } conversionService.addConverterFactory(new StringToNumberConverterFactory()); assertTrue(conversionService.canConvert(sourceType, targetType)); - @SuppressWarnings("unchecked") + @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)); + assertEquals((Integer) 37, result.get(2)); } - + public Map notGenericMapSource; - + @Test public void collectionMap() throws Exception { Map> map = new HashMap>(); @@ -109,11 +110,11 @@ public class MapToMapConverterTests { conversionService.convert(map, sourceType, targetType); } catch (ConversionFailedException e) { assertTrue(e.getCause() instanceof ConverterNotFoundException); - } + } conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); conversionService.addConverterFactory(new StringToNumberConverterFactory()); assertTrue(conversionService.canConvert(sourceType, targetType)); - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") Map> result = (Map>) conversionService.convert(map, sourceType, targetType); assertFalse(map.equals(result)); assertEquals(Arrays.asList(9, 12), result.get(1)); @@ -134,12 +135,12 @@ public class MapToMapConverterTests { conversionService.convert(map, sourceType, targetType); fail("Should have failed"); } catch (ConverterNotFoundException e) { - + } conversionService.addConverter(new CollectionToCollectionConverter(conversionService)); conversionService.addConverterFactory(new StringToNumberConverterFactory()); assertTrue(conversionService.canConvert(sourceType, targetType)); - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") Map> result = (Map>) conversionService.convert(map, sourceType, targetType); assertFalse(map.equals(result)); assertEquals(Arrays.asList(9, 12), result.get(1)); @@ -167,7 +168,7 @@ public class MapToMapConverterTests { assertTrue(conversionService.canConvert(Map.class, Map.class)); assertSame(map, conversionService.convert(map, Map.class)); } - + @Test public void emptyMap() throws Exception { Map map = new HashMap(); @@ -200,4 +201,26 @@ public class MapToMapConverterTests { public LinkedHashMap emptyMapDifferentTarget; + @Test + public void noDefaultConstructorCopyNotRequired() throws Exception { + // SPR-9284 + NoDefaultConstructorMap map = new NoDefaultConstructorMap( + Collections. singletonMap("1", 1)); + TypeDescriptor sourceType = TypeDescriptor.map(NoDefaultConstructorMap.class, + TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class)); + TypeDescriptor targetType = TypeDescriptor.map(NoDefaultConstructorMap.class, + TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class)); + assertTrue(conversionService.canConvert(sourceType, targetType)); + @SuppressWarnings("unchecked") + Map result = (Map) conversionService.convert(map, sourceType, targetType); + assertEquals(map, result); + assertEquals(NoDefaultConstructorMap.class, result.getClass()); + } + + public static class NoDefaultConstructorMap extends HashMap { + public NoDefaultConstructorMap(Map m) { + super(m); + } + } + }