HttpHeaders are no longer a MultiValueMap

This change removes the `MultiValueMap` nature of `HttpHeaders`, since
it inherits APIs that do not align well with underlying server
implementations. Notably, methods that allows to iterate over the whole
collection of headers are susceptible to artificially introduced
duplicates when multiple casings are used for a given header, depending
on the underlying implementation.

This change includes a dedicated key set implementation to support
iterator-based removal, and either keeps map method implementations that
are relevant or introduces header-focused methods that have a similar
responsibility (like `hasHeaderValues(String, List)` and
`containsHeaderValue(String, String)`).

In order to nudge users away from using an HttpHeaders as a Map, the
`asSingleValueMap` view is deprecated. In order to offer an escape
hatch to users that do make use of the `MultiValueMap` API, a similar
`asMultiValueMap` view is introduced but is immediately marked as
deprecated.

This change also adds map-like but header-focused assertions to
`HttpHeadersAssert`, since it cannot extend `AbstractMapAssert` anymore.

Closes gh-33913

Author: simonbasle

spring-test/src/main/java/org/springframework/mock/http/server/reactive/MockServerHttpRequest.java

@@ -298,9 +298,17 @@ public interface BaseBuilder<B extends BaseBuilder<B>> {
 		/**
 		 * Add the given header values.
 		 * @param headers the header values
+		 * @deprecated Use {@link #headers(HttpHeaders)}
 		 */
+		@Deprecated
 		B headers(MultiValueMap<String, String> headers);
 
+		/**
+		 * Add the given header values.
+		 * @param headers the header values
+		 */
+		B headers(HttpHeaders headers);
+
 		/**
 		 * Set the list of acceptable {@linkplain MediaType media types}, as
 		 * specified by the {@code Accept} header.
@@ -484,11 +492,18 @@ public BodyBuilder header(String headerName, String... headerValues) {
 		}
 
 		@Override
+		@Deprecated
 		public BodyBuilder headers(MultiValueMap<String, String> headers) {
 			this.headers.putAll(headers);
 			return this;
 		}
 
+		@Override
+		public BodyBuilder headers(HttpHeaders headers) {
+			this.headers.putAll(headers);
+			return this;
+		}
+
 		@Override
 		public BodyBuilder accept(MediaType... acceptableMediaTypes) {
 			this.headers.setAccept(Arrays.asList(acceptableMediaTypes));

spring-test/src/main/java/org/springframework/mock/web/MockPart.java

@@ -129,7 +129,7 @@ public Collection<String> getHeaders(String name) {
 
 	@Override
 	public Collection<String> getHeaderNames() {
-		return this.headers.keySet();
+		return this.headers.headerNames();
 	}
 
 	/**

spring-test/src/main/java/org/springframework/test/http/HttpHeadersAssert.java

@@ -20,10 +20,16 @@
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.time.temporal.ChronoUnit;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 
-import org.assertj.core.api.AbstractMapAssert;
+import org.assertj.core.api.AbstractCollectionAssert;
+import org.assertj.core.api.AbstractObjectAssert;
 import org.assertj.core.api.Assertions;
+import org.assertj.core.api.ObjectAssert;
+import org.assertj.core.presentation.Representation;
+import org.assertj.core.presentation.StandardRepresentation;
 
 import org.springframework.http.HttpHeaders;
 
@@ -34,54 +40,87 @@
  * @author Stephane Nicoll
  * @since 6.2
  */
-public class HttpHeadersAssert extends AbstractMapAssert<HttpHeadersAssert, HttpHeaders, String, List<String>> {
+public class HttpHeadersAssert extends AbstractObjectAssert<HttpHeadersAssert, HttpHeaders> {
 
 	private static final ZoneId GMT = ZoneId.of("GMT");
 
+	private final AbstractCollectionAssert<?, Collection<? extends String>, String, ObjectAssert<String>> namesAssert;
+
 
 	public HttpHeadersAssert(HttpHeaders actual) {
 		super(actual, HttpHeadersAssert.class);
 		as("HTTP headers");
+		withRepresentation(new Representation() {
+			@Override
+			public String toStringOf(Object object) {
+				if (object instanceof HttpHeaders headers) {
+					return headers.toString();
+				}
+				return StandardRepresentation.STANDARD_REPRESENTATION.toStringOf(object);
+			}
+		});
+		this.namesAssert = Assertions.assertThat(actual.headerNames())
+				.as("HTTP header names");
 	}
 
 	/**
 	 * Verify that the actual HTTP headers contain a header with the given
 	 * {@code name}.
 	 * @param name the name of an expected HTTP header
-	 * @see #containsKey
 	 */
 	public HttpHeadersAssert containsHeader(String name) {
-		return containsKey(name);
+		this.namesAssert
+				.as("check headers contain HTTP header '%s'", name)
+				.contains(name);
+		return this.myself;
 	}
 
 	/**
 	 * Verify that the actual HTTP headers contain the headers with the given
 	 * {@code names}.
 	 * @param names the names of expected HTTP headers
-	 * @see #containsKeys
 	 */
 	public HttpHeadersAssert containsHeaders(String... names) {
-		return containsKeys(names);
+		this.namesAssert
+				.as("check headers contain HTTP headers '%s'", Arrays.toString(names))
+				.contains(names);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the actual HTTP headers contain only the headers with the
+	 * given {@code names}, in any order and in a case-insensitive manner.
+	 * @param names the names of expected HTTP headers
+	 */
+	public HttpHeadersAssert containsOnlyHeaders(String... names) {
+		this.namesAssert
+				.as("check headers contains only HTTP headers '%s'", Arrays.toString(names))
+				.containsOnly(names);
+		return this.myself;
 	}
 
 	/**
 	 * Verify that the actual HTTP headers do not contain a header with the
 	 * given {@code name}.
 	 * @param name the name of an HTTP header that should not be present
-	 * @see #doesNotContainKey
 	 */
 	public HttpHeadersAssert doesNotContainHeader(String name) {
-		return doesNotContainKey(name);
+		this.namesAssert
+				.as("check headers does not contain HTTP header '%s'", name)
+				.doesNotContain(name);
+		return this.myself;
 	}
 
 	/**
 	 * Verify that the actual HTTP headers do not contain any of the headers
 	 * with the given {@code names}.
 	 * @param names the names of HTTP headers that should not be present
-	 * @see #doesNotContainKeys
 	 */
 	public HttpHeadersAssert doesNotContainsHeaders(String... names) {
-		return doesNotContainKeys(names);
+		this.namesAssert
+				.as("check headers does not contain HTTP headers '%s'", Arrays.toString(names))
+				.doesNotContain(names);
+		return this.myself;
 	}
 
 	/**
@@ -91,7 +130,7 @@ public HttpHeadersAssert doesNotContainsHeaders(String... names) {
 	 * @param value the expected value of the header
 	 */
 	public HttpHeadersAssert hasValue(String name, String value) {
-		containsKey(name);
+		containsHeader(name);
 		Assertions.assertThat(this.actual.getFirst(name))
 				.as("check primary value for HTTP header '%s'", name)
 				.isEqualTo(value);
@@ -105,7 +144,7 @@ public HttpHeadersAssert hasValue(String name, String value) {
 	 * @param value the expected value of the header
 	 */
 	public HttpHeadersAssert hasValue(String name, long value) {
-		containsKey(name);
+		containsHeader(name);
 		Assertions.assertThat(this.actual.getFirst(name))
 				.as("check primary long value for HTTP header '%s'", name)
 				.asLong().isEqualTo(value);
@@ -119,11 +158,149 @@ public HttpHeadersAssert hasValue(String name, long value) {
 	 * @param value the expected value of the header
 	 */
 	public HttpHeadersAssert hasValue(String name, Instant value) {
-		containsKey(name);
+		containsHeader(name);
 		Assertions.assertThat(this.actual.getFirstZonedDateTime(name))
 				.as("check primary date value for HTTP header '%s'", name)
 				.isCloseTo(ZonedDateTime.ofInstant(value, GMT), Assertions.within(999, ChronoUnit.MILLIS));
 		return this.myself;
 	}
 
+	/**
+	 * Verify that the given header has a full list of values exactly equal to
+	 * the given list of values, and in the same order.
+	 * @param name the considered header name (case-insensitive)
+	 * @param values the exhaustive list of expected values
+	 */
+	public HttpHeadersAssert hasExactlyValues(String name, List<String> values) {
+		containsHeader(name);
+		Assertions.assertThat(this.actual.get(name))
+				.as("check all values of HTTP header '%s'", name)
+				.containsExactlyElementsOf(values);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the given header has a full list of values exactly equal to
+	 * the given list of values, in any order.
+	 * @param name the considered header name (case-insensitive)
+	 * @param values the exhaustive list of expected values
+	 */
+	public HttpHeadersAssert hasExactlyValuesInAnyOrder(String name, List<String> values) {
+		containsHeader(name);
+		Assertions.assertThat(this.actual.get(name))
+				.as("check all values of HTTP header '%s' in any order", name)
+				.containsExactlyInAnyOrderElementsOf(values);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that headers are empty and no headers are present.
+	 */
+	public HttpHeadersAssert isEmpty() {
+		this.namesAssert
+				.as("check headers are empty")
+				.isEmpty();
+		return this.myself;
+	}
+
+	/**
+	 * Verify that headers are not empty and at least one header is present.
+	 */
+	public HttpHeadersAssert isNotEmpty() {
+		this.namesAssert
+				.as("check headers are not empty")
+				.isNotEmpty();
+		return this.myself;
+	}
+
+	/**
+	 * Verify that there is exactly {@code expected} headers present, when
+	 * considering header names in a case-insensitive manner.
+	 * @param expected the expected number of headers
+	 */
+	public HttpHeadersAssert hasSize(int expected) {
+		this.namesAssert
+				.as("check headers have size '%i'", expected)
+				.hasSize(expected);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number of headers present is strictly greater than the
+	 * given boundary, when considering header names in a case-insensitive
+	 * manner.
+	 * @param boundary the given value to compare actual header size to
+	 */
+	public HttpHeadersAssert hasSizeGreaterThan(int boundary) {
+		this.namesAssert
+				.as("check headers have size > '%i'", boundary)
+				.hasSizeGreaterThan(boundary);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number of headers present is greater or equal to the
+	 * given boundary, when considering header names in a case-insensitive
+	 * manner.
+	 * @param boundary the given value to compare actual header size to
+	 */
+	public HttpHeadersAssert hasSizeGreaterThanOrEqualTo(int boundary) {
+		this.namesAssert
+				.as("check headers have size >= '%i'", boundary)
+				.hasSizeGreaterThanOrEqualTo(boundary);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number of headers present is strictly less than the
+	 * given boundary, when considering header names in a case-insensitive
+	 * manner.
+	 * @param boundary the given value to compare actual header size to
+	 */
+	public HttpHeadersAssert hasSizeLessThan(int boundary) {
+		this.namesAssert
+				.as("check headers have size < '%i'", boundary)
+				.hasSizeLessThan(boundary);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number of headers present is less than or equal to the
+	 * given boundary, when considering header names in a case-insensitive
+	 * manner.
+	 * @param boundary the given value to compare actual header size to
+	 */
+	public HttpHeadersAssert hasSizeLessThanOrEqualTo(int boundary) {
+		this.namesAssert
+				.as("check headers have size <= '%i'", boundary)
+				.hasSizeLessThanOrEqualTo(boundary);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number of headers present is between the given boundaries
+	 * (inclusive), when considering header names in a case-insensitive manner.
+	 * @param lowerBoundary the lower boundary compared to which actual size
+	 * should be greater than or equal to
+	 * @param higherBoundary the higher boundary compared to which actual size
+	 * should be less than or equal to
+	 */
+	public HttpHeadersAssert hasSizeBetween(int lowerBoundary, int higherBoundary) {
+		this.namesAssert
+				.as("check headers have size between '%i' and '%i'", lowerBoundary, higherBoundary)
+				.hasSizeBetween(lowerBoundary, higherBoundary);
+		return this.myself;
+	}
+
+	/**
+	 * Verify that the number actual headers is the same as in the given
+	 * {@code HttpHeaders}.
+	 * @param other the {@code HttpHeaders} to compare size with
+	 */
+	public HttpHeadersAssert hasSameSizeAs(HttpHeaders other) {
+		this.namesAssert
+				.as("check headers have same size as '%s'", other)
+				.hasSize(other.size());
+		return this.myself;
+	}
 }

spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java

@@ -24,6 +24,7 @@
 
 import org.hamcrest.Matcher;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.test.web.client.MockRestServiceServer;
@@ -162,7 +163,7 @@ public static RequestMatcher queryParamList(String name, Matcher<? super List<St
 	public static RequestMatcher queryParam(String name, Matcher<? super String>... matchers) {
 		return request -> {
 			MultiValueMap<String, String> params = getQueryParams(request);
-			assertValueCount("query param", name, params, matchers.length);
+			assertValueCount(name, params, matchers.length);
 			for (int i = 0 ; i < matchers.length; i++) {
 				assertThat("Query param", params.get(name).get(i), matchers[i]);
 			}
@@ -190,7 +191,7 @@ public static RequestMatcher queryParam(String name, Matcher<? super String>...
 	public static RequestMatcher queryParam(String name, String... expectedValues) {
 		return request -> {
 			MultiValueMap<String, String> params = getQueryParams(request);
-			assertValueCount("query param", name, params, expectedValues.length);
+			assertValueCount(name, params, expectedValues.length);
 			for (int i = 0 ; i < expectedValues.length; i++) {
 				assertEquals("Query param [" + name + "]", expectedValues[i], params.get(name).get(i));
 			}
@@ -246,7 +247,7 @@ public static RequestMatcher headerList(String name, Matcher<? super List<String
 	@SafeVarargs
 	public static RequestMatcher header(String name, Matcher<? super String>... matchers) {
 		return request -> {
-			assertValueCount("header", name, request.getHeaders(), matchers.length);
+			assertValueCount(name, request.getHeaders(), matchers.length);
 			List<String> headerValues = request.getHeaders().get(name);
 			Assert.state(headerValues != null, "No header values");
 			for (int i = 0; i < matchers.length; i++) {
@@ -273,7 +274,7 @@ public static RequestMatcher header(String name, Matcher<? super String>... matc
 	 */
 	public static RequestMatcher header(String name, String... expectedValues) {
 		return request -> {
-			assertValueCount("header", name, request.getHeaders(), expectedValues.length);
+			assertValueCount(name, request.getHeaders(), expectedValues.length);
 			List<String> headerValues = request.getHeaders().get(name);
 			Assert.state(headerValues != null, "No header values");
 			for (int i = 0; i < expectedValues.length; i++) {
@@ -356,11 +357,22 @@ public static XpathRequestMatchers xpath(String expression, Map<String, String>
 	}
 
 
-	private static void assertValueCount(
-			String valueType, String name, MultiValueMap<String, String> map, int count) {
+	private static void assertValueCount(String name, MultiValueMap<String, String> map, int count) {
 
 		List<String> values = map.get(name);
-		String message = "Expected " + valueType + " <" + name + ">";
+		String message = "Expected query param <" + name + ">";
+		if (values == null) {
+			fail(message + " to exist but was null");
+		}
+		else if (count > values.size()) {
+			fail(message + " to have at least <" + count + "> values but found " + values);
+		}
+	}
+
+	private static void assertValueCount(String name, HttpHeaders headers, int count) {
+
+		List<String> values = headers.get(name);
+		String message = "Expected header <" + name + ">";
 		if (values == null) {
 			fail(message + " to exist but was null");
 		}