Skip to content

Commit

Permalink
fix fabric8io#4911: redefining readTimeout as just HttpRequest.timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed May 17, 2023
1 parent f6d0b6b commit 1e0dabc
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))));

Expand Down Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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() {
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> {
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
}

Expand Down

0 comments on commit 1e0dabc

Please sign in to comment.