Fixes #4808 - Review HttpClient Request header APIs.

Introduced:
* Request Request.headers(Consumer<HttpFields.Mutable>).
This allows applications to modify the headers, and chain calls.
It also delegates the precise semantic of put/add/remove/clear to HttpFields, so there is no API duplication.
* HttpRequest.header(HttpField) to efficiently add fields while normalizing the request (only used in implementation).

* HttpResponse.header(HttpField) to efficiently add fields while parsing the response (only used in implementation).
This pairs with HttpResponse.trailer(HttpField).
* HttpResponse.headers(Consumer<HttpFields.Mutable>) to modify the fields after they have been populated (only used in tests).

Removed:
* Request.[set,add,put,remove], replaced by headers(Consumer<HttpFields.Mutable>).

Deprecated:
* Request.header(String, String)
* Request.header(HttpHeader, String)
Both replaced by headers(Consumer<HttpFields.Mutable>) with clearer semantic for add/put/remove.

All the rest is code cleanup to remove the usage of the deprecated header() methods.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

examples/embedded/src/test/java/org/eclipse/jetty/embedded/ManyHandlersTest.java

@@ -23,6 +23,7 @@
 
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.server.Server;
@@ -59,7 +60,7 @@ public void testGetParams() throws Exception
 
         ContentResponse response = client.newRequest(uri)
             .method(HttpMethod.GET)
-            .header(HttpHeader.ACCEPT_ENCODING, "gzip")
+            .headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP))
             .send();
         assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200));
 
@@ -84,7 +85,7 @@ public void testGetHello() throws Exception
         URI uri = server.getURI().resolve("/hello");
         ContentResponse response = client.newRequest(uri)
             .method(HttpMethod.GET)
-            .header(HttpHeader.ACCEPT_ENCODING, "gzip")
+            .headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP))
             .send();
         assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200));
 

examples/embedded/src/test/java/org/eclipse/jetty/embedded/SecuredHelloHandlerTest.java

@@ -72,7 +72,7 @@ public void testGetWithAuth() throws Exception
         String authEncoded = Base64.getEncoder().encodeToString("user:password".getBytes(UTF_8));
         ContentResponse response = client.newRequest(uri)
             .method(HttpMethod.GET)
-            .header(HttpHeader.AUTHORIZATION, "Basic " + authEncoded)
+            .headers(headers -> headers.put(HttpHeader.AUTHORIZATION, "Basic " + authEncoded))
             .send();
         assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200));
 

jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java

@@ -256,7 +256,7 @@ private void copyIfAbsent(HttpRequest oldRequest, Request newRequest, HttpHeader
         {
             HttpField field = oldRequest.getHeaders().getField(header);
             if (field != null && !newRequest.getHeaders().contains(header))
-                newRequest.put(field);
+                newRequest.headers(headers -> headers.put(field));
         }
 
         private void forwardSuccessComplete(HttpRequest request, Response response)

jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java

@@ -444,7 +444,7 @@ public Request newRequest(URI uri)
 
     protected Request copyRequest(HttpRequest oldRequest, URI newURI)
     {
-        Request newRequest = newHttpRequest(oldRequest.getConversation(), newURI);
+        HttpRequest newRequest = newHttpRequest(oldRequest.getConversation(), newURI);
         newRequest.method(oldRequest.getMethod())
             .version(oldRequest.getVersion())
             .body(oldRequest.getBody())
@@ -471,10 +471,8 @@ protected Request copyRequest(HttpRequest oldRequest, URI newURI)
                 HttpHeader.PROXY_AUTHORIZATION == header)
                 continue;
 
-            String name = field.getName();
-            String value = field.getValue();
-            if (!newRequest.getHeaders().contains(name, value))
-                newRequest.header(name, value);
+            if (!newRequest.getHeaders().contains(field))
+                newRequest.header(field);
         }
         return newRequest;
     }

jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java

@@ -122,9 +122,9 @@ protected SendFailure send(HttpChannel channel, HttpExchange exchange)
         }
     }
 
-    protected void normalizeRequest(Request request)
+    protected void normalizeRequest(HttpRequest request)
     {
-        boolean normalized = ((HttpRequest)request).normalized();
+        boolean normalized = request.normalized();
         if (LOG.isDebugEnabled())
             LOG.debug("Normalizing {} {}", !normalized, request);
         if (normalized)
@@ -153,7 +153,7 @@ protected void normalizeRequest(Request request)
         if (version.getVersion() <= 11)
         {
             if (!headers.contains(HttpHeader.HOST))
-                request.put(getHttpDestination().getHostField());
+            request.header(getHttpDestination().getHostField());
         }
 
         // Add content headers
@@ -167,22 +167,19 @@ protected void normalizeRequest(Request request)
             if (!headers.contains(HttpHeader.CONTENT_TYPE))
             {
                 String contentType = content.getContentType();
+                if (contentType == null)
+                    contentType = getHttpClient().getDefaultRequestContentType();
                 if (contentType != null)
                 {
-                    request.put(new HttpField(HttpHeader.CONTENT_TYPE, contentType));
-                }
-                else
-                {
-                    contentType = getHttpClient().getDefaultRequestContentType();
-                    if (contentType != null)
-                        request.put(new HttpField(HttpHeader.CONTENT_TYPE, contentType));
+                    HttpField field = new HttpField(HttpHeader.CONTENT_TYPE, contentType);
+                    request.header(field);
                 }
             }
             long contentLength = content.getLength();
             if (contentLength >= 0)
             {
                 if (!headers.contains(HttpHeader.CONTENT_LENGTH))
-                    request.put(new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, contentLength));
+                    request.header(new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, contentLength));
             }
         }
 
@@ -195,7 +192,10 @@ protected void normalizeRequest(Request request)
                 cookies = convertCookies(HttpCookieStore.matchPath(uri, cookieStore.get(uri)), null);
             cookies = convertCookies(request.getCookies(), cookies);
             if (cookies != null)
-                request.header(HttpHeader.COOKIE, cookies.toString());
+            {
+                HttpField cookieField = new HttpField(HttpHeader.COOKIE, cookies.toString());
+                request.header(cookieField);
+            }
         }
 
         // Authentication

jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java

@@ -200,9 +200,9 @@ private void tunnel(HttpDestination destination, Connection connection)
             Origin.Address proxyAddress = destination.getConnectAddress();
             HttpClient httpClient = destination.getHttpClient();
             Request connect = new TunnelRequest(httpClient, proxyAddress)
-                    .method(HttpMethod.CONNECT)
-                    .path(target)
-                    .header(HttpHeader.HOST, target);
+                .method(HttpMethod.CONNECT)
+                .path(target)
+                .headers(headers -> headers.put(HttpHeader.HOST, target));
             ProxyConfiguration.Proxy proxy = destination.getProxy();
             if (proxy.isSecure())
                 connect.scheme(HttpScheme.HTTPS.asString());
@@ -262,8 +262,7 @@ public void onHeaders(Response response)
                 }
                 else
                 {
-                    HttpResponseException failure = new HttpResponseException("Unexpected " + response +
-                            " for " + response.getRequest(), response);
+                    HttpResponseException failure = new HttpResponseException("Unexpected " + response + " for " + response.getRequest(), response);
                     tunnelFailed(endPoint, failure);
                 }
             }

jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java

@@ -72,7 +72,7 @@
  */
 public abstract class HttpReceiver
 {
-    protected static final Logger LOG = LoggerFactory.getLogger(HttpReceiver.class);
+    private static final Logger LOG = LoggerFactory.getLogger(HttpReceiver.class);
 
     private final AtomicReference<ResponseState> responseState = new AtomicReference<>(ResponseState.IDLE);
     private final HttpChannel channel;
@@ -242,7 +242,7 @@ protected boolean responseHeader(HttpExchange exchange, HttpField field)
         boolean process = notifier.notifyHeader(exchange.getConversation().getResponseListeners(), response, field);
         if (process)
         {
-            response.getHeaderFieldsMutable().add(field);
+            response.header(field);
             HttpHeader fieldHeader = field.getHeader();
             if (fieldHeader != null)
             {

jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java

@@ -41,6 +41,7 @@
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
+import java.util.function.Consumer;
 import java.util.function.LongConsumer;
 import java.util.function.Supplier;
 
@@ -305,34 +306,6 @@ public Request accept(String... accepts)
         return this;
     }
 
-    @Override
-    public Request set(HttpFields fields)
-    {
-        headers.clear().add(fields);
-        return this;
-    }
-
-    @Override
-    public Request remove(HttpHeader header)
-    {
-        headers.remove(header);
-        return this;
-    }
-
-    @Override
-    public Request put(HttpField field)
-    {
-        headers.put(field);
-        return this;
-    }
-
-    @Override
-    public Request add(HttpField field)
-    {
-        headers.add(field);
-        return this;
-    }
-
     @Override
     public Request header(String name, String value)
     {
@@ -402,6 +375,19 @@ public HttpFields.Mutable getHeaders()
         return headers;
     }
 
+    @Override
+    public Request headers(Consumer<HttpFields.Mutable> consumer)
+    {
+        consumer.accept(headers);
+        return this;
+    }
+
+    public HttpRequest header(HttpField header)
+    {
+        headers.add(header);
+        return this;
+    }
+
     @Override
     @SuppressWarnings("unchecked")
     public <T extends RequestListener> List<T> getRequestListeners(Class<T> type)
@@ -707,7 +693,7 @@ public Request content(ContentProvider content)
     public Request content(ContentProvider content, String contentType)
     {
         if (contentType != null)
-            header(HttpHeader.CONTENT_TYPE, contentType);
+            headers.put(HttpHeader.CONTENT_TYPE, contentType);
         return body(ContentProvider.toRequestContent(content));
     }
 
@@ -886,7 +872,7 @@ public Throwable getAbortCause()
      * headers, etc.</p>
      *
      * @return whether this request was already normalized
-     * @see HttpConnection#normalizeRequest(Request)
+     * @see HttpConnection#normalizeRequest(HttpRequest)
      */
     boolean normalized()
     {

jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java

@@ -20,6 +20,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Consumer;
 
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
@@ -91,9 +92,16 @@ public HttpFields getHeaders()
         return headers.asImmutable();
     }
 
-    public HttpFields.Mutable getHeaderFieldsMutable()
+    public HttpResponse header(HttpField header)
     {
-        return headers;
+        headers.add(header);
+        return this;
+    }
+
+    public HttpResponse headers(Consumer<HttpFields.Mutable> consumer)
+    {
+        consumer.accept(headers);
+        return this;
     }
 
     @Override
@@ -110,7 +118,7 @@ public <T extends ResponseListener> List<T> getListeners(Class<T> type)
 
     public HttpFields getTrailers()
     {
-        return trailers;
+        return trailers.asImmutable();
     }
 
     public HttpResponse trailer(HttpField trailer)

jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java

@@ -31,10 +31,10 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.function.Consumer;
 
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.util.InputStreamResponseListener;
-import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
@@ -166,38 +166,22 @@ public interface Request
      */
     HttpFields getHeaders();
 
-    /** Set the headers, clearing any existing headers
-     * @param fields The fields to set
-     * @return this request object
-     */
-    Request set(HttpFields fields);
-
-    /**
-     * @param header the header to remove
-     * @return this request object
-     */
-    Request remove(HttpHeader header);
-
-    /**
-     * @param field the field to add
-     * @return this request object
-     * @see #header(HttpHeader, String)
-     */
-    Request add(HttpField field);
-
     /**
-     * @param field the field to put
+     * Modifies the headers of this request.
+     *
+     * @param consumer the code that modifies the headers of this request
      * @return this request object
-     * @see #header(HttpHeader, String)
      */
-    Request put(HttpField field);
+    Request headers(Consumer<HttpFields.Mutable> consumer);
 
     /**
      * @param name the name of the header
      * @param value the value of the header
      * @return this request object
      * @see #header(HttpHeader, String)
+     * @deprecated use {@link #headers(Consumer)} instead
      */
+    @Deprecated
     Request header(String name, String value);
 
     /**
@@ -208,7 +192,9 @@ public interface Request
      * @param header the header name
      * @param value the value of the header
      * @return this request object
+     * @deprecated use {@link #headers(Consumer)} instead
      */
+    @Deprecated
     Request header(HttpHeader header, String value);
 
     /**

jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java

@@ -261,7 +261,7 @@ private Delegate(HttpDestination destination)
         @Override
         public SendFailure send(HttpExchange exchange)
         {
-            Request request = exchange.getRequest();
+            HttpRequest request = exchange.getRequest();
             normalizeRequest(request);
 
             // Save the old idle timeout to restore it.
@@ -276,7 +276,7 @@ public SendFailure send(HttpExchange exchange)
         }
 
         @Override
-        protected void normalizeRequest(Request request)
+        protected void normalizeRequest(HttpRequest request)
         {
             super.normalizeRequest(request);
 
@@ -287,16 +287,15 @@ protected void normalizeRequest(Request request)
                         .idleTimeout(2 * connectTimeout, TimeUnit.MILLISECONDS);
             }
 
-            HttpRequest httpRequest = (HttpRequest)request;
-            HttpConversation conversation = httpRequest.getConversation();
+            HttpConversation conversation = request.getConversation();
             HttpUpgrader upgrader = (HttpUpgrader)conversation.getAttribute(HttpUpgrader.class.getName());
             if (upgrader == null)
             {
                 if (request instanceof HttpUpgrader.Factory)
                 {
                     upgrader = ((HttpUpgrader.Factory)request).newHttpUpgrader(HttpVersion.HTTP_1_1);
                     conversation.setAttribute(HttpUpgrader.class.getName(), upgrader);
-                    upgrader.prepare(httpRequest);
+                    upgrader.prepare(request);
                 }
                 else
                 {
@@ -305,7 +304,7 @@ protected void normalizeRequest(Request request)
                     {
                         upgrader = new ProtocolHttpUpgrader(getHttpDestination(), protocol);
                         conversation.setAttribute(HttpUpgrader.class.getName(), upgrader);
-                        upgrader.prepare(httpRequest);
+                        upgrader.prepare(request);
                     }
                 }
             }