Revise MultiValueMapCollector implementation and tests

See https://github.com/spring-projects/spring-data-commons/issues/3420
Closes gh-35958

spring-core/src/main/java/org/springframework/util/MultiValueMapCollector.java

@@ -17,7 +17,7 @@
 package org.springframework.util;
 
 import java.util.EnumSet;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -28,32 +28,74 @@ import java.util.function.Supplier;
 import java.util.stream.Collector;
 
 /**
- * A {@link Collector} for building a {@link MultiValueMap} from a {@link java.util.stream.Stream}.
- * <br/>
- * Moved from {@code org.springframework.data.util.MultiValueMapCollector}.
+ * A {@link Collector} for building a {@link MultiValueMap} from a
+ * {@link java.util.stream.Stream Stream}.
  *
- * @author Jens Schauder
+ * <p>Copied from the Spring Data Commons project.
  *
- * @param <T> – the type of input elements to the reduction operation
- * @param <K> – the type of the key elements
- * @param <V> – the type of the value elements
+ * @author Jens Schauder
+ * @author Florian Hof
+ * @author Sam Brannen
+ * @since 7.0.2
+ * @param <T> the type of input elements to the reduction operation
+ * @param <K> the key type
+ * @param <V> the value element type
  */
-public class MultiValueMapCollector<T, K, V> implements Collector<T, MultiValueMap<K, V>, MultiValueMap<K, V>> {
+public final class MultiValueMapCollector<T, K, V> implements Collector<T, MultiValueMap<K, V>, MultiValueMap<K, V>> {
+
 	private final Function<T, K> keyFunction;
+
 	private final Function<T, V> valueFunction;
 
-	public MultiValueMapCollector(Function<T, K> keyFunction, Function<T, V> valueFunction) {
+
+	private MultiValueMapCollector(Function<T, K> keyFunction, Function<T, V> valueFunction) {
 		this.keyFunction = keyFunction;
 		this.valueFunction = valueFunction;
 	}
 
+
+	/**
+	 * Create a new {@code MultiValueMapCollector} from the given key and value
+	 * functions.
+	 * @param <T> the type of input elements to the reduction operation
+	 * @param <K> the key type
+	 * @param <V> the value element type
+	 * @param keyFunction a {@code Function} which converts an element of type
+	 * {@code T} to a key of type {@code K}
+	 * @param valueFunction a {@code Function} which converts an element of type
+	 * {@code T} to an element of type {@code V}; supply {@link Function#identity()}
+	 * if no conversion should be performed
+	 * @return a new {@code MultiValueMapCollector}
+	 * @see #indexingBy(Function)
+	 */
+	public static <T, K, V> MultiValueMapCollector<T, K, V> of(Function<T, K> keyFunction, Function<T, V> valueFunction) {
+		return new MultiValueMapCollector<>(keyFunction, valueFunction);
+	}
+
+	/**
+	 * Create a new {@code MultiValueMapCollector} using the given {@code indexer}.
+	 * <p>Delegates to {@link #of(Function, Function)}, supplying the given
+	 * {@code indexer} as the key function and {@link Function#identity()}
+	 * as the value function.
+	 * <p>For example, if you would like to collect the elements of a {@code Stream}
+	 * of strings into a {@link MultiValueMap} keyed by the lengths of the strings,
+	 * you could create such a {@link Collector} via
+	 * {@code MultiValueMapCollector.indexingBy(String::length)}.
+	 * @param <K> the key type
+	 * @param <V> the value element type
+	 * @param indexer a {@code Function} which converts a value of type {@code V}
+	 * to a key of type {@code K}
+	 * @return a new {@code MultiValueMapCollector} based on an {@code indexer}
+	 * @see #of(Function, Function)
+	 */
 	public static <K, V> MultiValueMapCollector<V, K, V> indexingBy(Function<V, K> indexer) {
 		return new MultiValueMapCollector<>(indexer, Function.identity());
 	}
 
+
 	@Override
 	public Supplier<MultiValueMap<K, V>> supplier() {
-		return () -> CollectionUtils.toMultiValueMap(new HashMap<K, List<V>>());
+		return () -> CollectionUtils.toMultiValueMap(new LinkedHashMap<K, List<V>>());
 	}
 
 	@Override
@@ -80,4 +122,5 @@ public class MultiValueMapCollector<T, K, V> implements Collector<T, MultiValueM
 	public Set<Characteristics> characteristics() {
 		return EnumSet.of(Characteristics.IDENTITY_FINISH, Characteristics.UNORDERED);
 	}
+
 }

spring-core/src/test/java/org/springframework/util/MultiValueMapCollectorTests.java

@@ -16,9 +16,10 @@
 
 package org.springframework.util;
 
+import java.util.function.Function;
 import java.util.stream.Stream;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -26,16 +27,33 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link MultiValueMapCollector}.
  *
  * @author Florian Hof
+ * @author Sam Brannen
+ * @since 7.0.2
  */
 class MultiValueMapCollectorTests {
 
 	@Test
-	void indexingBy() {
-		MultiValueMapCollector<String, Integer, String> collector = MultiValueMapCollector.indexingBy(String::length);
-		MultiValueMap<Integer, String> content = Stream.of("abc", "ABC", "123", "1234", "abcdef", "ABCDEF").collect(collector);
-		assertThat(content.get(3)).containsOnly("abc", "ABC", "123");
-		assertThat(content.get(4)).containsOnly("abcdef", "ABCDEF");
-		assertThat(content.get(6)).containsOnly("1234");
-		assertThat(content.get(1)).isNull();
+	void ofFactoryMethod() {
+		Function<Integer, String> keyFunction = i -> (i % 2 == 0 ? "even" :"odd");
+		Function<Integer, Integer> valueFunction = i -> -i;
+
+		var collector = MultiValueMapCollector.of(keyFunction, valueFunction);
+		var multiValueMap = Stream.of(1, 2, 3, 4, 5).collect(collector);
+
+		assertThat(multiValueMap).containsOnlyKeys("even", "odd");
+		assertThat(multiValueMap.get("odd")).containsOnly(-1, -3, -5);
+		assertThat(multiValueMap.get("even")).containsOnly(-2, -4);
 	}
+
+	@Test
+	void indexingByFactoryMethod() {
+		var collector = MultiValueMapCollector.indexingBy(String::length);
+		var multiValueMap = Stream.of("abc", "ABC", "123", "1234", "cat", "abcdef", "ABCDEF").collect(collector);
+
+		assertThat(multiValueMap).containsOnlyKeys(3, 4, 6);
+		assertThat(multiValueMap.get(3)).containsOnly("abc", "ABC", "123", "cat");
+		assertThat(multiValueMap.get(4)).containsOnly("1234");
+		assertThat(multiValueMap.get(6)).containsOnly("abcdef", "ABCDEF");
+	}
+
 }