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

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<Class<?>> approximableCollectionTypes = new HashSet<Class<?>>(10);
+	private static final Set<Class<?>> approximableCollectionTypes = new HashSet<Class<?>>(11);
 
-	private static final Set<Class<?>> approximableMapTypes = new HashSet<Class<?>>(6);
+	private static final Set<Class<?>> approximableMapTypes = new HashSet<Class<?>>(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 <em>approximable</em> 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 <em>approximable</em>
 	 */
 	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 <E> Collection<E> createApproximateCollection(Object collection, int capacity) {
@@ -114,9 +115,10 @@ public abstract class CollectionFactory {
 			return new ArrayList<E>(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<E>) EnumSet.copyOf((EnumSet) collection);
+			// Cast is necessary for compilation in Eclipse 4.4.1.
+			Collection<E> enumSet = (Collection<E>) EnumSet.copyOf((EnumSet) collection);
+			enumSet.clear();
+			return enumSet;
 		}
 		else if (collection instanceof SortedSet) {
 			return new TreeSet<E>(((SortedSet<E>) collection).comparator());
@@ -130,9 +132,9 @@ public abstract class CollectionFactory {
 	 * Create the most appropriate collection for the given collection type.
 	 * <p>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 <E> Collection<E> 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 <E> Collection<E> 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<E>) 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 <em>approximable</em> 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 <em>approximable</em>
 	 */
 	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 <K, V> Map<K, V> 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<K, V>(((SortedMap<K, V>) 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.
 	 * <p>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 <K, V> Map<K, V> 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<? extends Enum> asEnumType(Class<?> enumType) {

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<Integer>, but the collection returned
-			// by createApproximateCollection() is of type Collection<Color>.
-			ints.iterator().next().intValue();
+			// by createApproximateCollection() is of type Collection<Color>. 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<String> set = createApproximateCollection(new HashSet<String>(), 2);
+		assertThat(set.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateCollectionFromNonEmptyHashSet() {
+		HashSet<String> hashSet = new HashSet<String>();
+		hashSet.add("foo");
+		Collection<String> set = createApproximateCollection(hashSet, 2);
+		assertThat(set.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateCollectionFromEmptyEnumSet() {
+		Collection<Color> colors = createApproximateCollection(EnumSet.noneOf(Color.class), 2);
+		assertThat(colors.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateCollectionFromNonEmptyEnumSet() {
+		Collection<Color> colors = createApproximateCollection(EnumSet.of(Color.BLUE), 2);
+		assertThat(colors.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateMapFromEmptyHashMap() {
+		Map<String, String> map = createApproximateMap(new HashMap<String, String>(), 2);
+		assertThat(map.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateMapFromNonEmptyHashMap() {
+		Map<String, String> hashMap = new HashMap<String, String>();
+		hashMap.put("foo", "bar");
+		Map<String, String> map = createApproximateMap(hashMap, 2);
+		assertThat(map.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateMapFromEmptyEnumMap() {
+		Map<Color, String> colors = createApproximateMap(new EnumMap<Color, String>(Color.class), 2);
+		assertThat(colors.size(), is(0));
+	}
+
+	@Test
+	public void createApproximateMapFromNonEmptyEnumMap() {
+		EnumMap<Color, String> enumMap = new EnumMap<Color, String>(Color.class);
+		enumMap.put(Color.BLUE, "blue");
+		Map<Color, String> colors = createApproximateMap(enumMap, 2);
+		assertThat(colors.size(), is(0));
+	}
+
 	@Test
 	public void createsCollectionsCorrectly() {
 		// interfaces