Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

HttpHeaders are no longer a MultiValueMap

Browse Source 
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
This commit is contained in:
Simon Baslé
2024-12-02 14:55:27 +01:00
parent 1e0ef99b0c
commit 0c6f5d7d29
100 changed files with 1116 additions and 508 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java
View File

@@ -495,10 +495,10 @@ final class DefaultWebClient implements WebClient {
}
private void initHeaders(HttpHeaders out) {
if (!CollectionUtils.isEmpty(defaultHeaders)) {
if (defaultHeaders != null && !defaultHeaders.isEmpty()) {
out.putAll(defaultHeaders);
}
if (!CollectionUtils.isEmpty(this.headers)) {
if (this.headers != null && !this.headers.isEmpty()) {
out.putAll(this.headers);
}
}

2
spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientRequestException.java
View File

@@ -58,7 +58,7 @@ public class WebClientRequestException extends WebClientException {
*/
private static HttpHeaders copy(HttpHeaders headers) {
HttpHeaders result = new HttpHeaders();
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
for (Map.Entry<String, List<String>> entry : headers.headerSet()) {
for (String value : entry.getValue()) {
result.add(entry.getKey(), value);
}

2
spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientResponseException.java
View File

@@ -150,7 +150,7 @@ public class WebClientResponseException extends WebClientException {
}
else {
HttpHeaders result = new HttpHeaders();
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
for (Map.Entry<String, List<String>> entry : headers.headerSet()) {
for (String value : entry.getValue()) {
result.add(entry.getKey(), value);
}

6
spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java
View File

@@ -357,6 +357,12 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
dst.putAll(src);
}
}
private static void copy(HttpHeaders src, HttpHeaders dst) {
if (!src.isEmpty()) {
dst.putAll(src);
}
}
}

2
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ExtendedWebExchangeDataBinder.java
View File

@@ -85,7 +85,7 @@ public class ExtendedWebExchangeDataBinder extends WebExchangeDataBinder {
vars.forEach((key, value) -> addValueIfNotPresent(map, "URI variable", key, value));
}
HttpHeaders headers = exchange.getRequest().getHeaders();
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
for (Map.Entry<String, List<String>> entry : headers.headerSet()) {
String name = entry.getKey();
if (!this.headerPredicate.test(entry.getKey())) {
continue;

9
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestHeaderMapMethodArgumentResolver.java
View File

@@ -55,17 +55,22 @@ public class RequestHeaderMapMethodArgumentResolver extends HandlerMethodArgumen
}
private boolean allParams(RequestHeader annotation, Class<?> type) {
return Map.class.isAssignableFrom(type);
return Map.class.isAssignableFrom(type) || HttpHeaders.class.isAssignableFrom(type);
}
@SuppressWarnings("deprecation")
@Override
public Object resolveArgumentValue(
MethodParameter methodParameter, BindingContext context, ServerWebExchange exchange) {
boolean isMultiValueMap = MultiValueMap.class.isAssignableFrom(methodParameter.getParameterType());
HttpHeaders headers = exchange.getRequest().getHeaders();
return (isMultiValueMap ? headers : headers.toSingleValueMap());
if (isMultiValueMap) {
return headers.asMultiValueMap();
}
boolean isHttpHeaders = HttpHeaders.class.isAssignableFrom(methodParameter.getParameterType());
return (isHttpHeaders ? headers : headers.toSingleValueMap());
}
}

3
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestHeaderMethodArgumentResolver.java
View File

@@ -25,6 +25,7 @@ import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.core.MethodParameter;
import org.springframework.core.ReactiveAdapterRegistry;
import org.springframework.core.convert.ConversionService;
import org.springframework.http.HttpHeaders;
import org.springframework.util.Assert;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.server.MissingRequestValueException;
@@ -68,7 +69,7 @@ public class RequestHeaderMethodArgumentResolver extends AbstractNamedValueSyncA
}
private boolean singleParam(RequestHeader annotation, Class<?> type) {
return !Map.class.isAssignableFrom(type);
return !Map.class.isAssignableFrom(type) && !HttpHeaders.class.isAssignableFrom(type);
}
@Override

2
spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java
View File

@@ -473,7 +473,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp
@Override
public HttpHeaders getHeaders() {
if (!super.getHeaders().containsKey(HttpHeaders.CONTENT_TYPE)) {
if (!super.getHeaders().containsHeader(HttpHeaders.CONTENT_TYPE)) {
return super.getHeaders();
}
// Content-type is set, ignore further updates

2
spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java
View File

@@ -92,7 +92,7 @@ public class JettyWebSocketClient implements WebSocketClient, Lifecycle {
ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
upgradeRequest.setSubProtocols(handler.getSubProtocols());
if (headers != null) {
headers.keySet().forEach(header -> upgradeRequest.setHeader(header, headers.getValuesAsList(header)));
headers.headerNames().forEach(header -> upgradeRequest.setHeader(header, headers.getValuesAsList(header)));
}
final AtomicReference<HandshakeInfo> handshakeInfo = new AtomicReference<>();

4
spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/StandardWebSocketClient.java
View File

@@ -179,12 +179,12 @@ public class StandardWebSocketClient implements WebSocketClient {
@Override
public void beforeRequest(Map<String, List<String>> requestHeaders) {
requestHeaders.putAll(this.requestHeaders);
this.requestHeaders.forEach(requestHeaders::put);
}
@Override
public void afterResponse(HandshakeResponse response) {
this.responseHeaders.putAll(response.getHeaders());
response.getHeaders().forEach(this.responseHeaders::put);
}
}

2
spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/UndertowWebSocketClient.java
View File

@@ -242,7 +242,7 @@ public class UndertowWebSocketClient implements WebSocketClient {
@Override
public void beforeRequest(Map<String, List<String>> headers) {
headers.putAll(this.requestHeaders);
this.requestHeaders.forEach(headers::put);
if (this.delegate != null) {
this.delegate.beforeRequest(headers);
}

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestBuilderTests.java
View File

@@ -70,7 +70,7 @@ class DefaultClientRequestBuilderTests {
assertThat(result.url()).isEqualTo(DEFAULT_URL);
assertThat(result.method()).isEqualTo(GET);
assertThat(result.headers()).hasSize(1);
assertThat(result.headers().size()).isOne();
assertThat(result.headers().getFirst("foo")).isEqualTo("baar");
assertThat(result.cookies()).hasSize(1);
assertThat(result.cookies().getFirst("baz")).isEqualTo("quux");

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java
View File

@@ -88,7 +88,7 @@ class DefaultClientResponseBuilderTests {
assertThat(result.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
assertThat(result.headers().asHttpHeaders()).hasSize(3);
assertThat(result.headers().asHttpHeaders().size()).isEqualTo(3);
assertThat(result.headers().asHttpHeaders().getFirst("foo")).isEqualTo("baar");
assertThat(result.headers().asHttpHeaders().getFirst("bar")).isEqualTo("baz");
assertThat(result.cookies()).hasSize(1);

4
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java
View File

@@ -338,7 +338,7 @@ class DefaultClientResponseTests {
WebClientResponseException exception = resultMono.block();
assertThat(exception.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
assertThat(exception.getMessage()).isEqualTo("404 Not Found from UNKNOWN https://example.org:9999/app/path");
assertThat(exception.getHeaders()).containsExactly(entry("Content-Type", List.of("text/plain")));
assertThat(exception.getHeaders().asMultiValueMap()).containsExactly(entry("Content-Type", List.of("text/plain")));
assertThat(exception.getResponseBodyAsByteArray()).isEqualTo(bytes);
}
@@ -397,7 +397,7 @@ class DefaultClientResponseTests {
WebClientResponseException exception = (WebClientResponseException) t;
assertThat(exception.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
assertThat(exception.getMessage()).isEqualTo("404 Not Found");
assertThat(exception.getHeaders()).containsExactly(entry("Content-Type",List.of("text/plain")));
assertThat(exception.getHeaders().asMultiValueMap()).containsExactly(entry("Content-Type",List.of("text/plain")));
assertThat(exception.getResponseBodyAsByteArray()).isEqualTo(bytes);
})
.verify();

6
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java
View File

@@ -295,17 +295,17 @@ public class DefaultWebClientTests {
WebClient.Builder builder1 = client1.mutate();
builder1.filters(filters -> assertThat(filters).hasSize(1));
builder1.defaultHeaders(headers -> assertThat(headers).hasSize(1));
builder1.defaultHeaders(headers -> assertThat(headers.size()).isOne());
builder1.defaultCookies(cookies -> assertThat(cookies).hasSize(1));
WebClient.Builder builder2 = client2.mutate();
builder2.filters(filters -> assertThat(filters).hasSize(2));
builder2.defaultHeaders(headers -> assertThat(headers).hasSize(2));
builder2.defaultHeaders(headers -> assertThat(headers.size()).isEqualTo(2));
builder2.defaultCookies(cookies -> assertThat(cookies).hasSize(2));
WebClient.Builder builder1a = client1a.mutate();
builder1a.filters(filters -> assertThat(filters).hasSize(2));
builder1a.defaultHeaders(headers -> assertThat(headers).hasSize(2));
builder1a.defaultHeaders(headers -> assertThat(headers.size()).isEqualTo(2));
builder1a.defaultCookies(cookies -> assertThat(cookies).hasSize(2));
}

12
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctionsTests.java
View File

@@ -103,13 +103,13 @@ class ExchangeFilterFunctionsTests {
ClientResponse response = mock();
ExchangeFunction exchange = r -> {
assertThat(r.headers().containsKey(HttpHeaders.AUTHORIZATION)).isTrue();
assertThat(r.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isTrue();
assertThat(r.headers().getFirst(HttpHeaders.AUTHORIZATION)).startsWith("Basic ");
return Mono.just(response);
};
ExchangeFilterFunction auth = ExchangeFilterFunctions.basicAuthentication("foo", "bar");
assertThat(request.headers().containsKey(HttpHeaders.AUTHORIZATION)).isFalse();
assertThat(request.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isFalse();
ClientResponse result = auth.filter(request, exchange).block();
assertThat(result).isEqualTo(response);
}
@@ -133,13 +133,13 @@ class ExchangeFilterFunctionsTests {
ClientResponse response = mock();
ExchangeFunction exchange = r -> {
assertThat(r.headers().containsKey(HttpHeaders.AUTHORIZATION)).isTrue();
assertThat(r.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isTrue();
assertThat(r.headers().getFirst(HttpHeaders.AUTHORIZATION)).startsWith("Basic ");
return Mono.just(response);
};
ExchangeFilterFunction auth = ExchangeFilterFunctions.basicAuthentication();
assertThat(request.headers().containsKey(HttpHeaders.AUTHORIZATION)).isFalse();
assertThat(request.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isFalse();
ClientResponse result = auth.filter(request, exchange).block();
assertThat(result).isEqualTo(response);
}
@@ -151,12 +151,12 @@ class ExchangeFilterFunctionsTests {
ClientResponse response = mock();
ExchangeFunction exchange = r -> {
assertThat(r.headers().containsKey(HttpHeaders.AUTHORIZATION)).isFalse();
assertThat(r.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isFalse();
return Mono.just(response);
};
ExchangeFilterFunction auth = ExchangeFilterFunctions.basicAuthentication();
assertThat(request.headers().containsKey(HttpHeaders.AUTHORIZATION)).isFalse();
assertThat(request.headers().containsHeader(HttpHeaders.AUTHORIZATION)).isFalse();
ClientResponse result = auth.filter(request, exchange).block();
assertThat(result).isEqualTo(response);
}

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java
View File

@@ -204,7 +204,7 @@ class WebClientDataBufferAllocatingTests extends AbstractDataBufferAllocatingTes
StepVerifier.create(result)
.assertNext(entity -> {
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.CREATED);
assertThat(entity.getHeaders()).containsEntry("Foo", Collections.singletonList("bar"));
assertThat(entity.getHeaders().hasHeaderValues("Foo", Collections.singletonList("bar"))).isTrue();
assertThat(entity.getBody()).isNull();
})
.expectComplete()

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientObservationTests.java
View File

@@ -91,7 +91,7 @@ class WebClientObservationTests {
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasLowCardinalityKeyValue("uri", "/base/resource/{id}");
assertThat(clientRequest.headers()).containsEntry("foo", Collections.singletonList("bar"));
assertThat(clientRequest.headers().hasHeaderValues("foo", Collections.singletonList("bar"))).isTrue();
}
@Test

2
spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilderTests.java
View File

@@ -73,7 +73,7 @@ class DefaultServerRequestBuilderTests {
assertThat(result.uri()).isEqualTo(uri);
assertThat(result.requestPath().pathWithinApplication().value()).isEqualTo("/bar");
assertThat(result.requestPath().contextPath().value()).isEqualTo("/foo");
assertThat(result.headers().asHttpHeaders()).hasSize(1);
assertThat(result.headers().asHttpHeaders().size()).isOne();
assertThat(result.headers().asHttpHeaders().getFirst("foo")).isEqualTo("baar");
assertThat(result.cookies()).hasSize(1);
assertThat(result.cookies().getFirst("baz").getValue()).isEqualTo("quux");

6
spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java
View File

@@ -417,7 +417,7 @@ class ResourceWebHandlerTests {
this.handler.handle(exchange).block(TIMEOUT);
HttpHeaders headers = exchange.getResponse().getHeaders();
assertThat(headers.containsKey("Last-Modified")).isTrue();
assertThat(headers.containsHeader("Last-Modified")).isTrue();
assertThat(resourceLastModifiedDate("test/foo.css") / 1000).isEqualTo(headers.getLastModified() / 1000);
}
@@ -448,7 +448,7 @@ class ResourceWebHandlerTests {
MockServerHttpResponse response = exchange.getResponse();
assertThat(response.getHeaders().getCacheControl()).isEqualTo("no-store");
assertThat(response.getHeaders().containsKey("Last-Modified")).isTrue();
assertThat(response.getHeaders().containsHeader("Last-Modified")).isTrue();
assertThat(resourceLastModifiedDate("test/foo.css") / 1000).isEqualTo(response.getHeaders().getLastModified() / 1000);
}
@@ -561,7 +561,7 @@ class ResourceWebHandlerTests {
HttpHeaders headers = exchange.getResponse().getHeaders();
assertThat(headers.getContentType()).isEqualTo(MediaType.parseMediaType("text/css"));
assertThat(headers.getContentLength()).isEqualTo(17);
assertThat(headers.containsKey("Last-Modified")).isFalse();
assertThat(headers.containsHeader("Last-Modified")).isFalse();
assertResponseBody(exchange, "h1 { color:red; }");
}

2
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/AbstractRequestMappingIntegrationTests.java
View File

@@ -123,7 +123,7 @@ public abstract class AbstractRequestMappingIntegrationTests extends AbstractHtt
}
private void addHeaders(RequestEntity.HeadersBuilder<?> builder, HttpHeaders headers) {
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
for (Map.Entry<String, List<String>> entry : headers.headerSet()) {
for (String value : entry.getValue()) {
builder.header(entry.getKey(), value);
}

12
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java
View File

@@ -157,7 +157,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT);
assertThat(exchange.getResponse().getHeaders()).isEmpty();
assertThat(exchange.getResponse().getHeaders().isEmpty()).isTrue();
assertResponseBodyIsEmpty(exchange);
}
@@ -171,7 +171,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(exchange.getResponse().getHeaders()).hasSize(1);
assertThat(exchange.getResponse().getHeaders().size()).isOne();
assertThat(exchange.getResponse().getHeaders().getFirst("Allow")).isEqualTo("GET,POST,OPTIONS");
assertResponseBodyIsEmpty(exchange);
}
@@ -186,7 +186,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.CREATED);
assertThat(exchange.getResponse().getHeaders()).hasSize(1);
assertThat(exchange.getResponse().getHeaders().size()).isOne();
assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(location);
assertResponseBodyIsEmpty(exchange);
}
@@ -236,7 +236,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
assertThat(exchange.getResponse().getHeaders()).hasSize(3);
assertThat(exchange.getResponse().getHeaders().size()).isEqualTo(3);
assertThat(exchange.getResponse().getHeaders().get("foo")).containsExactly("bar");
assertThat(exchange.getResponse().getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_PROBLEM_JSON);
assertResponseBody(exchange,
@@ -256,7 +256,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
assertThat(exchange.getResponse().getHeaders()).hasSize(2);
assertThat(exchange.getResponse().getHeaders().size()).isEqualTo(2);
assertThat(exchange.getResponse().getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_PROBLEM_JSON);
assertResponseBody(exchange,
"{\"type\":\"about:blank\"," +
@@ -385,7 +385,7 @@ class ResponseEntityResultHandlerTests {
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(exchange.getResponse().getHeaders()).hasSize(1);
assertThat(exchange.getResponse().getHeaders().size()).isOne();
assertThat(exchange.getResponse().getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
assertResponseBodyIsEmpty(exchange);
}

4
spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DefaultRenderingBuilderTests.java
View File

@@ -41,7 +41,7 @@ class DefaultRenderingBuilderTests {
assertThat(rendering.view()).isEqualTo("abc");
assertThat(rendering.modelAttributes()).isEqualTo(Collections.emptyMap());
assertThat(rendering.status()).isNull();
assertThat(rendering.headers()).isEmpty();
assertThat(rendering.headers().isEmpty()).isTrue();
}
@Test
@@ -97,7 +97,7 @@ class DefaultRenderingBuilderTests {
void header() {
Rendering rendering = Rendering.view("foo").header("foo", "bar").build();
assertThat(rendering.headers()).hasSize(1);
assertThat(rendering.headers().size()).isOne();
assertThat(rendering.headers().get("foo")).isEqualTo(Collections.singletonList("bar"));
}