Skip to content

Commit

Permalink
feat: introduce java.time methods and variables (#2826)
Browse files Browse the repository at this point in the history
* feat: introduce `java.time` methods and variables

* improve DurationDuration method name

* adjust naming
  • Loading branch information
diegomarquezp authored Dec 2, 2024
1 parent ca13691 commit baf30ee
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions>
public void close() throws Exception {
try (StorageClient s = storageClient) {
s.shutdownNow();
org.threeten.bp.Duration terminationAwaitDuration =
getOptions().getTerminationAwaitDuration();
java.time.Duration terminationAwaitDuration = getOptions().getTerminationAwaitDuration();
s.awaitTermination(terminationAwaitDuration.toMillis(), TimeUnit.MILLISECONDS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

package com.google.cloud.storage;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;
import static com.google.api.gax.util.TimeConversionUtils.toThreetenDuration;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.api.core.ObsoleteApi;
import com.google.api.gax.core.CredentialsProvider;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.api.gax.core.NoCredentialsProvider;
Expand Down Expand Up @@ -97,7 +100,6 @@
import java.util.Objects;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.threeten.bp.Duration;

/** @since 2.14.0 */
@TransportCompatibility(Transport.GRPC)
Expand All @@ -110,7 +112,7 @@ public final class GrpcStorageOptions extends StorageOptions
private static final String DEFAULT_HOST = "https://storage.googleapis.com";

private final GrpcRetryAlgorithmManager retryAlgorithmManager;
private final Duration terminationAwaitDuration;
private final java.time.Duration terminationAwaitDuration;
private final boolean attemptDirectPath;
private final boolean enableGrpcClientMetrics;

Expand All @@ -126,7 +128,8 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults)
builder.storageRetryStrategy, serviceDefaults.getStorageRetryStrategy()));
this.terminationAwaitDuration =
MoreObjects.firstNonNull(
builder.terminationAwaitDuration, serviceDefaults.getTerminationAwaitDuration());
builder.terminationAwaitDuration,
serviceDefaults.getTerminationAwaitDurationJavaTime());
this.attemptDirectPath = builder.attemptDirectPath;
this.enableGrpcClientMetrics = builder.enableGrpcClientMetrics;
this.grpcClientMetricsManuallyEnabled = builder.grpcMetricsManuallyEnabled;
Expand All @@ -145,7 +148,7 @@ GrpcRetryAlgorithmManager getRetryAlgorithmManager() {
}

@InternalApi
Duration getTerminationAwaitDuration() {
java.time.Duration getTerminationAwaitDuration() {
return terminationAwaitDuration;
}

Expand Down Expand Up @@ -314,17 +317,17 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
// seconds.
// To allow read streams to have longer lifespans, crank up their timeouts, instead rely
// on idleTimeout below.
.setLogicalTimeout(Duration.ofDays(28))
.setLogicalTimeout(java.time.Duration.ofDays(28))
.build();
Duration totalTimeout = baseRetrySettings.getTotalTimeout();
java.time.Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration();
Set<Code> startResumableWriteRetryableCodes =
builder.startResumableWriteSettings().getRetryableCodes();

// retries for unary methods are generally handled at a different level, except
// StartResumableWrite
builder.applyToAllUnaryMethods(
input -> {
input.setSimpleTimeoutNoRetries(totalTimeout);
input.setSimpleTimeoutNoRetriesDuration(totalTimeout);
return null;
});

Expand All @@ -342,7 +345,7 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
// for reads, the stream can be held open for a long time in order to read all bytes,
// this is totally valid. instead we want to monitor if the stream is doing work and if not
// timeout.
.setIdleTimeout(totalTimeout);
.setIdleTimeoutDuration(totalTimeout);
return Tuple.of(builder.build(), defaultOpts);
}

Expand Down Expand Up @@ -412,7 +415,7 @@ protected boolean shouldRefreshService(Storage cachedService) {
public static final class Builder extends StorageOptions.Builder {

private StorageRetryStrategy storageRetryStrategy;
private Duration terminationAwaitDuration;
private java.time.Duration terminationAwaitDuration;
private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath();
private boolean enableGrpcClientMetrics =
GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics();
Expand All @@ -436,6 +439,15 @@ public static final class Builder extends StorageOptions.Builder {
this.blobWriteSessionConfig = gso.blobWriteSessionConfig;
}

/**
* This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(java.time.Duration)}
* instead.
*/
@ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(java.time.Duration) instead")
public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationAwaitDuration) {
return setTerminationAwaitJavaTimeDuration(toJavaTimeDuration(terminationAwaitDuration));
}

/**
* Set the maximum duration in which to await termination of any outstanding requests when
* calling {@link Storage#close()}
Expand All @@ -444,7 +456,8 @@ public static final class Builder extends StorageOptions.Builder {
* @return the builder
* @since 2.14.0
*/
public Builder setTerminationAwaitDuration(Duration terminationAwaitDuration) {
public Builder setTerminationAwaitJavaTimeDuration(
java.time.Duration terminationAwaitDuration) {
this.terminationAwaitDuration =
requireNonNull(terminationAwaitDuration, "terminationAwaitDuration must be non null");
return this;
Expand Down Expand Up @@ -652,9 +665,15 @@ public StorageRetryStrategy getStorageRetryStrategy() {
return StorageRetryStrategy.getDefaultStorageRetryStrategy();
}

/** This method is obsolete. Use {@link #getTerminationAwaitDurationJavaTime()} instead. */
@ObsoleteApi("Use getTerminationAwaitDurationJavaTime() instead")
public org.threeten.bp.Duration getTerminationAwaitDuration() {
return toThreetenDuration(getTerminationAwaitDurationJavaTime());
}

/** @since 2.14.0 */
public Duration getTerminationAwaitDuration() {
return Duration.ofMinutes(1);
public java.time.Duration getTerminationAwaitDurationJavaTime() {
return java.time.Duration.ofMinutes(1);
}

/** @since 2.14.0 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.common.base.Strings;
import java.io.IOException;
import java.io.InputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
Expand All @@ -44,7 +45,6 @@
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.threeten.bp.Duration;

/**
* Utility to create a remote storage configuration for testing. Storage options can be obtained via
Expand Down Expand Up @@ -233,13 +233,13 @@ public static RemoteStorageHelper create() throws StorageHelperException {
private static RetrySettings retrySettings() {
return RetrySettings.newBuilder()
.setMaxAttempts(10)
.setMaxRetryDelay(Duration.ofMillis(30000L))
.setTotalTimeout(Duration.ofMillis(120000L))
.setInitialRetryDelay(Duration.ofMillis(250L))
.setMaxRetryDelayDuration(Duration.ofMillis(30000L))
.setTotalTimeoutDuration(Duration.ofMillis(120000L))
.setInitialRetryDelayDuration(Duration.ofMillis(250L))
.setRetryDelayMultiplier(1.0)
.setInitialRpcTimeout(Duration.ofMillis(120000L))
.setInitialRpcTimeoutDuration(Duration.ofMillis(120000L))
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ofMillis(120000L))
.setMaxRpcTimeoutDuration(Duration.ofMillis(120000L))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@
import io.grpc.Status.Code;
import io.grpc.stub.StreamObserver;
import java.io.IOException;
import java.time.Duration;
import java.util.Iterator;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.threeten.bp.Duration;

/**
* ReadObject leverages gRPC ServerStream to read a stream of ReadObjectResponse messages spanning
Expand All @@ -48,7 +48,7 @@
* #getTotalTimeout()}, gax will interrupt the stream with a DEADLINE_EXCEEDED error.
*
* <p>Instead of relying on total stream timeout, we rely on idleTimeout for the stream via {@link
* com.google.api.gax.rpc.ServerStreamingCallSettings.Builder#setIdleTimeout(Duration)}.
* com.google.api.gax.rpc.ServerStreamingCallSettings.Builder#setIdleTimeoutDuration(Duration)}.
*
* <p>These tests force specific timeout scenarios to happen against an in-process grpc server to
* ensure our configuration of the StorageClient properly translates to the behavior we want.
Expand Down Expand Up @@ -117,7 +117,7 @@ public void readObject(
.setRetrySettings(
RetrySettings.newBuilder()
.setMaxAttempts(3)
.setTotalTimeout(Duration.ofMillis(totalTimeoutMillis))
.setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis))
.build())
.build()
.getStorageSettings();
Expand Down Expand Up @@ -196,7 +196,7 @@ public void readObject(
.setRetrySettings(
RetrySettings.newBuilder()
.setMaxAttempts(3)
.setTotalTimeout(Duration.ofMillis(totalTimeoutMillis))
.setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis))
.build())
.build()
.getStorageSettings();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.Optional;
import java.util.logging.Logger;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.threeten.bp.Clock;
import org.threeten.bp.Instant;
import org.threeten.bp.ZoneId;
import org.threeten.bp.ZoneOffset;
import org.threeten.bp.format.DateTimeFormatter;

@RunWith(StorageITRunner.class)
@SingleBackend(Backend.TEST_BENCH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@
import com.google.cloud.storage.it.runner.annotations.Backend;
import com.google.cloud.storage.it.runner.annotations.CrossRun;
import com.google.cloud.storage.it.runner.annotations.Inject;
import java.time.Duration;
import java.time.Instant;
import java.util.stream.StreamSupport;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.threeten.bp.Duration;
import org.threeten.bp.Instant;

@RunWith(StorageITRunner.class)
@CrossRun(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.threeten.bp.Duration;

/**
* A {@link ManagedLifecycle} which integrates with the <a target="_blank"
Expand Down Expand Up @@ -260,10 +260,10 @@ public void start() {
runWithRetries(
TestBench.this::listRetryTests,
RetrySettings.newBuilder()
.setTotalTimeout(Duration.ofSeconds(30))
.setInitialRetryDelay(Duration.ofMillis(500))
.setTotalTimeoutDuration(Duration.ofSeconds(30))
.setInitialRetryDelayDuration(Duration.ofMillis(500))
.setRetryDelayMultiplier(1.5)
.setMaxRetryDelay(Duration.ofSeconds(5))
.setMaxRetryDelayDuration(Duration.ofSeconds(5))
.build(),
new BasicResultRetryAlgorithm<List<RetryTestResource>>() {
@Override
Expand Down Expand Up @@ -335,10 +335,10 @@ public void stop() {
throw new NotShutdownException();
},
RetrySettings.newBuilder()
.setTotalTimeout(Duration.ofSeconds(30))
.setInitialRetryDelay(Duration.ofMillis(500))
.setTotalTimeoutDuration(Duration.ofSeconds(30))
.setInitialRetryDelayDuration(Duration.ofMillis(500))
.setRetryDelayMultiplier(1.5)
.setMaxRetryDelay(Duration.ofSeconds(5))
.setMaxRetryDelayDuration(Duration.ofSeconds(5))
.build(),
new BasicResultRetryAlgorithm<List<?>>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.common.collect.ImmutableList;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand All @@ -43,7 +44,6 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import org.threeten.bp.Duration;

public class RemoteStorageHelperTest {

Expand Down Expand Up @@ -273,8 +273,8 @@ public void testCreateFromStream() {
assertEquals(60000, ((HttpTransportOptions) options.getTransportOptions()).getConnectTimeout());
assertEquals(60000, ((HttpTransportOptions) options.getTransportOptions()).getReadTimeout());
assertEquals(10, options.getRetrySettings().getMaxAttempts());
assertEquals(Duration.ofMillis(30000), options.getRetrySettings().getMaxRetryDelay());
assertEquals(Duration.ofMillis(120000), options.getRetrySettings().getTotalTimeout());
assertEquals(Duration.ofMillis(250), options.getRetrySettings().getInitialRetryDelay());
assertEquals(Duration.ofMillis(30000), options.getRetrySettings().getMaxRetryDelayDuration());
assertEquals(Duration.ofMillis(120000), options.getRetrySettings().getTotalTimeoutDuration());
assertEquals(Duration.ofMillis(250), options.getRetrySettings().getInitialRetryDelayDuration());
}
}

0 comments on commit baf30ee

Please sign in to comment.