Merge pull request #4845 from eclipse/jetty-10.0.x-4808-review_httpcl…

…ient_header_api Fixes #4808 - Review HttpClient Request header APIs.
jetty · May 8, 2020 · 0018c29 · 0018c29
2 parents 63bfb43 + b6c6684
commit 0018c29
Show file tree

Hide file tree

Showing 61 changed files with 344 additions and 383 deletions.
diff --git a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/ManyHandlersTest.java b/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));
 

diff --git a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/SecuredHelloHandlerTest.java b/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));
 

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java b/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)

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/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.addHeader(field);
         }
         return newRequest;
     }

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/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.addHeader(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.addHeader(field);
                 }
             }
             long contentLength = content.getLength();
             if (contentLength >= 0)
             {
                 if (!headers.contains(HttpHeader.CONTENT_LENGTH))
-                    request.put(new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, contentLength));
+                    request.addHeader(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.addHeader(cookieField);
+            }
         }
 
         // Authentication

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java b/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);
                 }
             }

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/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.addHeader(field);
             HttpHeader fieldHeader = field.getHeader();
             if (fieldHeader != null)
             {

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/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;
 
@@ -306,34 +307,7 @@ public Request accept(String... accepts)
     }
 
     @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
+    @Deprecated
     public Request header(String name, String value)
     {
         if (value == null)
@@ -344,6 +318,7 @@ public Request header(String name, String value)
     }
 
     @Override
+    @Deprecated
     public Request header(HttpHeader header, String value)
     {
         if (value == null)
@@ -402,6 +377,19 @@ public HttpFields.Mutable getHeaders()
         return headers;
     }
 
+    @Override
+    public Request headers(Consumer<HttpFields.Mutable> consumer)
+    {
+        consumer.accept(headers);
+        return this;
+    }
+
+    public HttpRequest addHeader(HttpField header)
+    {
+        headers.add(header);
+        return this;
+    }
+
     @Override
     @SuppressWarnings("unchecked")
     public <T extends RequestListener> List<T> getRequestListeners(Class<T> type)
@@ -707,7 +695,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 +874,7 @@ public Throwable getAbortCause()
      * headers, etc.</p>
      *
      * @return whether this request was already normalized
-     * @see HttpConnection#normalizeRequest(Request)
+     * @see HttpConnection#normalizeRequest(HttpRequest)
      */
     boolean normalized()
     {

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java b/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 addHeader(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 == null ? null : trailers.asImmutable();
     }
 
     public HttpResponse trailer(HttpField trailer)

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java b/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);
 
     /**