From c09055b83afe5530e11ad3f7a61af0f758efa3a8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 8 May 2023 12:01:58 +0200 Subject: [PATCH] Consistent support for MultiValueMap and common Map implementations Closes gh-30440 --- .../core/CollectionFactory.java | 48 ++++++++--------- .../core/CollectionFactoryTests.java | 54 ++++++++++--------- 2 files changed, 53 insertions(+), 49 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 cbda09255006..9d9332dbff3a 100644 --- a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java +++ b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java @@ -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. @@ -73,11 +73,13 @@ public final class CollectionFactory { private static final Set> 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); @@ -118,13 +120,7 @@ public static boolean isApproximableCollectionType(@Nullable Class collection */ @SuppressWarnings({"rawtypes", "unchecked"}) public static Collection 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 copy = EnumSet.copyOf(enumSet); copy.clear(); return copy; @@ -132,6 +128,12 @@ else if (collection instanceof EnumSet enumSet) { 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); } @@ -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)) { @@ -246,6 +248,9 @@ public static Map 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); } @@ -292,26 +297,21 @@ public static Map createMap(Class mapType, int capacity) { @SuppressWarnings({"rawtypes", "unchecked"}) public static Map 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 { 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 570099d83af8..32292b39778a 100644 --- a/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java +++ b/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java @@ -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. @@ -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 @@ -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