From 1e753695fb8357b836664d4a8e16a2178d2f347d Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Wed, 14 Sep 2022 12:07:38 +0300 Subject: [PATCH] Ensure HttpClientConfig#connectionProvider returns the original provider used to create HttpClient Fixes #2462 --- .../transport/ClientTransportConfig.java | 2 +- .../netty/http/client/HttpClientConfig.java | 11 +++- .../netty/http/client/HttpClientConnect.java | 5 +- .../netty/http/client/HttpClientTest.java | 4 +- .../client/HttpConnectionProviderTest.java | 52 ++++++++++++++++--- 5 files changed, 59 insertions(+), 15 deletions(-) diff --git a/reactor-netty-core/src/main/java/reactor/netty/transport/ClientTransportConfig.java b/reactor-netty-core/src/main/java/reactor/netty/transport/ClientTransportConfig.java index ec11b36b10..916acd5ded 100644 --- a/reactor-netty-core/src/main/java/reactor/netty/transport/ClientTransportConfig.java +++ b/reactor-netty-core/src/main/java/reactor/netty/transport/ClientTransportConfig.java @@ -60,7 +60,7 @@ public int channelHash() { * * @return the {@link ConnectionProvider} */ - public final ConnectionProvider connectionProvider() { + public ConnectionProvider connectionProvider() { return connectionProvider; } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java index 062517539d..0b414da173 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java @@ -115,6 +115,11 @@ public ChannelOperations.OnSetup channelOperationsProvider() { return (ch, c, msg) -> new HttpClientOperations(ch, c, cookieEncoder, cookieDecoder); } + @Override + public ConnectionProvider connectionProvider() { + return httpConnectionProvider().http1ConnectionProvider(); + } + /** * Return the configured {@link ClientCookieDecoder} or the default {@link ClientCookieDecoder#STRICT}. * @@ -321,7 +326,7 @@ public WebsocketClientSpec websocketClientSpec() { Function uriTagValue; WebsocketClientSpec websocketClientSpec; - HttpClientConfig(ConnectionProvider connectionProvider, Map, ?> options, + HttpClientConfig(HttpConnectionProvider connectionProvider, Map, ?> options, Supplier remoteAddress) { super(connectionProvider, options, remoteAddress); this.acceptGzip = false; @@ -444,6 +449,10 @@ void deferredConf(Function> deferrer) { } } + HttpConnectionProvider httpConnectionProvider() { + return (HttpConnectionProvider) super.connectionProvider(); + } + void protocols(HttpProtocol... protocols) { this.protocols = protocols; int _protocols = 0; diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java index 615374ed6a..aa8c202c39 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java @@ -57,7 +57,6 @@ import reactor.netty.channel.AbortedException; import reactor.netty.http.HttpOperations; import reactor.netty.http.HttpProtocol; -import reactor.netty.resources.ConnectionProvider; import reactor.netty.tcp.TcpClientConfig; import reactor.netty.transport.AddressUtils; import reactor.netty.transport.ProxyProvider; @@ -81,7 +80,7 @@ class HttpClientConnect extends HttpClient { final HttpClientConfig config; - HttpClientConnect(ConnectionProvider provider) { + HttpClientConnect(HttpConnectionProvider provider) { this.config = new HttpClientConfig( provider, Collections.singletonMap(ChannelOption.AUTO_READ, false), @@ -265,7 +264,7 @@ public void subscribe(CoreSubscriber actual) { AddressResolverGroup resolver = _config.resolverInternal(); - _config.connectionProvider() + _config.httpConnectionProvider() .acquire(_config, observer, handler, resolver) .subscribe(new ClientTransportSubscriber(sink)); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java index b3e4f66b80..77f97f878f 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java @@ -3165,7 +3165,7 @@ private void doTestIssue1943(HttpProtocol protocol) { HttpClientConfig config = client.configuration(); LoopResources loopResources1 = config.loopResources(); - ConnectionProvider provider1 = ((HttpConnectionProvider) config.connectionProvider()).http1ConnectionProvider(); + ConnectionProvider provider1 = config.connectionProvider(); AddressResolverGroup resolverGroup1 = config.defaultAddressResolverGroup(); try { @@ -3182,7 +3182,7 @@ private void doTestIssue1943(HttpProtocol protocol) { HttpResources.reset(); LoopResources loopResources2 = config.loopResources(); - ConnectionProvider provider2 = ((HttpConnectionProvider) config.connectionProvider()).http1ConnectionProvider(); + ConnectionProvider provider2 = config.connectionProvider(); AddressResolverGroup resolverGroup2 = config.defaultAddressResolverGroup(); assertThat(loopResources1).isNotSameAs(loopResources2); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpConnectionProviderTest.java index d43802ce35..3160dfa1de 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpConnectionProviderTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpConnectionProviderTest.java @@ -18,6 +18,7 @@ import org.junit.jupiter.api.Test; import reactor.netty.http.HttpResources; import reactor.netty.resources.ConnectionProvider; +import reactor.util.annotation.Nullable; import java.time.Duration; @@ -34,8 +35,7 @@ void maxConnectionsCustomConnectionProvider() { ConnectionProvider provider = ConnectionProvider.create("maxConnectionsCustomConnectionProvider", 1); try { HttpClient client = HttpClient.create(provider); - HttpConnectionProvider configuredConnectionProvider = - (HttpConnectionProvider) client.configuration().connectionProvider(); + HttpConnectionProvider configuredConnectionProvider = client.configuration().httpConnectionProvider(); assertThat(configuredConnectionProvider.maxConnections()) .isEqualTo(provider.maxConnections()); // Http2ConnectionProvider inherits the configuration from the configured provider @@ -51,8 +51,7 @@ void maxConnectionsCustomConnectionProvider() { @Test void maxConnectionsDefaultConnectionProvider() { HttpClient client = HttpClient.create(); - HttpConnectionProvider configuredConnectionProvider = - (HttpConnectionProvider) client.configuration().connectionProvider(); + HttpConnectionProvider configuredConnectionProvider = client.configuration().httpConnectionProvider(); assertThat(configuredConnectionProvider.maxConnections()) .isEqualTo(((ConnectionProvider) HttpResources.get()).maxConnections()); // Http2ConnectionProvider inherits the configuration from HttpResources @@ -66,8 +65,7 @@ void maxConnectionsPerHostCustomConnectionProvider() { ConnectionProvider provider = ConnectionProvider.create("maxConnectionsPerHostCustomConnectionProvider", 1); try { HttpClient client = HttpClient.create(provider); - HttpConnectionProvider configuredConnectionProvider = - (HttpConnectionProvider) client.configuration().connectionProvider(); + HttpConnectionProvider configuredConnectionProvider = client.configuration().httpConnectionProvider(); assertThat(configuredConnectionProvider.maxConnectionsPerHost()) .isEqualTo(provider.maxConnectionsPerHost()); // Http2ConnectionProvider inherits the configuration from the configured provider @@ -83,8 +81,7 @@ void maxConnectionsPerHostCustomConnectionProvider() { @Test void maxConnectionsPerHostDefaultConnectionProvider() { HttpClient client = HttpClient.create(); - HttpConnectionProvider configuredConnectionProvider = - (HttpConnectionProvider) client.configuration().connectionProvider(); + HttpConnectionProvider configuredConnectionProvider = client.configuration().httpConnectionProvider(); assertThat(configuredConnectionProvider.maxConnectionsPerHost()) .isEqualTo(((ConnectionProvider) HttpResources.get()).maxConnectionsPerHost()); // Http2ConnectionProvider inherits the configuration from HttpResources @@ -92,4 +89,43 @@ void maxConnectionsPerHostDefaultConnectionProvider() { .isEqualTo(HttpResources.get().getOrCreateHttp2ConnectionProvider(HTTP2_CONNECTION_PROVIDER_FACTORY) .maxConnectionsPerHost()); } + + @Test + void returnOriginalConnectionProvider() { + testReturnOriginalConnectionProvider(HttpClient.create(), null); + } + + @Test + void returnOriginalConnectionProviderNewConnection() { + ConnectionProvider provider = ConnectionProvider.newConnection(); + testReturnOriginalConnectionProvider(HttpClient.create(provider), provider); + } + + @Test + void returnOriginalConnectionProviderUsingBuilder() { + ConnectionProvider provider = + ConnectionProvider.builder("provider") + .maxConnections(1) + .disposeTimeout(Duration.ofSeconds(1L)) + .pendingAcquireTimeout(Duration.ofSeconds(1L)) + .maxIdleTime(Duration.ofSeconds(1L)) + .maxLifeTime(Duration.ofSeconds(10L)) + .lifo() + .build(); + + testReturnOriginalConnectionProvider(HttpClient.create(provider), provider); + } + + private void testReturnOriginalConnectionProvider(HttpClient httpClient, @Nullable ConnectionProvider originalProvider) { + ConnectionProvider provider = httpClient.configuration().connectionProvider(); + try { + assertThat(provider).isSameAs(originalProvider != null ? originalProvider : HttpResources.get()); + } + finally { + if (originalProvider != null) { + provider.disposeLater() + .block(Duration.ofSeconds(5)); + } + } + } }