Consistent MultiValueMap behavior for empty list values

This commit extracts MultiValueMapAdapter from CollectionUtils and reuses its implementation as base class of LinkedMultiValueMap.

Closes gh-25140

Author: jhoeller

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -16,15 +16,13 @@
 
 package org.springframework.util;
 
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -379,25 +377,28 @@ public static <E> Iterator<E> toIterator(@Nullable Enumeration<E> enumeration) {
 
 	/**
 	 * Adapt a {@code Map<K, List<V>>} to an {@code MultiValueMap<K, V>}.
-	 * @param map the original map
-	 * @return the multi-value map
+	 * @param targetMap the original map
+	 * @return the adapted multi-value map (wrapping the original map)
 	 * @since 3.1
 	 */
-	public static <K, V> MultiValueMap<K, V> toMultiValueMap(Map<K, List<V>> map) {
-		return new MultiValueMapAdapter<>(map);
+	public static <K, V> MultiValueMap<K, V> toMultiValueMap(Map<K, List<V>> targetMap) {
+		Assert.notNull(targetMap, "'targetMap' must not be null");
+		return new MultiValueMapAdapter<>(targetMap);
 	}
 
 	/**
 	 * Return an unmodifiable view of the specified multi-value map.
-	 * @param  map the map for which an unmodifiable view is to be returned.
-	 * @return an unmodifiable view of the specified multi-value map.
+	 * @param targetMap the map for which an unmodifiable view is to be returned.
+	 * @return an unmodifiable view of the specified multi-value map
 	 * @since 3.1
 	 */
 	@SuppressWarnings("unchecked")
-	public static <K, V> MultiValueMap<K, V> unmodifiableMultiValueMap(MultiValueMap<? extends K, ? extends V> map) {
-		Assert.notNull(map, "'map' must not be null");
-		Map<K, List<V>> result = new LinkedHashMap<>(map.size());
-		map.forEach((key, value) -> {
+	public static <K, V> MultiValueMap<K, V> unmodifiableMultiValueMap(
+			MultiValueMap<? extends K, ? extends V> targetMap) {
+
+		Assert.notNull(targetMap, "'targetMap' must not be null");
+		Map<K, List<V>> result = new LinkedHashMap<>(targetMap.size());
+		targetMap.forEach((key, value) -> {
 			List<? extends V> values = Collections.unmodifiableList(value);
 			result.put(key, (List<V>) values);
 		});
@@ -434,141 +435,4 @@ public void remove() throws UnsupportedOperationException {
 	}
 
 
-	/**
-	 * Adapts a Map to the MultiValueMap contract.
-	 */
-	@SuppressWarnings("serial")
-	private static class MultiValueMapAdapter<K, V> implements MultiValueMap<K, V>, Serializable {
-
-		private final Map<K, List<V>> map;
-
-		public MultiValueMapAdapter(Map<K, List<V>> map) {
-			Assert.notNull(map, "'map' must not be null");
-			this.map = map;
-		}
-
-		@Override
-		@Nullable
-		public V getFirst(K key) {
-			List<V> values = this.map.get(key);
-			return (values != null ? values.get(0) : null);
-		}
-
-		@Override
-		public void add(K key, @Nullable V value) {
-			List<V> values = this.map.computeIfAbsent(key, k -> new LinkedList<>());
-			values.add(value);
-		}
-
-		@Override
-		public void addAll(K key, List<? extends V> values) {
-			List<V> currentValues = this.map.computeIfAbsent(key, k -> new LinkedList<>());
-			currentValues.addAll(values);
-		}
-
-		@Override
-		public void addAll(MultiValueMap<K, V> values) {
-			for (Entry<K, List<V>> entry : values.entrySet()) {
-				addAll(entry.getKey(), entry.getValue());
-			}
-		}
-
-		@Override
-		public void set(K key, @Nullable V value) {
-			List<V> values = new LinkedList<>();
-			values.add(value);
-			this.map.put(key, values);
-		}
-
-		@Override
-		public void setAll(Map<K, V> values) {
-			values.forEach(this::set);
-		}
-
-		@Override
-		public Map<K, V> toSingleValueMap() {
-			LinkedHashMap<K, V> singleValueMap = new LinkedHashMap<>(this.map.size());
-			this.map.forEach((key, value) -> singleValueMap.put(key, value.get(0)));
-			return singleValueMap;
-		}
-
-		@Override
-		public int size() {
-			return this.map.size();
-		}
-
-		@Override
-		public boolean isEmpty() {
-			return this.map.isEmpty();
-		}
-
-		@Override
-		public boolean containsKey(Object key) {
-			return this.map.containsKey(key);
-		}
-
-		@Override
-		public boolean containsValue(Object value) {
-			return this.map.containsValue(value);
-		}
-
-		@Override
-		public List<V> get(Object key) {
-			return this.map.get(key);
-		}
-
-		@Override
-		public List<V> put(K key, List<V> value) {
-			return this.map.put(key, value);
-		}
-
-		@Override
-		public List<V> remove(Object key) {
-			return this.map.remove(key);
-		}
-
-		@Override
-		public void putAll(Map<? extends K, ? extends List<V>> map) {
-			this.map.putAll(map);
-		}
-
-		@Override
-		public void clear() {
-			this.map.clear();
-		}
-
-		@Override
-		public Set<K> keySet() {
-			return this.map.keySet();
-		}
-
-		@Override
-		public Collection<List<V>> values() {
-			return this.map.values();
-		}
-
-		@Override
-		public Set<Entry<K, List<V>>> entrySet() {
-			return this.map.entrySet();
-		}
-
-		@Override
-		public boolean equals(Object other) {
-			if (this == other) {
-				return true;
-			}
-			return this.map.equals(other);
-		}
-
-		@Override
-		public int hashCode() {
-			return this.map.hashCode();
-		}
-
-		@Override
-		public String toString() {
-			return this.map.toString();
-		}
-	}
-
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -220,8 +220,8 @@ public LinkedCaseInsensitiveMap<V> clone() {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
-		return this.targetMap.equals(obj);
+	public boolean equals(@Nullable Object other) {
+		return (this == other || this.targetMap.equals(other));
 	}
 
 	@Override