fix: OkHttpClientFactory.additionalConfig can be used to override the default OkHttp Dispatcher

Signed-off-by: Marc Nuri <[email protected]>

Author: manusa

CHANGELOG.md

@@ -8,6 +8,7 @@
 * Fix #4832: NO_PROXY can match cidr with bit suffix <10
 * Fix #4851: adding buffer cloning to ensure buffers cannot be modified after sending
 * Fix #4794: improving the semantics of manually calling informer stop
+* Fix #4797: OkHttpClientFactory.additionalConfig can be used to override the default OkHttp Dispatcher
 * Fix #4798: fix leader election release on cancel
 * Fix #4815: (java-generator) create target download directory if it doesn't exist
 * Fix #4846: allowed for pod read / copy operations to distinguish when the target doesn't exist

httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientBuilderImpl.java

@@ -19,7 +19,6 @@
 import io.fabric8.kubernetes.client.http.StandardHttpClientBuilder;
 import okhttp3.Authenticator;
 import okhttp3.ConnectionSpec;
-import okhttp3.Dispatcher;
 import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
 import okhttp3.Protocol;
@@ -36,7 +35,7 @@
 class OkHttpClientBuilderImpl
     extends StandardHttpClientBuilder<OkHttpClientImpl, OkHttpClientFactory, OkHttpClientBuilderImpl> {
 
-  private okhttp3.OkHttpClient.Builder builder;
+  private final okhttp3.OkHttpClient.Builder builder;
 
   public OkHttpClientBuilderImpl(OkHttpClientFactory clientFactory, okhttp3.OkHttpClient.Builder builder) {
     super(clientFactory);
@@ -81,8 +80,7 @@ public OkHttpClientImpl initialBuild(okhttp3.OkHttpClient.Builder builder) {
     }
     if (tlsVersions != null) {
       ConnectionSpec spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
-          .tlsVersions(Arrays.asList(tlsVersions)
-              .stream()
+          .tlsVersions(Arrays.stream(tlsVersions)
               .map(tls -> okhttp3.TlsVersion.valueOf(tls.name()))
               .toArray(okhttp3.TlsVersion[]::new))
           .build();
@@ -91,14 +89,6 @@ public OkHttpClientImpl initialBuild(okhttp3.OkHttpClient.Builder builder) {
     if (preferHttp11) {
       builder.protocols(Collections.singletonList(Protocol.HTTP_1_1));
     }
-    Dispatcher dispatcher = new Dispatcher();
-    // websockets and long running http requests count against this and eventually starve
-    // the work that can be done
-    dispatcher.setMaxRequests(Integer.MAX_VALUE);
-    // long running http requests count against this and eventually exhaust
-    // the work that can be done
-    dispatcher.setMaxRequestsPerHost(Integer.MAX_VALUE);
-    builder.dispatcher(dispatcher);
     return derivedBuild(builder);
   }
 

httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactory.java

@@ -20,6 +20,7 @@
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.http.HttpClient;
 import io.fabric8.kubernetes.client.utils.HttpClientUtils;
+import okhttp3.Dispatcher;
 import okhttp3.OkHttpClient;
 import okhttp3.logging.HttpLoggingInterceptor;
 import org.slf4j.Logger;
@@ -38,7 +39,7 @@ public boolean isDefault() {
    * Subclasses may use this to apply a base configuration to the builder
    */
   protected OkHttpClient.Builder newOkHttpClientBuilder() {
-    return new OkHttpClient.Builder();
+    return new OkHttpClient.Builder().dispatcher(initDispatcher());
   }
 
   /**
@@ -108,4 +109,15 @@ protected boolean shouldDisableHttp2() {
     return System.getProperty("java.version", "").startsWith("1.8");
   }
 
+  protected Dispatcher initDispatcher() {
+    Dispatcher dispatcher = new Dispatcher();
+    // websockets and long-running http requests count against this and eventually starve
+    // the work that can be done
+    dispatcher.setMaxRequests(Integer.MAX_VALUE);
+    // long-running http requests count against this and eventually exhaust
+    // the work that can be done
+    dispatcher.setMaxRequestsPerHost(Integer.MAX_VALUE);
+    return dispatcher;
+  }
+
 }

kubernetes-client/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java renamed to httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java

@@ -18,19 +18,22 @@
 
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.ConfigBuilder;
+import okhttp3.Dispatcher;
+import okhttp3.OkHttpClient;
 import org.junit.jupiter.api.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 class OkHttpClientFactoryTest {
 
   @Test
   void shouldNotRespectMaxRequests() {
-    OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build();
+    OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(Config.empty()).build();
 
     assertEquals(Integer.MAX_VALUE, client.getOkHttpClient().dispatcher().getMaxRequests());
 
-    Config config = new ConfigBuilder()
+    Config config = new ConfigBuilder(Config.empty())
         .withMaxConcurrentRequests(120)
         .build();
 
@@ -40,11 +43,11 @@ void shouldNotRespectMaxRequests() {
 
   @Test
   void shouldNotRespectMaxRequestsPerHost() {
-    OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build();
+    OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(Config.empty()).build();
 
     assertEquals(Integer.MAX_VALUE, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost());
 
-    Config config = new ConfigBuilder()
+    Config config = new ConfigBuilder(Config.empty())
         .withMaxConcurrentRequestsPerHost(20)
         .build();
 
@@ -53,4 +56,37 @@ void shouldNotRespectMaxRequestsPerHost() {
     assertEquals(Integer.MAX_VALUE, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost());
   }
 
+  @Test
+  void additionalConfigCanOverrideDispatcher() {
+    final Dispatcher dispatcher = new Dispatcher();
+    OkHttpClientImpl client = new OkHttpClientFactory() {
+      @Override
+      protected void additionalConfig(OkHttpClient.Builder builder) {
+        dispatcher.setMaxRequests(1337);
+        builder.dispatcher(dispatcher);
+      }
+    }
+        .newBuilder(Config.empty())
+        .build();
+    assertThat(client.getOkHttpClient().dispatcher())
+        .isSameAs(dispatcher)
+        .hasFieldOrPropertyWithValue("maxRequests", 1337);
+  }
+
+  @Test
+  void initDispatcherCanOverrideDispatcher() {
+    OkHttpClientImpl client = new OkHttpClientFactory() {
+      @Override
+      protected Dispatcher initDispatcher() {
+        final Dispatcher dispatcher = super.initDispatcher();
+        dispatcher.setMaxRequests(1337);
+        return dispatcher;
+      }
+    }
+        .newBuilder(Config.empty())
+        .build();
+    assertThat(client.getOkHttpClient().dispatcher())
+        .hasFieldOrPropertyWithValue("maxRequests", 1337);
+  }
+
 }