Revert "HTTPCLIENT-1482: added ExpectContinueTrigger, extended RequestConfig, and modified RequestExpectContinue so that Expect: 100-continue is emitted only when the underlying connection has already processed at least one request; default behaviour (ALWAYS) preserved, API remains binary-compatible. (#683)"

This reverts commit 97fd931

Author: ok2c

httpclient5/src/main/java/org/apache/hc/client5/http/config/ExpectContinueTrigger.java

@@ -36,7 +36,6 @@
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http2.priority.PriorityValue;
-import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
 
@@ -68,9 +67,6 @@ public class RequestConfig implements Cloneable {
     private final boolean hardCancellationEnabled;
     private final boolean protocolUpgradeEnabled;
     private final Path unixDomainSocket;
-
-    private final ExpectContinueTrigger expectContinueTrigger;
-
     /**
      * HTTP/2 Priority header value to emit when using H2+. Null means “don’t emit”.
      */
@@ -82,7 +78,7 @@ public class RequestConfig implements Cloneable {
     protected RequestConfig() {
         this(false, null, null, false, false, 0, false, null, null,
                 DEFAULT_CONNECTION_REQUEST_TIMEOUT, null, null, DEFAULT_CONN_KEEP_ALIVE, false, false, false, null,
-                ExpectContinueTrigger.ALWAYS, null);
+                null);
     }
 
     RequestConfig(
@@ -103,7 +99,6 @@ protected RequestConfig() {
             final boolean hardCancellationEnabled,
             final boolean protocolUpgradeEnabled,
             final Path unixDomainSocket,
-            final ExpectContinueTrigger expectContinueTrigger,
             final PriorityValue h2Priority) {
         super();
         this.expectContinueEnabled = expectContinueEnabled;
@@ -123,7 +118,6 @@ protected RequestConfig() {
         this.hardCancellationEnabled = hardCancellationEnabled;
         this.protocolUpgradeEnabled = protocolUpgradeEnabled;
         this.unixDomainSocket = unixDomainSocket;
-        this.expectContinueTrigger = expectContinueTrigger;
         this.h2Priority = h2Priority;
     }
 
@@ -249,10 +243,6 @@ public Path getUnixDomainSocket() {
         return unixDomainSocket;
     }
 
-    public ExpectContinueTrigger getExpectContinueTrigger() {
-        return expectContinueTrigger;
-    }
-
     /**
      * Returns the HTTP/2+ priority preference for this request or {@code null} if unset.
      * @since 5.6
@@ -288,6 +278,7 @@ public String toString() {
         builder.append(", hardCancellationEnabled=").append(hardCancellationEnabled);
         builder.append(", protocolUpgradeEnabled=").append(protocolUpgradeEnabled);
         builder.append(", unixDomainSocket=").append(unixDomainSocket);
+        builder.append(", h2Priority=").append(h2Priority);
         builder.append("]");
         return builder.toString();
     }
@@ -337,7 +328,6 @@ public static class Builder {
         private boolean hardCancellationEnabled;
         private boolean protocolUpgradeEnabled;
         private Path unixDomainSocket;
-        private ExpectContinueTrigger expectContinueTrigger;
         private PriorityValue h2Priority;
 
         Builder() {
@@ -349,7 +339,6 @@ public static class Builder {
             this.contentCompressionEnabled = true;
             this.hardCancellationEnabled = true;
             this.protocolUpgradeEnabled = true;
-            this.expectContinueTrigger = ExpectContinueTrigger.ALWAYS;
         }
 
         /**
@@ -697,20 +686,6 @@ public Builder setUnixDomainSocket(final Path unixDomainSocket) {
             return this;
         }
 
-        /**
-         * Defines under which circumstances the client should add the
-         * {@code Expect: 100-continue} header to entity-enclosing requests.
-         *
-         * @param trigger expectation-continue trigger strategy
-         * @return this builder
-         * @see ExpectContinueTrigger
-         * @since 5.6
-         */
-        public Builder setExpectContinueTrigger(final ExpectContinueTrigger trigger) {
-            this.expectContinueTrigger = Args.notNull(trigger, "ExpectContinueTrigger");
-            return this;
-        }
-
         /**
          * Sets HTTP/2+ request priority. If {@code null}, the header is not emitted.
          * @since 5.6
@@ -740,7 +715,6 @@ public RequestConfig build() {
                     hardCancellationEnabled,
                     protocolUpgradeEnabled,
                     unixDomainSocket,
-                    expectContinueTrigger,
                     h2Priority);
         }
 

httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java

@@ -29,11 +29,9 @@
 
 import java.io.IOException;
 
-import org.apache.hc.client5.http.config.ExpectContinueTrigger;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
-import org.apache.hc.core5.http.EndpointDetails;
 import org.apache.hc.core5.http.EntityDetails;
 import org.apache.hc.core5.http.HeaderElements;
 import org.apache.hc.core5.http.HttpException;
@@ -43,7 +41,6 @@
 import org.apache.hc.core5.http.HttpVersion;
 import org.apache.hc.core5.http.ProtocolVersion;
 import org.apache.hc.core5.http.protocol.HttpContext;
-import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.util.Args;
 
 /**
@@ -71,20 +68,13 @@ public void process(final HttpRequest request, final EntityDetails entity, final
         if (!request.containsHeader(HttpHeaders.EXPECT)) {
             final HttpClientContext clientContext = HttpClientContext.cast(context);
             final ProtocolVersion version = request.getVersion() != null ? request.getVersion() : clientContext.getProtocolVersion();
-            final RequestConfig config = clientContext.getRequestConfigOrDefault();
-            if (!config.isExpectContinueEnabled()) {
-                return;
-            }
-            if (config.getExpectContinueTrigger() == ExpectContinueTrigger.IF_REUSED) {
-                final EndpointDetails details = HttpCoreContext.cast(context).getEndpointDetails();
-                if (details != null && details.getRequestCount() == 0) {
-                    return;
-                }
-            }
             // Do not send the expect header if request body is known to be empty
             if (entity != null
                     && entity.getContentLength() != 0 && !version.lessEquals(HttpVersion.HTTP_1_0)) {
-                request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE);
+                final RequestConfig config = clientContext.getRequestConfigOrDefault();
+                if (config.isExpectContinueEnabled()) {
+                    request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE);
+                }
             }
         }
     }

httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestExpectContinue.java

@@ -27,21 +27,16 @@
 
 package org.apache.hc.client5.http.protocol;
 
-import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 
-import org.apache.hc.client5.http.config.ExpectContinueTrigger;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HeaderElements;
-import org.apache.hc.core5.http.HttpConnectionMetrics;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpVersion;
-import org.apache.hc.core5.http.impl.BasicEndpointDetails;
 import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
-import org.apache.hc.core5.util.Timeout;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -124,94 +119,4 @@ void testRequestExpectContinueIgnoreNonenclosingRequests() throws Exception {
         Assertions.assertEquals(0, request.getHeaders().length);
     }
 
-
-    @Test
-    void testRequestExpectContinueIfReused() throws Exception {
-        final HttpClientContext context = HttpClientContext.create();
-        final RequestConfig config = RequestConfig.custom()
-                .setExpectContinueEnabled(true)
-                .setExpectContinueTrigger(ExpectContinueTrigger.IF_REUSED)
-                .build();
-        context.setRequestConfig(config);
-
-        final HttpConnectionMetrics metrics = new HttpConnectionMetrics() {
-            @Override
-            public long getRequestCount() {
-                return 1;
-            }
-
-            @Override
-            public long getResponseCount() {
-                return 0;
-            }
-
-            @Override
-            public long getSentBytesCount() {
-                return 0;
-            }
-
-            @Override
-            public long getReceivedBytesCount() {
-                return 0;
-            }
-        };
-
-        final BasicEndpointDetails reused = new BasicEndpointDetails(
-                new InetSocketAddress("localhost", 0),
-                new InetSocketAddress("localhost", 80),
-                metrics,
-                Timeout.ofSeconds(30));
-        context.setEndpointDetails(reused);
-
-        final ClassicHttpRequest request = new BasicClassicHttpRequest("POST", "/");
-        request.setEntity(new StringEntity("data", StandardCharsets.US_ASCII));
-
-        new RequestExpectContinue().process(request, request.getEntity(), context);
-
-        final Header header = request.getFirstHeader(HttpHeaders.EXPECT);
-        Assertions.assertNotNull(header);
-        Assertions.assertEquals(HeaderElements.CONTINUE, header.getValue());
-    }
-
-    @Test
-    void testNoExpectContinueFreshConnectionWithIfReused() throws Exception {
-        final HttpClientContext context = HttpClientContext.create();
-        final RequestConfig cfg = RequestConfig.custom()
-                .setExpectContinueEnabled(true)
-                .setExpectContinueTrigger(ExpectContinueTrigger.IF_REUSED)
-                .build();
-        context.setRequestConfig(cfg);
-
-        // fresh endpoint: requestCount == 0
-        context.setEndpointDetails(new BasicEndpointDetails(
-                new InetSocketAddress("localhost", 0),
-                new InetSocketAddress("localhost", 80),
-                null,
-                Timeout.ofSeconds(30)));
-
-        final ClassicHttpRequest req = new BasicClassicHttpRequest("POST", "/");
-        req.setEntity(new StringEntity("data", StandardCharsets.US_ASCII));
-
-        new RequestExpectContinue().process(req, req.getEntity(), context);
-
-        Assertions.assertNull(req.getFirstHeader(HttpHeaders.EXPECT));
-    }
-
-    @Test
-    void testHeaderAlreadyPresentIsNotDuplicated() throws Exception {
-        final HttpClientContext context = HttpClientContext.create();
-        final RequestConfig cfg = RequestConfig.custom()
-                .setExpectContinueEnabled(true)
-                .build();                      // default trigger = ALWAYS
-        context.setRequestConfig(cfg);
-
-        final ClassicHttpRequest req = new BasicClassicHttpRequest("POST", "/");
-        req.setEntity(new StringEntity("data", StandardCharsets.US_ASCII));
-        req.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE); // pre-existing
-
-        new RequestExpectContinue().process(req, req.getEntity(), context);
-
-        final Header[] headers = req.getHeaders(HttpHeaders.EXPECT);
-        Assertions.assertEquals(1, headers.length);                 // no duplicates
-    }
 }