fix #4911: redefining readTimeout as just HttpRequest.timeout

shawkins · manusa · commit e06c2a6fed6d · 2023-05-18T20:11:15.000+02:00
diff --git a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java
@@ -258,9 +258,9 @@ public CompletableFuture<HttpResponse<AsyncBody>> consumeBytesDirect(StandardHtt
   java.net.http.HttpRequest.Builder requestBuilder(StandardHttpRequest request) {
     java.net.http.HttpRequest.Builder requestBuilder = java.net.http.HttpRequest.newBuilder();
 
-    Duration readTimeout = request.getReadTimeout();
-    if (readTimeout != null && !java.time.Duration.ZERO.equals(readTimeout)) {
-      requestBuilder.timeout(readTimeout);
+    Duration timeout = request.getTimeout();
+    if (timeout != null && !java.time.Duration.ZERO.equals(timeout)) {
+      requestBuilder.timeout(timeout);
     }
 
     request.headers().entrySet().stream()
@@ -311,9 +311,9 @@ public CompletableFuture<WebSocketResponse> buildWebSocketDirect(
     if (standardWebSocketBuilder.getSubprotocol() != null) {
       newBuilder.subprotocols(standardWebSocketBuilder.getSubprotocol());
     }
-    Duration readTimeout = request.getReadTimeout();
-    if (readTimeout != null && !java.time.Duration.ZERO.equals(readTimeout)) {
-      newBuilder.connectTimeout(readTimeout);
+    Duration timeout = request.getTimeout();
+    if (timeout != null && !java.time.Duration.ZERO.equals(timeout)) {
+      newBuilder.connectTimeout(timeout);
     }
 
     AtomicLong queueSize = new AtomicLong();
diff --git a/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClient.java b/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClient.java
@@ -99,8 +99,8 @@ private Request newRequest(StandardHttpRequest originalRequest) {
     final var request = requestBuilder.build();
 
     var jettyRequest = jetty.newRequest(request.uri()).method(request.method());
-    if (originalRequest.getReadTimeout() != null) {
-      jettyRequest.timeout(originalRequest.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS);
+    if (originalRequest.getTimeout() != null) {
+      jettyRequest.timeout(originalRequest.getTimeout().toMillis(), TimeUnit.MILLISECONDS);
     }
     jettyRequest.headers(m -> request.headers().forEach((k, l) -> l.forEach(v -> m.add(k, v))));
 
@@ -143,8 +143,8 @@ public CompletableFuture<WebSocketResponse> buildWebSocketDirect(StandardWebSock
         cur.setSubProtocols(standardWebSocketBuilder.getSubprotocol());
       }
       cur.setHeaders(request.headers());
-      if (request.getReadTimeout() != null) {
-        cur.setTimeout(request.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS);
+      if (request.getTimeout() != null) {
+        cur.setTimeout(request.getTimeout().toMillis(), TimeUnit.MILLISECONDS);
       }
       // Extra-future required because we can't Map the UpgradeException to a WebSocketHandshakeException easily
       final CompletableFuture<WebSocketResponse> future = new CompletableFuture<>();
diff --git a/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientImpl.java b/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientImpl.java
@@ -296,9 +296,9 @@ private CompletableFuture<HttpResponse<AsyncBody>> sendAsync(StandardHttpRequest
     CompletableFuture<HttpResponse<AsyncBody>> future = new CompletableFuture<>();
 
     okhttp3.OkHttpClient.Builder clientBuilder = null;
-    if (request.getReadTimeout() != null) {
+    if (request.getTimeout() != null) {
       clientBuilder = httpClient.newBuilder();
-      clientBuilder.readTimeout(request.getReadTimeout());
+      clientBuilder.callTimeout(request.getTimeout());
     }
     if (request.isForStreaming()) {
       if (clientBuilder == null) {
diff --git a/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClient.java b/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClient.java
@@ -69,8 +69,8 @@ public CompletableFuture<WebSocketResponse> buildWebSocketDirect(StandardWebSock
 
     final StandardHttpRequest request = standardWebSocketBuilder.asHttpRequest();
 
-    if (request.getReadTimeout() != null) {
-      options.setTimeout(request.getReadTimeout().toMillis());
+    if (request.getTimeout() != null) {
+      options.setTimeout(request.getTimeout().toMillis());
     }
 
     request.headers().entrySet().stream()
@@ -107,8 +107,8 @@ public CompletableFuture<HttpResponse<AsyncBody>> consumeBytesDirect(StandardHtt
     options.setAbsoluteURI(request.uri().toString());
     options.setMethod(HttpMethod.valueOf(request.method()));
 
-    if (request.getReadTimeout() != null) {
-      options.setTimeout(request.getReadTimeout().toMillis());
+    if (request.getTimeout() != null) {
+      options.setTimeout(request.getTimeout().toMillis());
     }
 
     // Proxy authorization is handled manually since the proxyAuthorization value is the actual header
diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpRequest.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpRequest.java
@@ -99,9 +99,11 @@ default Builder post(Map<String, String> formData) {
     Builder expectContinue();
 
     /**
-     * Sets the read timeout for normal http requests.
+     * Sets the full request cycle timeout.
+     * <br>
+     * Defaults to 0, meaning no timeout
      */
-    Builder readTimeout(long readTimeout, TimeUnit unit);
+    Builder timeout(long timeout, TimeUnit unit);
 
     /**
      * Sets the request to be used for streaming.
diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardHttpClient.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardHttpClient.java
@@ -86,7 +86,7 @@ public CompletableFuture<HttpResponse<AsyncBody>> consumeBytes(HttpRequest reque
 
     retryWithExponentialBackoff(result, () -> consumeBytesOnce(standardHttpRequest, consumer), request.uri(),
         HttpResponse::code,
-        r -> r.body().cancel(), standardHttpRequest.getReadTimeout());
+        r -> r.body().cancel(), standardHttpRequest.getTimeout());
     return result;
   }
 
@@ -226,7 +226,7 @@ final CompletableFuture<WebSocket> buildWebSocket(StandardWebSocketBuilder stand
     retryWithExponentialBackoff(intermediate, () -> buildWebSocketOnce(standardWebSocketBuilder, listener),
         request.uri(),
         r -> Optional.of(r.webSocketUpgradeResponse).map(HttpResponse::code).orElse(null),
-        r -> Optional.ofNullable(r.webSocket).ifPresent(w -> w.sendClose(1000, null)), request.getReadTimeout());
+        r -> Optional.ofNullable(r.webSocket).ifPresent(w -> w.sendClose(1000, null)), request.getTimeout());
 
     CompletableFuture<WebSocket> result = new CompletableFuture<>();
 
diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardHttpRequest.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardHttpRequest.java
@@ -95,7 +95,7 @@ public long getLength() {
   private final String bodyString;
   private final BodyContent body;
   private final boolean expectContinue;
-  private final Duration readTimeout;
+  private final Duration timeout;
   private final boolean forStreaming;
 
   /**
@@ -111,7 +111,7 @@ public StandardHttpRequest(Map<String, List<String>> headers, URI uri, String me
   }
 
   StandardHttpRequest(Map<String, List<String>> headers, URI uri, String method, String bodyString,
-      BodyContent body, boolean expectContinue, String contentType, Duration readTimeout, boolean forStreaming) {
+      BodyContent body, boolean expectContinue, String contentType, Duration timeout, boolean forStreaming) {
     super(headers);
     this.id = UUID.randomUUID();
     this.uri = uri;
@@ -120,7 +120,7 @@ public StandardHttpRequest(Map<String, List<String>> headers, URI uri, String me
     this.body = body;
     this.expectContinue = expectContinue;
     this.contentType = contentType;
-    this.readTimeout = readTimeout;
+    this.timeout = timeout;
     this.forStreaming = forStreaming;
   }
 
@@ -167,8 +167,8 @@ public boolean isForStreaming() {
     return forStreaming;
   }
 
-  public Duration getReadTimeout() {
-    return readTimeout;
+  public Duration getTimeout() {
+    return timeout;
   }
 
   public static final class Builder extends AbstractBasicBuilder<Builder> implements HttpRequest.Builder {
@@ -178,7 +178,7 @@ public static final class Builder extends AbstractBasicBuilder<Builder> implemen
     private String bodyAsString;
     private boolean expectContinue;
     private String contentType;
-    protected Duration readTimeout;
+    protected Duration timeout;
     protected boolean forStreaming;
 
     public Builder() {
@@ -192,19 +192,19 @@ public Builder(StandardHttpRequest original) {
       body = original.body;
       expectContinue = original.expectContinue;
       contentType = original.contentType;
-      readTimeout = original.readTimeout;
+      timeout = original.timeout;
       forStreaming = original.forStreaming;
     }
 
     @Override
     public StandardHttpRequest build() {
       return new StandardHttpRequest(getHeaders(), Objects.requireNonNull(getUri()), method, bodyAsString, body, expectContinue,
-          contentType, readTimeout, forStreaming);
+          contentType, timeout, forStreaming);
     }
 
     @Override
-    public HttpRequest.Builder readTimeout(long readTimeout, TimeUnit unit) {
-      this.readTimeout = Duration.ofNanos(unit.toNanos(readTimeout));
+    public HttpRequest.Builder timeout(long timeout, TimeUnit unit) {
+      this.timeout = Duration.ofNanos(unit.toNanos(timeout));
       return this;
     }
 
diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardWebSocketBuilder.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/StandardWebSocketBuilder.java
@@ -79,7 +79,7 @@ public StandardHttpRequest asHttpRequest() {
 
   @Override
   public Builder connectTimeout(long timeout, TimeUnit timeUnit) {
-    this.builder.readTimeout(timeout, timeUnit);
+    this.builder.timeout(timeout, timeUnit);
     return this;
   }
 
diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/StandardHttpClientTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/StandardHttpClientTest.java
@@ -216,7 +216,7 @@ void testClosePreviousBeforeRetry() throws Exception {
   @Test
   void testRequestTimeout() throws Exception {
     CompletableFuture<HttpResponse<AsyncBody>> consumeFuture = client.consumeBytes(
-        client.newHttpRequestBuilder().uri("http://localhost").readTimeout(1, TimeUnit.MILLISECONDS).build(),
+        client.newHttpRequestBuilder().uri("http://localhost").timeout(1, TimeUnit.MILLISECONDS).build(),
         (value, asyncBody) -> {
         });
 
diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java
@@ -401,7 +401,7 @@ public L list(Integer limitVal, String continueVal) {
   public CompletableFuture<L> submitList(ListOptions listOptions) {
     try {
       URL fetchListUrl = fetchListUrl(getNamespacedUrl(), defaultListOptions(listOptions, null));
-      HttpRequest.Builder requestBuilder = withReadTimeout(httpClient.newHttpRequestBuilder()).url(fetchListUrl);
+      HttpRequest.Builder requestBuilder = withRequestTimeout(httpClient.newHttpRequestBuilder()).url(fetchListUrl);
       Type refinedType = listType.equals(DefaultKubernetesResourceList.class)
           ? Serialization.jsonMapper().getTypeFactory().constructParametricType(listType, type)
           : listType;
diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java
@@ -489,8 +489,8 @@ protected <T> T handleRawGet(URL resourceUrl, Class<T> type) throws IOException
     return handleRaw(type, resourceUrl.toString(), "GET", null);
   }
 
-  HttpRequest.Builder withReadTimeout(HttpRequest.Builder builder) {
-    return builder.readTimeout(getRequestConfig().getRequestTimeout(), TimeUnit.MILLISECONDS);
+  HttpRequest.Builder withRequestTimeout(HttpRequest.Builder builder) {
+    return builder.timeout(getRequestConfig().getRequestTimeout(), TimeUnit.MILLISECONDS);
   }
 
   /**
@@ -536,7 +536,7 @@ protected <T> T waitForResult(CompletableFuture<T> future) throws IOException {
    * @throws IOException IOException
    */
   protected <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type) throws IOException {
-    return waitForResult(handleResponse(httpClient, withReadTimeout(requestBuilder), new TypeReference<T>() {
+    return waitForResult(handleResponse(httpClient, withRequestTimeout(requestBuilder), new TypeReference<T>() {
       @Override
       public Type getType() {
         return type;
@@ -765,7 +765,7 @@ public <R1> R1 handleRaw(Class<R1> result, String uri, String method, Object pay
       } else if (payload != null) {
         body = Serialization.asJson(payload);
       }
-      HttpRequest request = httpClient.newHttpRequestBuilder().uri(uri).method(method, JSON, body).build();
+      HttpRequest request = withRequestTimeout(httpClient.newHttpRequestBuilder().uri(uri).method(method, JSON, body)).build();
       HttpResponse<R1> response = waitForResult(httpClient.sendAsync(request, result));
       assertResponseCode(request, response);
       return response.body();
diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java b/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java
@@ -20,7 +20,6 @@
 import io.fabric8.kubernetes.api.model.EventList;
 import io.fabric8.kubernetes.client.Client;
 import io.fabric8.kubernetes.client.KubernetesClientException;
-import io.fabric8.kubernetes.client.RequestConfigBuilder;
 import io.fabric8.kubernetes.client.dsl.Triggerable;
 import io.fabric8.kubernetes.client.dsl.Typeable;
 import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperation;
@@ -80,8 +79,7 @@ public class BuildConfigOperationsImpl
   private final String asFile;
 
   public BuildConfigOperationsImpl(Client client) {
-    this(new BuildConfigOperationContext(), HasMetadataOperationsImpl.defaultContext(client).withRequestConfig(
-        new RequestConfigBuilder(client.getConfiguration().getRequestConfig()).withRequestTimeout(0).build()));
+    this(new BuildConfigOperationContext(), HasMetadataOperationsImpl.defaultContext(client));
   }
 
   public BuildConfigOperationsImpl(BuildConfigOperationContext context, OperationContext superContext) {
@@ -241,10 +239,7 @@ public CommitterEmailable<AuthorMessageAsFileTimeoutInputStreamable<Build>> with
   @Override
   public BuildConfigOperationsImpl withTimeout(long timeout, TimeUnit unit) {
     return new BuildConfigOperationsImpl(getContext(), context
-        .withTimeout(timeout, unit)
-        .withRequestConfig(new RequestConfigBuilder(context.getRequestConfig())
-            .withRequestTimeout((int) unit.toMillis(timeout))
-            .build()));
+        .withTimeout(timeout, unit));
   }
 
   @Override
@@ -262,7 +257,7 @@ protected Build submitToApiServer(InputStream inputStream, long contentLength) {
       HttpRequest.Builder requestBuilder = this.httpClient.newHttpRequestBuilder()
           .post("application/octet-stream", inputStream, contentLength)
           .expectContinue()
-          .readTimeout(getOperationContext().getTimeout(), getOperationContext().getTimeoutUnit())
+          .timeout(getOperationContext().getTimeout(), getOperationContext().getTimeoutUnit())
           .uri(getQueryParameters());
       return waitForResult(handleResponse(this.httpClient, requestBuilder, new TypeReference<Build>() {
         @Override
diff --git a/openshift-client/src/test/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImplTest.java b/openshift-client/src/test/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImplTest.java
@@ -59,7 +59,7 @@ public void setUp() {
     when(httpClient.newHttpRequestBuilder()
         .post(any(), any(), anyLong())
         .header(any(), any())
-        .readTimeout(anyLong(), any())
+        .timeout(anyLong(), any())
         .uri(any(String.class))
         .build()).thenReturn(response);
 
@@ -71,23 +71,23 @@ public void setUp() {
 
   @Test
   void requestTimeoutDefaultsToZero() {
-    assertThat(new BuildConfigOperationsImpl(client).getOperationContext().getRequestConfig().getRequestTimeout())
+    assertThat(new BuildConfigOperationsImpl(client).getOperationContext().getTimeout())
         .isZero();
   }
 
   @Test
   void withTimeoutOverridesRequestTimeout() {
     final BuildConfigOperationsImpl buildConfigOperations = new BuildConfigOperationsImpl(client)
         .withTimeout(1337, TimeUnit.MILLISECONDS);
-    assertThat(buildConfigOperations.getOperationContext().getRequestConfig().getRequestTimeout())
+    assertThat(buildConfigOperations.getOperationContext().getTimeout())
         .isEqualTo(1337);
   }
 
   @Test
   void withTimeoutInMillisOverridesRequestTimeout() {
     final BuildConfigOperationsImpl buildConfigOperations = new BuildConfigOperationsImpl(client)
         .withTimeoutInMillis(1337);
-    assertThat(buildConfigOperations.getOperationContext().getRequestConfig().getRequestTimeout())
+    assertThat(buildConfigOperations.getOperationContext().getTimeout())
         .isEqualTo(1337);
   }
 

Original file line number	Diff line number	Diff line change
`@@ -99,8 +99,8 @@ private Request newRequest(StandardHttpRequest originalRequest) {`
`99`	`99`	`final var request = requestBuilder.build();`
`100`	`100`
`101`	`101`	`var jettyRequest = jetty.newRequest(request.uri()).method(request.method());`
`102`		`- if (originalRequest.getReadTimeout() != null) {`
`103`		`- jettyRequest.timeout(originalRequest.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS);`
	`102`	`+ if (originalRequest.getTimeout() != null) {`
	`103`	`+ jettyRequest.timeout(originalRequest.getTimeout().toMillis(), TimeUnit.MILLISECONDS);`
`104`	`104`	`}`
`105`	`105`	`jettyRequest.headers(m -> request.headers().forEach((k, l) -> l.forEach(v -> m.add(k, v))));`
`106`	`106`
`@@ -143,8 +143,8 @@ public CompletableFuture<WebSocketResponse> buildWebSocketDirect(StandardWebSock`
`143`	`143`	`cur.setSubProtocols(standardWebSocketBuilder.getSubprotocol());`
`144`	`144`	`}`
`145`	`145`	`cur.setHeaders(request.headers());`
`146`		`- if (request.getReadTimeout() != null) {`
`147`		`- cur.setTimeout(request.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS);`
	`146`	`+ if (request.getTimeout() != null) {`
	`147`	`+ cur.setTimeout(request.getTimeout().toMillis(), TimeUnit.MILLISECONDS);`
`148`	`148`	`}`
`149`	`149`	`// Extra-future required because we can't Map the UpgradeException to a WebSocketHandshakeException easily`
`150`	`150`	`final CompletableFuture<WebSocketResponse> future = new CompletableFuture<>();`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ public StandardHttpRequest asHttpRequest() {`
`79`	`79`
`80`	`80`	`@Override`
`81`	`81`	`public Builder connectTimeout(long timeout, TimeUnit timeUnit) {`
`82`		`- this.builder.readTimeout(timeout, timeUnit);`
	`82`	`+ this.builder.timeout(timeout, timeUnit);`
`83`	`83`	`return this;`
`84`	`84`	`}`
`85`	`85`