From aec284a4ca4db3adf6f7d8ae9da6ddc30ce367f1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 10 Dec 2014 22:59:00 +0100 Subject: [PATCH] Create empty EnumSets & EnumMaps in CollectionFactory SPR-12483 introduced automatic type conversion support for EnumSet and EnumMap. However, the corresponding changes in CollectionFactory contradict the existing contract for the "create approximate" methods by creating a copy of the supplied set or map, thereby potentially including elements in the returned collection when the returned collection should in fact be empty. This commit addresses this issue by ensuring that the collections returned by createApproximateCollection() and createApproximateMap() are always empty. Furthermore, this commit improves the Javadoc throughout the CollectionFactory class. Issue: SPR-12533 --- .../core/CollectionFactory.java | 61 ++++++++++-------- .../core/CollectionFactoryTests.java | 64 +++++++++++++++++-- 2 files changed, 93 insertions(+), 32 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java index 6a36f24162..c51e7132cd 100644 --- a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java +++ b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java @@ -54,9 +54,9 @@ import org.springframework.util.MultiValueMap; */ public abstract class CollectionFactory { - private static final Set> approximableCollectionTypes = new HashSet>(10); + private static final Set> approximableCollectionTypes = new HashSet>(11); - private static final Set> approximableMapTypes = new HashSet>(6); + private static final Set> approximableMapTypes = new HashSet>(7); static { @@ -85,10 +85,10 @@ public abstract class CollectionFactory { /** - * Determine whether the given collection type is an approximable type, + * Determine whether the given collection type is an approximable type, * i.e. a type that {@link #createApproximateCollection} can approximate. * @param collectionType the collection type to check - * @return {@code true} if the type is approximable + * @return {@code true} if the type is approximable */ public static boolean isApproximableCollectionType(Class collectionType) { return (collectionType != null && approximableCollectionTypes.contains(collectionType)); @@ -96,14 +96,15 @@ public abstract class CollectionFactory { /** * Create the most approximate collection for the given collection. - * @param collection the original Collection object + * @param collection the original collection object * @param capacity the initial capacity - * @return the new Collection instance - * @see java.util.LinkedHashSet - * @see java.util.TreeSet - * @see java.util.EnumSet - * @see java.util.ArrayList + * @return the new, empty collection instance + * @see #isApproximableCollectionType * @see java.util.LinkedList + * @see java.util.ArrayList + * @see java.util.EnumSet + * @see java.util.TreeSet + * @see java.util.LinkedHashSet */ @SuppressWarnings({ "unchecked", "cast", "rawtypes" }) public static Collection createApproximateCollection(Object collection, int capacity) { @@ -114,9 +115,10 @@ public abstract class CollectionFactory { return new ArrayList(capacity); } else if (collection instanceof EnumSet) { - // superfluous cast necessary for bug in Eclipse 4.4.1. - // see https://bugs.eclipse.org/bugs/show_bug.cgi?id=454644 - return (Collection) EnumSet.copyOf((EnumSet) collection); + // Cast is necessary for compilation in Eclipse 4.4.1. + Collection enumSet = (Collection) EnumSet.copyOf((EnumSet) collection); + enumSet.clear(); + return enumSet; } else if (collection instanceof SortedSet) { return new TreeSet(((SortedSet) collection).comparator()); @@ -130,9 +132,9 @@ public abstract class CollectionFactory { * Create the most appropriate collection for the given collection type. *

Delegates to {@link #createCollection(Class, Class, int)} with a * {@code null} element type. - * @param collectionType the desired type of the target Collection + * @param collectionType the desired type of the target collection * @param capacity the initial capacity - * @return the new Collection instance + * @return the new collection instance */ public static Collection createCollection(Class collectionType, int capacity) { return createCollection(collectionType, null, capacity); @@ -140,16 +142,16 @@ public abstract class CollectionFactory { /** * Create the most appropriate collection for the given collection type. - * @param collectionType the desired type of the target Collection; never {@code null} + * @param collectionType the desired type of the target collection; never {@code null} * @param elementType the collection's element type, or {@code null} if not known * (note: only relevant for {@link EnumSet} creation) * @param capacity the initial capacity - * @return the new Collection instance + * @return the new collection instance * @since 4.1.3 * @see java.util.LinkedHashSet + * @see java.util.ArrayList * @see java.util.TreeSet * @see java.util.EnumSet - * @see java.util.ArrayList */ @SuppressWarnings({ "unchecked", "cast" }) public static Collection createCollection(Class collectionType, Class elementType, int capacity) { @@ -170,8 +172,7 @@ public abstract class CollectionFactory { } else if (EnumSet.class.equals(collectionType)) { Assert.notNull(elementType, "Cannot create EnumSet for unknown element type"); - // superfluous cast necessary for bug in Eclipse 4.4.1. - // see https://bugs.eclipse.org/bugs/show_bug.cgi?id=454644 + // Cast is necessary for compilation in Eclipse 4.4.1. return (Collection) EnumSet.noneOf(asEnumType(elementType)); } else { @@ -189,11 +190,10 @@ public abstract class CollectionFactory { } /** - * Determine whether the given map type is an approximable type, + * Determine whether the given map type is an approximable type, * i.e. a type that {@link #createApproximateMap} can approximate. * @param mapType the map type to check - * @return {@code true} if the type is approximable, - * {@code false} if it is not + * @return {@code true} if the type is approximable */ public static boolean isApproximableMapType(Class mapType) { return (mapType != null && approximableMapTypes.contains(mapType)); @@ -203,14 +203,18 @@ public abstract class CollectionFactory { * Create the most approximate map for the given map. * @param map the original Map object * @param capacity the initial capacity - * @return the new Map instance + * @return the new, empty Map instance + * @see #isApproximableMapType + * @see java.util.EnumMap * @see java.util.TreeMap * @see java.util.LinkedHashMap */ @SuppressWarnings({"unchecked", "rawtypes"}) public static Map createApproximateMap(Object map, int capacity) { if (map instanceof EnumMap) { - return new EnumMap((Map) map); + EnumMap enumMap = new EnumMap((EnumMap) map); + enumMap.clear(); + return enumMap; } else if (map instanceof SortedMap) { return new TreeMap(((SortedMap) map).comparator()); @@ -221,7 +225,7 @@ public abstract class CollectionFactory { } /** - * Create the most approximate map for the given map. + * Create the most appropriate map for the given map type. *

Delegates to {@link #createMap(Class, Class, int)} with a * {@code null} key type. * @param mapType the desired type of the target Map @@ -233,7 +237,7 @@ public abstract class CollectionFactory { } /** - * Create the most approximate map for the given map. + * Create the most appropriate map for the given map type. * @param mapType the desired type of the target Map * @param keyType the map's key type, or {@code null} if not known * (note: only relevant for {@link EnumMap} creation) @@ -242,8 +246,8 @@ public abstract class CollectionFactory { * @since 4.1.3 * @see java.util.LinkedHashMap * @see java.util.TreeMap - * @see java.util.EnumMap * @see org.springframework.util.LinkedMultiValueMap + * @see java.util.EnumMap */ @SuppressWarnings({"unchecked", "rawtypes"}) public static Map createMap(Class mapType, Class keyType, int capacity) { @@ -284,6 +288,7 @@ public abstract class CollectionFactory { * @param enumType the enum type, never {@code null} * @return the given type as subtype of {@link Enum} * @throws IllegalArgumentException if the given type is not a subtype of {@link Enum} + * @since 4.1.4 */ @SuppressWarnings("rawtypes") private static Class asEnumType(Class enumType) { diff --git a/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java b/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java index b3be5d4cc5..fe8e8b55d3 100644 --- a/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java +++ b/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java @@ -70,8 +70,9 @@ public class CollectionFactoryTests { // next line and not as a result of the previous line. try { // Note that ints is of type Collection, but the collection returned - // by createApproximateCollection() is of type Collection. - ints.iterator().next().intValue(); + // by createApproximateCollection() is of type Collection. Thus, 42 + // cannot be cast to a Color. + ints.add(42); fail("Should have thrown a ClassCastException"); } catch (ClassCastException e) { @@ -97,8 +98,9 @@ public class CollectionFactoryTests { // next line and not as a result of the previous line. try { // Note that the 'map' key is of type String, but the keys in the map returned - // by createApproximateMap() are of type Color. - map.keySet().iterator().next().split(","); + // by createApproximateMap() are of type Color. Thus "foo" cannot be cast to a + // Color. + map.put("foo", 1); fail("Should have thrown a ClassCastException"); } catch (ClassCastException e) { @@ -106,6 +108,60 @@ public class CollectionFactoryTests { } } + @Test + public void createApproximateCollectionFromEmptyHashSet() { + Collection set = createApproximateCollection(new HashSet(), 2); + assertThat(set.size(), is(0)); + } + + @Test + public void createApproximateCollectionFromNonEmptyHashSet() { + HashSet hashSet = new HashSet(); + hashSet.add("foo"); + Collection set = createApproximateCollection(hashSet, 2); + assertThat(set.size(), is(0)); + } + + @Test + public void createApproximateCollectionFromEmptyEnumSet() { + Collection colors = createApproximateCollection(EnumSet.noneOf(Color.class), 2); + assertThat(colors.size(), is(0)); + } + + @Test + public void createApproximateCollectionFromNonEmptyEnumSet() { + Collection colors = createApproximateCollection(EnumSet.of(Color.BLUE), 2); + assertThat(colors.size(), is(0)); + } + + @Test + public void createApproximateMapFromEmptyHashMap() { + Map map = createApproximateMap(new HashMap(), 2); + assertThat(map.size(), is(0)); + } + + @Test + public void createApproximateMapFromNonEmptyHashMap() { + Map hashMap = new HashMap(); + hashMap.put("foo", "bar"); + Map map = createApproximateMap(hashMap, 2); + assertThat(map.size(), is(0)); + } + + @Test + public void createApproximateMapFromEmptyEnumMap() { + Map colors = createApproximateMap(new EnumMap(Color.class), 2); + assertThat(colors.size(), is(0)); + } + + @Test + public void createApproximateMapFromNonEmptyEnumMap() { + EnumMap enumMap = new EnumMap(Color.class); + enumMap.put(Color.BLUE, "blue"); + Map colors = createApproximateMap(enumMap, 2); + assertThat(colors.size(), is(0)); + } + @Test public void createsCollectionsCorrectly() { // interfaces -- GitLab