From 5034d1e7b337cae294e9f4914da1dcbc480114ee Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 25 Jul 2019 13:02:09 +0200 Subject: [PATCH] Introduce ServerHttpRequest.Builder.header() variant for setting headers Prior to this commit, the `header(String, String)` method in the ServerHttpRequest.Builder API actually added a header value instead of setting or overriding a header value. Since this conflicted with the stated behavior in the Javadoc as well as the original intention of the method, we have decided to introduce an overloaded variant `header(String, String...)` which accepts a var-args list of header values to set or override. In addition, this commit deprecates the existing `header(String, String)` method for removal in Spring Framework 5.2. In order not to be a breaking change for custom implementations of the builder API, this commit implements the new `header(String, String...)` method as an interface `default` method, with the intent to remove the default implementation in Spring Framework 5.2 closes gh-23333 --- .../DefaultServerHttpRequestBuilder.java | 1 + .../server/reactive/ServerHttpRequest.java | 29 +++++++++-- .../reactive/ServerHttpRequestTests.java | 50 ++++++++++++++++++- 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java index acb57af710..45edd3e0b6 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java @@ -111,6 +111,7 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { } @Override + @SuppressWarnings("deprecation") public ServerHttpRequest.Builder header(String key, String value) { this.httpHeaders.add(key, value); return this; diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java index 2176defa48..c71cc0636a 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -18,6 +18,7 @@ package org.springframework.http.server.reactive; import java.net.InetSocketAddress; import java.net.URI; +import java.util.Arrays; import java.util.function.Consumer; import org.springframework.http.HttpCookie; @@ -34,6 +35,7 @@ import org.springframework.util.MultiValueMap; * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Sam Brannen * @since 5.0 */ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage { @@ -137,9 +139,29 @@ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage Builder contextPath(String contextPath); /** - * Set or override the specified header. + * Add the given, single header value under the given name. + * @param headerName the header name + * @param headerValue the header value + * @deprecated This method will be removed in Spring Framework 5.2 in + * favor of {@link #header(String, String...)}. */ - Builder header(String key, String value); + @Deprecated + Builder header(String headerName, String headerValue); + + /** + * Set or override the specified header values under the given name. + *

If you need to set a single header value, you may invoke this + * method with an explicit one-element array — for example, + * header("key", new String[] { "value" }) — or you + * may choose to use {@link #headers(Consumer)} for greater control. + * @param headerName the header name + * @param headerValues the header values + * @since 5.1.9 + * @see #headers(Consumer) + */ + default Builder header(String headerName, String... headerValues) { + return headers(httpHeaders -> httpHeaders.put(headerName, Arrays.asList(headerValues))); + } /** * Manipulate request headers. The provided {@code HttpHeaders} contains @@ -147,6 +169,7 @@ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage * {@linkplain HttpHeaders#set(String, String) overwrite} or * {@linkplain HttpHeaders#remove(Object) remove} existing values, or * use any other {@link HttpHeaders} methods. + * @see #header(String, String...) */ Builder headers(Consumer headersConsumer); diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java index 611b8b8d41..d1ca3b134d 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java @@ -41,6 +41,7 @@ import static org.mockito.Mockito.*; * Unit tests for {@link AbstractServerHttpRequest}. * * @author Rossen Stoyanchev + * @author Sam Brannen */ public class ServerHttpRequestTests { @@ -88,7 +89,6 @@ public class ServerHttpRequestTests { @Test public void mutateRequest() throws Exception { - SslInfo sslInfo = mock(SslInfo.class); ServerHttpRequest request = createHttpRequest("/").mutate().sslInfo(sslInfo).build(); assertSame(sslInfo, request.getSslInfo()); @@ -123,6 +123,54 @@ public class ServerHttpRequestTests { assertEquals("name=%E6%89%8E%E6%A0%B9", request.getURI().getRawQuery()); } + @Test + @SuppressWarnings("deprecation") + public void mutateHeaderByAddingHeaderValues() throws Exception { + String headerName = "key"; + String headerValue1 = "value1"; + String headerValue2 = "value2"; + + ServerHttpRequest request = createHttpRequest("/path"); + assertNull(request.getHeaders().get(headerName)); + + request = request.mutate().header(headerName, headerValue1).build(); + + assertNotNull(request.getHeaders().get(headerName)); + assertEquals(1, request.getHeaders().get(headerName).size()); + assertEquals(headerValue1, request.getHeaders().get(headerName).get(0)); + + request = request.mutate().header(headerName, headerValue2).build(); + + assertNotNull(request.getHeaders().get(headerName)); + assertEquals(2, request.getHeaders().get(headerName).size()); + assertEquals(headerValue1, request.getHeaders().get(headerName).get(0)); + assertEquals(headerValue2, request.getHeaders().get(headerName).get(1)); + } + + @Test + public void mutateHeaderBySettingHeaderValues() throws Exception { + String headerName = "key"; + String headerValue1 = "value1"; + String headerValue2 = "value2"; + String headerValue3 = "value3"; + + ServerHttpRequest request = createHttpRequest("/path"); + assertNull(request.getHeaders().get(headerName)); + + request = request.mutate().header(headerName, headerValue1, headerValue2).build(); + + assertNotNull(request.getHeaders().get(headerName)); + assertEquals(2, request.getHeaders().get(headerName).size()); + assertEquals(headerValue1, request.getHeaders().get(headerName).get(0)); + assertEquals(headerValue2, request.getHeaders().get(headerName).get(1)); + + request = request.mutate().header(headerName, new String[] { headerValue3 }).build(); + + assertNotNull(request.getHeaders().get(headerName)); + assertEquals(1, request.getHeaders().get(headerName).size()); + assertEquals(headerValue3, request.getHeaders().get(headerName).get(0)); + } + private ServerHttpRequest createHttpRequest(String uriString) throws Exception { URI uri = URI.create(uriString); MockHttpServletRequest request = new TestHttpServletRequest(uri);