From baf30ee91febbcda7d0f64b0083b789c4384a3c0 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 2 Dec 2024 17:03:14 -0500 Subject: [PATCH] feat: introduce `java.time` methods and variables (#2826) * feat: introduce `java.time` methods and variables * improve DurationDuration method name * adjust naming --- .../google/cloud/storage/GrpcStorageImpl.java | 3 +- .../cloud/storage/GrpcStorageOptions.java | 43 +++++++++++++------ .../storage/testing/RemoteStorageHelper.java | 12 +++--- .../cloud/storage/ITGapicReadTimeoutTest.java | 8 ++-- .../storage/it/ITBlobWriteChannelTest.java | 10 ++--- .../google/cloud/storage/it/ITHmacTest.java | 4 +- .../storage/it/runner/registry/TestBench.java | 14 +++--- .../testing/RemoteStorageHelperTest.java | 8 ++-- 8 files changed, 60 insertions(+), 42 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index d0003c7119..d7472f5ff9 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -189,8 +189,7 @@ final class GrpcStorageImpl extends BaseService 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); } } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 7a833fdeec..436be1e70d 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -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; @@ -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) @@ -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; @@ -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; @@ -145,7 +148,7 @@ GrpcRetryAlgorithmManager getRetryAlgorithmManager() { } @InternalApi - Duration getTerminationAwaitDuration() { + java.time.Duration getTerminationAwaitDuration() { return terminationAwaitDuration; } @@ -314,9 +317,9 @@ private Tuple> 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 startResumableWriteRetryableCodes = builder.startResumableWriteSettings().getRetryableCodes(); @@ -324,7 +327,7 @@ private Tuple> resolveSettingsAndOpts() throw // StartResumableWrite builder.applyToAllUnaryMethods( input -> { - input.setSimpleTimeoutNoRetries(totalTimeout); + input.setSimpleTimeoutNoRetriesDuration(totalTimeout); return null; }); @@ -342,7 +345,7 @@ private Tuple> 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); } @@ -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(); @@ -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()} @@ -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; @@ -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 */ diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java index 50bfdc8743..2d604a287c 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java @@ -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; @@ -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 @@ -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(); } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGapicReadTimeoutTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGapicReadTimeoutTest.java index 9cef9159c0..40cf755c97 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGapicReadTimeoutTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGapicReadTimeoutTest.java @@ -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 @@ -48,7 +48,7 @@ * #getTotalTimeout()}, gax will interrupt the stream with a DEADLINE_EXCEEDED error. * *

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)}. * *

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. @@ -117,7 +117,7 @@ public void readObject( .setRetrySettings( RetrySettings.newBuilder() .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(totalTimeoutMillis)) + .setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis)) .build()) .build() .getStorageSettings(); @@ -196,7 +196,7 @@ public void readObject( .setRetrySettings( RetrySettings.newBuilder() .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(totalTimeoutMillis)) + .setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis)) .build()) .build() .getStorageSettings(); diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java index 9695232b86..2a2dd9a665 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java @@ -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) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITHmacTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITHmacTest.java index 36239edc5a..191514570e 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITHmacTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITHmacTest.java @@ -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( diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java index 8a58a07212..d40a29954d 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java @@ -53,6 +53,7 @@ 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; @@ -60,7 +61,6 @@ 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 >() { @Override @@ -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>() { @Override diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/testing/RemoteStorageHelperTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/testing/RemoteStorageHelperTest.java index 508109e3e2..61e9574ed2 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/testing/RemoteStorageHelperTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/testing/RemoteStorageHelperTest.java @@ -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; @@ -43,7 +44,6 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; -import org.threeten.bp.Duration; public class RemoteStorageHelperTest { @@ -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()); } }