Skip to content

Commit aec284a

Browse files
committed
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
1 parent fb426fe commit aec284a

File tree

2 files changed

+93
-32
lines changed

2 files changed

+93
-32
lines changed

spring-core/src/main/java/org/springframework/core/CollectionFactory.java

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@
5454
*/
5555
public abstract class CollectionFactory {
5656

57-
private static final Set<Class<?>> approximableCollectionTypes = new HashSet<Class<?>>(10);
57+
private static final Set<Class<?>> approximableCollectionTypes = new HashSet<Class<?>>(11);
5858

59-
private static final Set<Class<?>> approximableMapTypes = new HashSet<Class<?>>(6);
59+
private static final Set<Class<?>> approximableMapTypes = new HashSet<Class<?>>(7);
6060

6161

6262
static {
@@ -85,25 +85,26 @@ public abstract class CollectionFactory {
8585

8686

8787
/**
88-
* Determine whether the given collection type is an approximable type,
88+
* Determine whether the given collection type is an <em>approximable</em> type,
8989
* i.e. a type that {@link #createApproximateCollection} can approximate.
9090
* @param collectionType the collection type to check
91-
* @return {@code true} if the type is approximable
91+
* @return {@code true} if the type is <em>approximable</em>
9292
*/
9393
public static boolean isApproximableCollectionType(Class<?> collectionType) {
9494
return (collectionType != null && approximableCollectionTypes.contains(collectionType));
9595
}
9696

9797
/**
9898
* Create the most approximate collection for the given collection.
99-
* @param collection the original Collection object
99+
* @param collection the original collection object
100100
* @param capacity the initial capacity
101-
* @return the new Collection instance
102-
* @see java.util.LinkedHashSet
103-
* @see java.util.TreeSet
104-
* @see java.util.EnumSet
105-
* @see java.util.ArrayList
101+
* @return the new, empty collection instance
102+
* @see #isApproximableCollectionType
106103
* @see java.util.LinkedList
104+
* @see java.util.ArrayList
105+
* @see java.util.EnumSet
106+
* @see java.util.TreeSet
107+
* @see java.util.LinkedHashSet
107108
*/
108109
@SuppressWarnings({ "unchecked", "cast", "rawtypes" })
109110
public static <E> Collection<E> createApproximateCollection(Object collection, int capacity) {
@@ -114,9 +115,10 @@ else if (collection instanceof List) {
114115
return new ArrayList<E>(capacity);
115116
}
116117
else if (collection instanceof EnumSet) {
117-
// superfluous cast necessary for bug in Eclipse 4.4.1.
118-
// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=454644
119-
return (Collection<E>) EnumSet.copyOf((EnumSet) collection);
118+
// Cast is necessary for compilation in Eclipse 4.4.1.
119+
Collection<E> enumSet = (Collection<E>) EnumSet.copyOf((EnumSet) collection);
120+
enumSet.clear();
121+
return enumSet;
120122
}
121123
else if (collection instanceof SortedSet) {
122124
return new TreeSet<E>(((SortedSet<E>) collection).comparator());
@@ -130,26 +132,26 @@ else if (collection instanceof SortedSet) {
130132
* Create the most appropriate collection for the given collection type.
131133
* <p>Delegates to {@link #createCollection(Class, Class, int)} with a
132134
* {@code null} element type.
133-
* @param collectionType the desired type of the target Collection
135+
* @param collectionType the desired type of the target collection
134136
* @param capacity the initial capacity
135-
* @return the new Collection instance
137+
* @return the new collection instance
136138
*/
137139
public static <E> Collection<E> createCollection(Class<?> collectionType, int capacity) {
138140
return createCollection(collectionType, null, capacity);
139141
}
140142

141143
/**
142144
* Create the most appropriate collection for the given collection type.
143-
* @param collectionType the desired type of the target Collection; never {@code null}
145+
* @param collectionType the desired type of the target collection; never {@code null}
144146
* @param elementType the collection's element type, or {@code null} if not known
145147
* (note: only relevant for {@link EnumSet} creation)
146148
* @param capacity the initial capacity
147-
* @return the new Collection instance
149+
* @return the new collection instance
148150
* @since 4.1.3
149151
* @see java.util.LinkedHashSet
152+
* @see java.util.ArrayList
150153
* @see java.util.TreeSet
151154
* @see java.util.EnumSet
152-
* @see java.util.ArrayList
153155
*/
154156
@SuppressWarnings({ "unchecked", "cast" })
155157
public static <E> Collection<E> createCollection(Class<?> collectionType, Class<?> elementType, int capacity) {
@@ -170,8 +172,7 @@ else if (SortedSet.class.equals(collectionType) || NavigableSet.class.equals(col
170172
}
171173
else if (EnumSet.class.equals(collectionType)) {
172174
Assert.notNull(elementType, "Cannot create EnumSet for unknown element type");
173-
// superfluous cast necessary for bug in Eclipse 4.4.1.
174-
// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=454644
175+
// Cast is necessary for compilation in Eclipse 4.4.1.
175176
return (Collection<E>) EnumSet.noneOf(asEnumType(elementType));
176177
}
177178
else {
@@ -189,11 +190,10 @@ else if (EnumSet.class.equals(collectionType)) {
189190
}
190191

191192
/**
192-
* Determine whether the given map type is an approximable type,
193+
* Determine whether the given map type is an <em>approximable</em> type,
193194
* i.e. a type that {@link #createApproximateMap} can approximate.
194195
* @param mapType the map type to check
195-
* @return {@code true} if the type is approximable,
196-
* {@code false} if it is not
196+
* @return {@code true} if the type is <em>approximable</em>
197197
*/
198198
public static boolean isApproximableMapType(Class<?> mapType) {
199199
return (mapType != null && approximableMapTypes.contains(mapType));
@@ -203,14 +203,18 @@ public static boolean isApproximableMapType(Class<?> mapType) {
203203
* Create the most approximate map for the given map.
204204
* @param map the original Map object
205205
* @param capacity the initial capacity
206-
* @return the new Map instance
206+
* @return the new, empty Map instance
207+
* @see #isApproximableMapType
208+
* @see java.util.EnumMap
207209
* @see java.util.TreeMap
208210
* @see java.util.LinkedHashMap
209211
*/
210212
@SuppressWarnings({"unchecked", "rawtypes"})
211213
public static <K, V> Map<K, V> createApproximateMap(Object map, int capacity) {
212214
if (map instanceof EnumMap) {
213-
return new EnumMap((Map) map);
215+
EnumMap enumMap = new EnumMap((EnumMap) map);
216+
enumMap.clear();
217+
return enumMap;
214218
}
215219
else if (map instanceof SortedMap) {
216220
return new TreeMap<K, V>(((SortedMap<K, V>) map).comparator());
@@ -221,7 +225,7 @@ else if (map instanceof SortedMap) {
221225
}
222226

223227
/**
224-
* Create the most approximate map for the given map.
228+
* Create the most appropriate map for the given map type.
225229
* <p>Delegates to {@link #createMap(Class, Class, int)} with a
226230
* {@code null} key type.
227231
* @param mapType the desired type of the target Map
@@ -233,7 +237,7 @@ public static <K, V> Map<K, V> createMap(Class<?> mapType, int capacity) {
233237
}
234238

235239
/**
236-
* Create the most approximate map for the given map.
240+
* Create the most appropriate map for the given map type.
237241
* @param mapType the desired type of the target Map
238242
* @param keyType the map's key type, or {@code null} if not known
239243
* (note: only relevant for {@link EnumMap} creation)
@@ -242,8 +246,8 @@ public static <K, V> Map<K, V> createMap(Class<?> mapType, int capacity) {
242246
* @since 4.1.3
243247
* @see java.util.LinkedHashMap
244248
* @see java.util.TreeMap
245-
* @see java.util.EnumMap
246249
* @see org.springframework.util.LinkedMultiValueMap
250+
* @see java.util.EnumMap
247251
*/
248252
@SuppressWarnings({"unchecked", "rawtypes"})
249253
public static <K, V> Map<K, V> createMap(Class<?> mapType, Class<?> keyType, int capacity) {
@@ -284,6 +288,7 @@ else if (EnumMap.class.equals(mapType)) {
284288
* @param enumType the enum type, never {@code null}
285289
* @return the given type as subtype of {@link Enum}
286290
* @throws IllegalArgumentException if the given type is not a subtype of {@link Enum}
291+
* @since 4.1.4
287292
*/
288293
@SuppressWarnings("rawtypes")
289294
private static Class<? extends Enum> asEnumType(Class<?> enumType) {

spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ public void createApproximateCollectionIsNotTypeSafe() {
7070
// next line and not as a result of the previous line.
7171
try {
7272
// Note that ints is of type Collection<Integer>, but the collection returned
73-
// by createApproximateCollection() is of type Collection<Color>.
74-
ints.iterator().next().intValue();
73+
// by createApproximateCollection() is of type Collection<Color>. Thus, 42
74+
// cannot be cast to a Color.
75+
ints.add(42);
7576
fail("Should have thrown a ClassCastException");
7677
}
7778
catch (ClassCastException e) {
@@ -97,15 +98,70 @@ public void createApproximateMapIsNotTypeSafe() {
9798
// next line and not as a result of the previous line.
9899
try {
99100
// Note that the 'map' key is of type String, but the keys in the map returned
100-
// by createApproximateMap() are of type Color.
101-
map.keySet().iterator().next().split(",");
101+
// by createApproximateMap() are of type Color. Thus "foo" cannot be cast to a
102+
// Color.
103+
map.put("foo", 1);
102104
fail("Should have thrown a ClassCastException");
103105
}
104106
catch (ClassCastException e) {
105107
/* expected */
106108
}
107109
}
108110

111+
@Test
112+
public void createApproximateCollectionFromEmptyHashSet() {
113+
Collection<String> set = createApproximateCollection(new HashSet<String>(), 2);
114+
assertThat(set.size(), is(0));
115+
}
116+
117+
@Test
118+
public void createApproximateCollectionFromNonEmptyHashSet() {
119+
HashSet<String> hashSet = new HashSet<String>();
120+
hashSet.add("foo");
121+
Collection<String> set = createApproximateCollection(hashSet, 2);
122+
assertThat(set.size(), is(0));
123+
}
124+
125+
@Test
126+
public void createApproximateCollectionFromEmptyEnumSet() {
127+
Collection<Color> colors = createApproximateCollection(EnumSet.noneOf(Color.class), 2);
128+
assertThat(colors.size(), is(0));
129+
}
130+
131+
@Test
132+
public void createApproximateCollectionFromNonEmptyEnumSet() {
133+
Collection<Color> colors = createApproximateCollection(EnumSet.of(Color.BLUE), 2);
134+
assertThat(colors.size(), is(0));
135+
}
136+
137+
@Test
138+
public void createApproximateMapFromEmptyHashMap() {
139+
Map<String, String> map = createApproximateMap(new HashMap<String, String>(), 2);
140+
assertThat(map.size(), is(0));
141+
}
142+
143+
@Test
144+
public void createApproximateMapFromNonEmptyHashMap() {
145+
Map<String, String> hashMap = new HashMap<String, String>();
146+
hashMap.put("foo", "bar");
147+
Map<String, String> map = createApproximateMap(hashMap, 2);
148+
assertThat(map.size(), is(0));
149+
}
150+
151+
@Test
152+
public void createApproximateMapFromEmptyEnumMap() {
153+
Map<Color, String> colors = createApproximateMap(new EnumMap<Color, String>(Color.class), 2);
154+
assertThat(colors.size(), is(0));
155+
}
156+
157+
@Test
158+
public void createApproximateMapFromNonEmptyEnumMap() {
159+
EnumMap<Color, String> enumMap = new EnumMap<Color, String>(Color.class);
160+
enumMap.put(Color.BLUE, "blue");
161+
Map<Color, String> colors = createApproximateMap(enumMap, 2);
162+
assertThat(colors.size(), is(0));
163+
}
164+
109165
@Test
110166
public void createsCollectionsCorrectly() {
111167
// interfaces

0 commit comments

Comments
 (0)