Skip to content

Commit

Permalink
Consistent support for MultiValueMap and common Map implementations
Browse files Browse the repository at this point in the history 
Closes gh-30440
  • Loading branch information
@jhoeller
jhoeller committed May 8, 2023
1 parent 2c15dcc commit c09055b
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 49 deletions.
48 changes: 24 additions & 24 deletions spring-core/src/main/java/org/springframework/core/CollectionFactory.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -73,11 +73,13 @@ public final class CollectionFactory {
private static final Set<Class<?>> approximableMapTypes = Set.of(
// Standard map interfaces
Map.class,
MultiValueMap.class,
SortedMap.class,
NavigableMap.class,
// Common concrete map classes
HashMap.class,
LinkedHashMap.class,
LinkedMultiValueMap.class,
TreeMap.class,
EnumMap.class);

Expand Down Expand Up @@ -118,20 +120,20 @@ public static boolean isApproximableCollectionType(@Nullable Class<?> collection
*/
@SuppressWarnings({"rawtypes", "unchecked"})
public static <E> Collection<E> createApproximateCollection(@Nullable Object collection, int capacity) {
if (collection instanceof LinkedList) {
return new LinkedList<>();
}
else if (collection instanceof List) {
return new ArrayList<>(capacity);
}
else if (collection instanceof EnumSet enumSet) {
if (collection instanceof EnumSet enumSet) {
Collection<E> copy = EnumSet.copyOf(enumSet);
copy.clear();
return copy;
}
else if (collection instanceof SortedSet sortedSet) {
return new TreeSet<>(sortedSet.comparator());
}
else if (collection instanceof LinkedList) {
return new LinkedList<>();
}
else if (collection instanceof List) {
return new ArrayList<>(capacity);
}
else {
return new LinkedHashSet<>(capacity);
}
Expand Down Expand Up @@ -187,8 +189,8 @@ else if (ArrayList.class == collectionType || List.class == collectionType) {
else if (LinkedList.class == collectionType) {
return new LinkedList<>();
}
else if (TreeSet.class == collectionType || NavigableSet.class == collectionType
|| SortedSet.class == collectionType) {
else if (TreeSet.class == collectionType || NavigableSet.class == collectionType ||
SortedSet.class == collectionType) {
return new TreeSet<>();
}
else if (EnumSet.class.isAssignableFrom(collectionType)) {
Expand Down Expand Up @@ -246,6 +248,9 @@ public static <K, V> Map<K, V> createApproximateMap(@Nullable Object map, int ca
else if (map instanceof SortedMap sortedMap) {
return new TreeMap<>(sortedMap.comparator());
}
else if (map instanceof MultiValueMap) {
return new LinkedMultiValueMap(capacity);
}
else {
return new LinkedHashMap<>(capacity);
}
Expand Down Expand Up @@ -292,26 +297,21 @@ public static <K, V> Map<K, V> createMap(Class<?> mapType, int capacity) {
@SuppressWarnings({"rawtypes", "unchecked"})
public static <K, V> Map<K, V> createMap(Class<?> mapType, @Nullable Class<?> keyType, int capacity) {
Assert.notNull(mapType, "Map type must not be null");
if (mapType.isInterface()) {
if (Map.class == mapType) {
return new LinkedHashMap<>(capacity);
}
else if (SortedMap.class == mapType || NavigableMap.class == mapType) {
return new TreeMap<>();
}
else if (MultiValueMap.class == mapType) {
return new LinkedMultiValueMap();
}
else {
throw new IllegalArgumentException("Unsupported Map interface: " + mapType.getName());
}
if (LinkedHashMap.class == mapType || HashMap.class == mapType || Map.class == mapType) {
return new LinkedHashMap<>(capacity);
}
else if (LinkedMultiValueMap.class == mapType || MultiValueMap.class == mapType) {
return new LinkedMultiValueMap();
}
else if (TreeMap.class == mapType || SortedMap.class == mapType || NavigableMap.class == mapType) {
return new TreeMap<>();
}
else if (EnumMap.class == mapType) {
Assert.notNull(keyType, "Cannot create EnumMap for unknown key type");
return new EnumMap(asEnumType(keyType));
}
else {
if (!Map.class.isAssignableFrom(mapType)) {
if (mapType.isInterface() || !Map.class.isAssignableFrom(mapType)) {
throw new IllegalArgumentException("Unsupported Map type: " + mapType.getName());
}
try {
Expand Down
54 changes: 29 additions & 25 deletions spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -209,21 +209,23 @@ void createApproximateMapFromNonEmptyEnumMap() {
@Test
void createsCollectionsCorrectly() {
// interfaces
assertThat(createCollection(List.class, 0)).isInstanceOf(ArrayList.class);
assertThat(createCollection(Set.class, 0)).isInstanceOf(LinkedHashSet.class);
assertThat(createCollection(Collection.class, 0)).isInstanceOf(LinkedHashSet.class);
assertThat(createCollection(SortedSet.class, 0)).isInstanceOf(TreeSet.class);
assertThat(createCollection(NavigableSet.class, 0)).isInstanceOf(TreeSet.class);

assertThat(createCollection(List.class, String.class, 0)).isInstanceOf(ArrayList.class);
assertThat(createCollection(Set.class, String.class, 0)).isInstanceOf(LinkedHashSet.class);
assertThat(createCollection(Collection.class, String.class, 0)).isInstanceOf(LinkedHashSet.class);
assertThat(createCollection(SortedSet.class, String.class, 0)).isInstanceOf(TreeSet.class);
assertThat(createCollection(NavigableSet.class, String.class, 0)).isInstanceOf(TreeSet.class);
testCollection(List.class, ArrayList.class);
testCollection(Set.class, LinkedHashSet.class);
testCollection(Collection.class, LinkedHashSet.class);
testCollection(SortedSet.class, TreeSet.class);
testCollection(NavigableSet.class, TreeSet.class);

// concrete types
assertThat(createCollection(HashSet.class, 0)).isInstanceOf(HashSet.class);
assertThat(createCollection(HashSet.class, String.class, 0)).isInstanceOf(HashSet.class);
testCollection(ArrayList.class, ArrayList.class);
testCollection(HashSet.class, LinkedHashSet.class);
testCollection(LinkedHashSet.class, LinkedHashSet.class);
testCollection(TreeSet.class, TreeSet.class);
}

private void testCollection(Class<?> collectionType, Class<?> resultType) {
assertThat(CollectionFactory.isApproximableCollectionType(collectionType)).isTrue();
assertThat(createCollection(collectionType, 0)).isInstanceOf(resultType);
assertThat(createCollection(collectionType, String.class, 0)).isInstanceOf(resultType);
}

@Test
Expand Down Expand Up @@ -258,20 +260,22 @@ void rejectsNullCollectionType() {
@Test
void createsMapsCorrectly() {
// interfaces
assertThat(createMap(Map.class, 0)).isInstanceOf(LinkedHashMap.class);
assertThat(createMap(SortedMap.class, 0)).isInstanceOf(TreeMap.class);
assertThat(createMap(NavigableMap.class, 0)).isInstanceOf(TreeMap.class);
assertThat(createMap(MultiValueMap.class, 0)).isInstanceOf(LinkedMultiValueMap.class);

assertThat(createMap(Map.class, String.class, 0)).isInstanceOf(LinkedHashMap.class);
assertThat(createMap(SortedMap.class, String.class, 0)).isInstanceOf(TreeMap.class);
assertThat(createMap(NavigableMap.class, String.class, 0)).isInstanceOf(TreeMap.class);
assertThat(createMap(MultiValueMap.class, String.class, 0)).isInstanceOf(LinkedMultiValueMap.class);
testMap(Map.class, LinkedHashMap.class);
testMap(SortedMap.class, TreeMap.class);
testMap(NavigableMap.class, TreeMap.class);
testMap(MultiValueMap.class, LinkedMultiValueMap.class);

// concrete types
assertThat(createMap(HashMap.class, 0)).isInstanceOf(HashMap.class);
testMap(HashMap.class, LinkedHashMap.class);
testMap(LinkedHashMap.class, LinkedHashMap.class);
testMap(TreeMap.class, TreeMap.class);
testMap(LinkedMultiValueMap.class, LinkedMultiValueMap.class);
}

assertThat(createMap(HashMap.class, String.class, 0)).isInstanceOf(HashMap.class);
private void testMap(Class<?> mapType, Class<?> resultType) {
assertThat(CollectionFactory.isApproximableMapType(mapType)).isTrue();
assertThat(createMap(mapType, 0)).isInstanceOf(resultType);
assertThat(createMap(mapType, String.class, 0)).isInstanceOf(resultType);
}

@Test
Expand Down

0 comments on commit c09055b

Please sign in to comment.