HTTP client transport simplification

Motivation:

The Transport inner class of HttpClient implementation grouping per protocol objects is not longer necessary since those objects are identical regardless of the protocol.

Author: vietj

vertx-core/src/main/java/io/vertx/core/http/impl/CleanableHttpClient.java

@@ -15,7 +15,7 @@
 import io.vertx.core.http.*;
 import io.vertx.core.http.HttpClientConnection;
 import io.vertx.core.internal.VertxInternal;
-import io.vertx.core.internal.http.HttpChannelConnector;
+import io.vertx.core.internal.http.HttpClientTransport;
 import io.vertx.core.internal.http.HttpClientInternal;
 import io.vertx.core.internal.net.endpoint.EndpointResolverInternal;
 import io.vertx.core.net.ClientSSLOptions;
@@ -103,7 +103,7 @@ public Function<HttpClientResponse, Future<RequestOptions>> redirectHandler() {
   }
 
   @Override
-  public HttpChannelConnector channelConnector() {
+  public HttpClientTransport channelConnector() {
     return delegate.channelConnector();
   }
 

…ttp/impl/Http1xOrH2ChannelConnector.java …http/impl/Http1xOrH2ClientTransport.javavertx-core/src/main/java/io/vertx/core/http/impl/Http1xOrH2ChannelConnector.java renamed to vertx-core/src/main/java/io/vertx/core/http/impl/Http1xOrH2ClientTransport.java vertx-core/src/main/java/io/vertx/core/http/impl/Http1xOrH2ChannelConnector.java renamed to vertx-core/src/main/java/io/vertx/core/http/impl/Http1xOrH2ClientTransport.java

@@ -34,7 +34,7 @@
 import io.vertx.core.http.impl.http2.multiplex.Http2MultiplexClientChannelInitializer;
 import io.vertx.core.internal.ContextInternal;
 import io.vertx.core.internal.PromiseInternal;
-import io.vertx.core.internal.http.HttpChannelConnector;
+import io.vertx.core.internal.http.HttpClientTransport;
 import io.vertx.core.internal.http.HttpHeadersInternal;
 import io.vertx.core.internal.net.NetClientInternal;
 import io.vertx.core.net.*;
@@ -57,12 +57,12 @@
  *
  * @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
  */
-public class Http1xOrH2ChannelConnector implements HttpChannelConnector {
+public class Http1xOrH2ClientTransport implements HttpClientTransport {
 
-  public static Http1xOrH2ChannelConnector create(NetClientInternal netClient,
-                                    HttpClientConfig config,
-                                    HttpClientMetrics clientMetrics) {
-    return new Http1xOrH2ChannelConnector(netClient,
+  public static Http1xOrH2ClientTransport create(NetClientInternal netClient,
+                                                 HttpClientConfig config,
+                                                 HttpClientMetrics clientMetrics) {
+    return new Http1xOrH2ClientTransport(netClient,
       config.getTracingPolicy(),
       config.isDecompressionSupported(),
       config.getLogActivity(),
@@ -89,18 +89,18 @@ public static Http1xOrH2ChannelConnector create(NetClientInternal netClient,
   private final HttpClientMetrics clientMetrics;
   private final NetClientInternal netClient;
 
-  public Http1xOrH2ChannelConnector(NetClientInternal netClient,
-                                    TracingPolicy tracingPolicy,
-                                    boolean useDecompression,
-                                    boolean logActivity,
-                                    ByteBufFormat logFormat,
-                                    boolean forceSni,
-                                    Http1ClientConfig http1Config,
-                                    Http2ClientConfig http2Config,
-                                    Duration idleTimeout,
-                                    Duration readIdleTimeout,
-                                    Duration writeIdleTimeout,
-                                    HttpClientMetrics clientMetrics) {
+  public Http1xOrH2ClientTransport(NetClientInternal netClient,
+                                   TracingPolicy tracingPolicy,
+                                   boolean useDecompression,
+                                   boolean logActivity,
+                                   ByteBufFormat logFormat,
+                                   boolean forceSni,
+                                   Http1ClientConfig http1Config,
+                                   Http2ClientConfig http2Config,
+                                   Duration idleTimeout,
+                                   Duration readIdleTimeout,
+                                   Duration writeIdleTimeout,
+                                   HttpClientMetrics clientMetrics) {
 
     if (!http1Config.isKeepAlive() && http1Config.isPipelining()) {
       throw new IllegalStateException("Cannot have pipelining with no keep alive");
@@ -221,7 +221,7 @@ public Future<HttpClientConnection> wrap(ContextInternal context, HttpConnectPar
     return promise.future();
   }
 
-  public Future<HttpClientConnection> httpConnect(ContextInternal context, SocketAddress server, HostAndPort authority, HttpConnectParams params, ClientMetrics<?, ?, ?> metrics) {
+  public Future<HttpClientConnection> connect(ContextInternal context, SocketAddress server, HostAndPort authority, HttpConnectParams params, ClientMetrics<?, ?, ?> metrics) {
 
     if (params.sslOptions != null && !params.sslOptions.isUseAlpn() && params.ssl && params.protocol == HttpVersion.HTTP_2) {
       return context.failedFuture("Must enable ALPN when using H2");

…ore/http/impl/Http3ChannelConnector.java …core/http/impl/Http3ClientTransport.javavertx-core/src/main/java/io/vertx/core/http/impl/Http3ChannelConnector.java renamed to vertx-core/src/main/java/io/vertx/core/http/impl/Http3ClientTransport.java vertx-core/src/main/java/io/vertx/core/http/impl/Http3ChannelConnector.java renamed to vertx-core/src/main/java/io/vertx/core/http/impl/Http3ClientTransport.java

@@ -13,20 +13,18 @@
 import io.netty.handler.codec.http3.Http3;
 import io.vertx.core.Future;
 import io.vertx.core.Promise;
-import io.vertx.core.http.Http3ClientOptions;
 import io.vertx.core.http.Http3Settings;
 import io.vertx.core.http.impl.config.HttpClientConfig;
 import io.vertx.core.http.impl.http3.Http3ClientConnection;
 import io.vertx.core.internal.ContextInternal;
 import io.vertx.core.internal.VertxInternal;
-import io.vertx.core.internal.http.HttpChannelConnector;
+import io.vertx.core.internal.http.HttpClientTransport;
 import io.vertx.core.internal.quic.QuicConnectionInternal;
 import io.vertx.core.net.*;
 import io.vertx.core.net.impl.quic.QuicClientImpl;
 import io.vertx.core.spi.metrics.ClientMetrics;
 import io.vertx.core.spi.metrics.HttpClientMetrics;
 import io.vertx.core.spi.metrics.TransportMetrics;
-import io.vertx.core.spi.metrics.VertxMetrics;
 import io.vertx.core.spi.observability.HttpRequest;
 import io.vertx.core.spi.observability.HttpResponse;
 
@@ -36,7 +34,7 @@
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.BiFunction;
 
-public class Http3ChannelConnector implements HttpChannelConnector {
+public class Http3ClientTransport implements HttpClientTransport {
 
   private final VertxInternal vertx;
   private final HttpClientMetrics<?, ?, ?> clientMetrics;
@@ -46,7 +44,7 @@ public class Http3ChannelConnector implements HttpChannelConnector {
   private final long keepAliveTimeoutMillis;
   private final Http3Settings localSettings;
 
-  public Http3ChannelConnector(VertxInternal vertxInternal, HttpClientMetrics<?, ?, ?> clientMetrics,  HttpClientConfig options) {
+  public Http3ClientTransport(VertxInternal vertxInternal, HttpClientMetrics<?, ?, ?> clientMetrics, HttpClientConfig options) {
 
     ClientSSLOptions sslOptions = options.getSslOptions()
       .copy()
@@ -83,7 +81,7 @@ public Http3ChannelConnector(VertxInternal vertxInternal, HttpClientMetrics<?, ?
   }
 
   @Override
-  public Future<HttpClientConnection> httpConnect(ContextInternal context, SocketAddress server, HostAndPort authority, HttpConnectParams params, ClientMetrics<?, ?, ?> metrics) {
+  public Future<HttpClientConnection> connect(ContextInternal context, SocketAddress server, HostAndPort authority, HttpConnectParams params, ClientMetrics<?, ?, ?> metrics) {
 
     lock.lock();
     Future<QuicClient> fut = clientFuture;

vertx-core/src/main/java/io/vertx/core/http/impl/HttpClientBuilderInternal.java

@@ -9,6 +9,7 @@
 import io.vertx.core.internal.CloseFuture;
 import io.vertx.core.internal.ContextInternal;
 import io.vertx.core.internal.VertxInternal;
+import io.vertx.core.internal.http.HttpClientTransport;
 import io.vertx.core.internal.http.HttpClientInternal;
 import io.vertx.core.internal.net.NetClientInternal;
 import io.vertx.core.net.ProxyOptions;
@@ -116,11 +117,12 @@ private EndpointResolver endpointResolver(HttpClientConfig co) {
     return null;
   }
 
-  private HttpClientImpl createHttpClientImpl(HttpClientMetrics<?, ?, ?> metrics,
+  private HttpClientImpl createHttpClientImpl(HttpClientConfig clientConfig,
+                                              HttpClientMetrics<?, ?, ?> metrics,
                                               EndpointResolver resolver,
                                               Function<HttpClientResponse, Future<RequestOptions>> redirectHandler,
-                                              HttpClientImpl.Transport config,
-                                              HttpClientImpl.Transport quicTransport) {
+                                              HttpClientTransport tcpTransport,
+                                              HttpClientTransport quicTransport) {
     boolean followAlternativeServices;
     ProxyOptions proxyOptions;
     List<String> nonProxyHosts;
@@ -136,8 +138,28 @@ private HttpClientImpl createHttpClientImpl(HttpClientMetrics<?, ?, ?> metrics,
     PoolOptions po;
     po = poolOptions != null ? poolOptions : new PoolOptions();
     HttpClientOptions options = HttpClientBuilderInternal.this.clientOptions;
-    return new HttpClientImpl(vertx, resolver, redirectHandler, metrics, po,
-      proxyOptions, nonProxyHosts, loadBalancer, followAlternativeServices, resolverKeepAlive, config, quicTransport) {
+    Handler<HttpConnection> connectHandler = connectionHandler(clientConfig);
+    return new HttpClientImpl(
+      vertx,
+      resolver,
+      redirectHandler,
+      metrics,
+      po,
+      proxyOptions,
+      nonProxyHosts,
+      loadBalancer,
+      followAlternativeServices,
+      resolverKeepAlive,
+      clientConfig.isVerifyHost(),
+      clientConfig.isSsl(),
+      clientConfig.getDefaultHost(),
+      clientConfig.getDefaultPort(),
+      clientConfig.getMaxRedirects(),
+      clientConfig.getDefaultProtocolVersion(),
+      clientConfig.getSslOptions(),
+      connectHandler,
+      tcpTransport,
+      quicTransport) {
       @Override
       public HttpClientOptions options() {
         return options == null ? new HttpClientOptions() : new HttpClientOptions(options);
@@ -175,7 +197,9 @@ private Handler<HttpConnection> connectionHandler(HttpClientConfig options) {
     Handler<HttpConnection> handler = connectHandler;
     if (windowSize > 0) {
       return connection -> {
-        connection.setWindowSize(windowSize);
+        if (connection.protocolVersion() == HttpVersion.HTTP_2) {
+          connection.setWindowSize(windowSize);
+        }
         if (handler != null) {
           handler.handle(connection);
         }
@@ -193,31 +217,17 @@ public HttpClientAgent build() {
       co = new HttpClientConfig(new HttpClientOptions());
     }
 
-    // Copy options here ????
-    HttpClientImpl.Transport quicTransport;
+    HttpClientTransport quicTransport;
     HttpClientMetrics<?, ?, ?> metrics;
     if (co.getHttp3Config() != null) {
       metrics = vertx.metrics() != null ? vertx.metrics().createHttpClientMetrics(new HttpClientOptions()) : null;
-      quicTransport = new HttpClientImpl.Transport(
-        connectHandler,
-        new Http3ChannelConnector(vertx, metrics, co),
-        co.isVerifyHost(),
-        true,
-        co.getDefaultHost(),
-        co.getDefaultPort(),
-        co.getMaxRedirects(),
-        HttpVersion.HTTP_3,
-        co.getSslOptions()
-      );
+      quicTransport = new Http3ClientTransport(vertx, metrics, co);
     } else {
       quicTransport = null;
       metrics = null;
-      if (co == null) {
-        co = new HttpClientConfig();
-      }
     }
 
-    HttpClientImpl.Transport transport;
+    HttpClientTransport transport;
     String shared;
     EndpointResolver resolver;
     if (co.getHttp1Config() != null && co.getHttp2Config() != null) {
@@ -229,40 +239,30 @@ public HttpClientAgent build() {
       }
       NetClientConfig netClientConfig = netClientConfig(co);
       NetClientInternal tcpClient = new NetClientBuilder(vertx, netClientConfig.setProxyOptions(null)).metrics(metrics).build();
-      Handler<HttpConnection> connectHandler = connectionHandler(co);
-      transport = new HttpClientImpl.Transport(
-        connectHandler,
-        Http1xOrH2ChannelConnector.create(tcpClient, co, metrics),
-        co.isVerifyHost(),
-        co.isSsl(),
-        co.getDefaultHost(),
-        co.getDefaultPort(),
-        co.getMaxRedirects(),
-        co.getDefaultProtocolVersion(),
-        co.getSslOptions()
-      );
+      transport = Http1xOrH2ClientTransport.create(tcpClient, co, metrics);
     } else {
       resolver = null;
       transport = null;
       shared = null;
     }
 
 
+    HttpClientConfig co2 = co;
     CloseFuture cf = resolveCloseFuture();
     HttpClientAgent client;
     Closeable closeable;
     if (shared != null) {
       CloseFuture closeFuture = new CloseFuture();
       HttpClientMetrics<?, ?, ?> m = metrics;
       client = vertx.createSharedResource("__vertx.shared.httpClients", co.getName(), closeFuture, cf_ -> {
-        HttpClientImpl impl = createHttpClientImpl(m, resolver, redirectHandler, transport, quicTransport);
+        HttpClientImpl impl = createHttpClientImpl(co2, m, resolver, redirectHandler, transport, quicTransport);
         cf_.add(completion -> impl.close().onComplete(completion));
         return impl;
       });
       client = new CleanableHttpClient((HttpClientInternal) client, vertx.cleaner(), (timeout, timeunit) -> closeFuture.close());
       closeable = closeFuture;
     } else {
-      HttpClientImpl impl = createHttpClientImpl(metrics, resolver, redirectHandler, transport, quicTransport);
+      HttpClientImpl impl = createHttpClientImpl(co2, metrics, resolver, redirectHandler, transport, quicTransport);
       closeable = impl;
       client = new CleanableHttpClient(impl, vertx.cleaner(), impl::shutdown);
     }