From f202c3b550936168b9876860853876aa6d51c6a1 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Thu, 14 Nov 2024 14:18:33 -0500 Subject: [PATCH] feat: introduce `java.time` to java-core (#3330) ## In this PR: * Modify publicly exposed methods to offer a `java.time` alternative (suffixed with `Duration`). These methods will now hold the actual implementation, whereas the old signatures will encapsulate the new ones. Example: `retryDelay(org.threeten.bp.Duration)` encapsulates `retryDelayDuration(java.time.Duration)` ## Notes ### _CLIRR_ ``` [ERROR] 7012: com.google.cloud.testing.BaseEmulatorHelper$EmulatorRunner: Method 'public int waitForDuration(java.time.Duration)' has been added to an interface ``` This new interface method was added. However, we ignore this change alert because the method has a `default` implementation. ### Addressing a behavior change in `Timestamp` #### Problem When using java.time functions, parsing a datetime string with offset produces a wrong time object (offset is not added to the effective epoch seconds). #### Context Full context in https://github.com/googleapis/sdk-platform-java/pull/3330/files#r1828424787 https://github.com/adoptium/jdk/commit/c6d209b505932b43d58502b119b86a6fa5b5338e was introduced as a fix (in Java 9) meant for [this issue](https://bugs.openjdk.org/browse/JDK-8066982). In java 8, this means that the offset value is stored separately in a variable that is not respected by the parsing functions before this fix. The workaround is to use `appendZoneOrOffsetId()`, which stores the offset value in the `zone` variable of a parsing context, which is [respected as of java 8](https://github.com/adoptium/jdk8u/blob/31b88042fba46e87fba83e8bfd43ae0ecb5a9afd/jdk/src/share/classes/java/time/format/Parsed.java#L589-L591). Additionally, under the consideration of this having unwanted side effects, we expanded the test suite to test for more edge and normal cases using an offset string. We also [searched](https://bugs.openjdk.org/browse/JDK-8202948?jql=affectedVersion%20%3D%20%228%22%20AND%20text%20~%20%22offset%22) the JDK's issue tracking database and found somewhat similar issues with parsing and offsets but no workaround that addressed our specific situation. This is also cause of no backports of the [fix](https://github.com/adoptium/jdk/commit/c6d209b505932b43d58502b119b86a6fa5b5338e) for Java 9 into Java 8. #### Outcome The behavior is kept the same as stated by our tests and downstream checks --- .../clirr-ignored-differences.xml | 6 ++ .../java/com/google/cloud/RetryOption.java | 39 +++++++--- .../java/com/google/cloud/ServiceOptions.java | 12 ++-- .../main/java/com/google/cloud/Timestamp.java | 30 +++++--- .../cloud/testing/BaseEmulatorHelper.java | 58 ++++++++++++--- .../com/google/cloud/RetryOptionTest.java | 45 ++++++++---- .../com/google/cloud/SerializationTest.java | 4 +- .../java/com/google/cloud/TimestampTest.java | 72 +++++++++++++++++-- .../cloud/testing/BaseEmulatorHelperTest.java | 40 +++++++++-- 9 files changed, 247 insertions(+), 59 deletions(-) diff --git a/java-core/google-cloud-core/clirr-ignored-differences.xml b/java-core/google-cloud-core/clirr-ignored-differences.xml index 0f7f80a7b4..6792bcb968 100644 --- a/java-core/google-cloud-core/clirr-ignored-differences.xml +++ b/java-core/google-cloud-core/clirr-ignored-differences.xml @@ -12,4 +12,10 @@ com/google/cloud/ReadChannel long limit() + + + 7012 + com/google/cloud/testing/BaseEmulatorHelper$EmulatorRunner + int waitForDuration(java.time.Duration) + diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/RetryOption.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/RetryOption.java index a1069b48a2..f69b0d1c76 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/RetryOption.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/RetryOption.java @@ -16,12 +16,13 @@ package com.google.cloud; +import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration; import static com.google.common.base.Preconditions.checkNotNull; import com.google.api.core.BetaApi; +import com.google.api.core.ObsoleteApi; import com.google.api.gax.retrying.RetrySettings; import java.io.Serializable; -import org.threeten.bp.Duration; /** * This class represents an options wrapper around the {@link RetrySettings} class and is an @@ -51,13 +52,25 @@ private RetryOption(OptionType type, Object value) { this.value = checkNotNull(value); } - /** See {@link RetrySettings#getTotalTimeout()}. */ - public static RetryOption totalTimeout(Duration totalTimeout) { + /** This method is obsolete. Use {@link #totalTimeoutDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use totalTimeouDuration() instead") + public static RetryOption totalTimeout(org.threeten.bp.Duration totalTimeout) { + return totalTimeoutDuration(toJavaTimeDuration(totalTimeout)); + } + + /** See {@link RetrySettings#getTotalTimeoutDuration()}. */ + public static RetryOption totalTimeoutDuration(java.time.Duration totalTimeout) { return new RetryOption(OptionType.TOTAL_TIMEOUT, totalTimeout); } - /** See {@link RetrySettings#getInitialRetryDelay()}. */ - public static RetryOption initialRetryDelay(Duration initialRetryDelay) { + /** This method is obsolete. Use {@link #initialRetryDelayDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use initialRetryDelayDuration() instead") + public static RetryOption initialRetryDelay(org.threeten.bp.Duration initialRetryDelay) { + return initialRetryDelayDuration(toJavaTimeDuration(initialRetryDelay)); + } + + /** See {@link RetrySettings#getInitialRetryDelayDuration()}. */ + public static RetryOption initialRetryDelayDuration(java.time.Duration initialRetryDelay) { return new RetryOption(OptionType.INITIAL_RETRY_DELAY, initialRetryDelay); } @@ -66,8 +79,14 @@ public static RetryOption retryDelayMultiplier(double retryDelayMultiplier) { return new RetryOption(OptionType.RETRY_DELAY_MULTIPLIER, retryDelayMultiplier); } - /** See {@link RetrySettings#getMaxRetryDelay()}. */ - public static RetryOption maxRetryDelay(Duration maxRetryDelay) { + /** This method is obsolete. Use {@link #maxRetryDelayDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use maxRetryDelayDuration() instead") + public static RetryOption maxRetryDelay(org.threeten.bp.Duration maxRetryDelay) { + return maxRetryDelayDuration(toJavaTimeDuration(maxRetryDelay)); + } + + /** See {@link RetrySettings#getMaxRetryDelayDuration()}. */ + public static RetryOption maxRetryDelayDuration(java.time.Duration maxRetryDelay) { return new RetryOption(OptionType.MAX_RETRY_DELAY, maxRetryDelay); } @@ -124,16 +143,16 @@ public static RetrySettings mergeToSettings(RetrySettings settings, RetryOption. for (RetryOption option : options) { switch (option.type) { case TOTAL_TIMEOUT: - builder.setTotalTimeout((Duration) option.value); + builder.setTotalTimeoutDuration((java.time.Duration) option.value); break; case INITIAL_RETRY_DELAY: - builder.setInitialRetryDelay((Duration) option.value); + builder.setInitialRetryDelayDuration((java.time.Duration) option.value); break; case RETRY_DELAY_MULTIPLIER: builder.setRetryDelayMultiplier((Double) option.value); break; case MAX_RETRY_DELAY: - builder.setMaxRetryDelay((Duration) option.value); + builder.setMaxRetryDelayDuration((java.time.Duration) option.value); break; case MAX_ATTEMPTS: builder.setMaxAttempts((Integer) option.value); diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index 985fac4804..92aaa9d6a9 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -63,6 +63,7 @@ import java.io.Serializable; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -70,7 +71,6 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.threeten.bp.Duration; /** * Abstract class representing service options. @@ -787,13 +787,13 @@ public static RetrySettings getNoRetrySettings() { private static RetrySettings.Builder getDefaultRetrySettingsBuilder() { return RetrySettings.newBuilder() .setMaxAttempts(6) - .setInitialRetryDelay(Duration.ofMillis(1000L)) - .setMaxRetryDelay(Duration.ofMillis(32_000L)) + .setInitialRetryDelayDuration(Duration.ofMillis(1000L)) + .setMaxRetryDelayDuration(Duration.ofMillis(32_000L)) .setRetryDelayMultiplier(2.0) - .setTotalTimeout(Duration.ofMillis(50_000L)) - .setInitialRpcTimeout(Duration.ofMillis(50_000L)) + .setTotalTimeoutDuration(Duration.ofMillis(50_000L)) + .setInitialRpcTimeoutDuration(Duration.ofMillis(50_000L)) .setRpcTimeoutMultiplier(1.0) - .setMaxRpcTimeout(Duration.ofMillis(50_000L)); + .setMaxRpcTimeoutDuration(Duration.ofMillis(50_000L)); } protected abstract Set getScopes(); diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index e0308c3836..d24cb2a37e 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -18,18 +18,19 @@ import static com.google.common.base.Preconditions.checkArgument; +import com.google.api.core.ObsoleteApi; import com.google.protobuf.util.Timestamps; import java.io.Serializable; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; +import java.time.temporal.TemporalAccessor; import java.util.Date; import java.util.Objects; import java.util.concurrent.TimeUnit; -import org.threeten.bp.Instant; -import org.threeten.bp.LocalDateTime; -import org.threeten.bp.ZoneOffset; -import org.threeten.bp.format.DateTimeFormatter; -import org.threeten.bp.format.DateTimeFormatterBuilder; -import org.threeten.bp.format.DateTimeParseException; -import org.threeten.bp.temporal.TemporalAccessor; /** * Represents a timestamp with nanosecond precision. Timestamps cover the range [0001-01-01, @@ -54,7 +55,7 @@ public final class Timestamp implements Comparable, Serializable { new DateTimeFormatterBuilder() .appendOptional(DateTimeFormatter.ISO_LOCAL_DATE_TIME) .optionalStart() - .appendOffsetId() + .appendZoneOrOffsetId() .optionalEnd() .toFormatter() .withZone(ZoneOffset.UTC); @@ -189,6 +190,17 @@ public com.google.protobuf.Timestamp toProto() { return com.google.protobuf.Timestamp.newBuilder().setSeconds(seconds).setNanos(nanos).build(); } + /** This method is obsolete. Use {@link #parseTimestampDuration(String)} instead */ + @ObsoleteApi("Use parseTimestampDuration(String) instead") + public static Timestamp parseTimestamp(String timestamp) { + try { + return parseTimestampDuration(timestamp); + } catch (DateTimeParseException ex) { + throw new org.threeten.bp.format.DateTimeParseException( + ex.getMessage(), ex.getParsedString(), ex.getErrorIndex()); + } + } + /** * Creates a Timestamp instance from the given string. Input string should be in the RFC 3339 * format, like '2020-12-01T10:15:30.000Z' or with the timezone offset, such as @@ -198,7 +210,7 @@ public com.google.protobuf.Timestamp toProto() { * @return created Timestamp * @throws DateTimeParseException if unable to parse */ - public static Timestamp parseTimestamp(String timestamp) { + public static Timestamp parseTimestampDuration(String timestamp) { TemporalAccessor temporalAccessor = timestampParser.parse(timestamp); Instant instant = Instant.from(temporalAccessor); return ofTimeSecondsAndNanos(instant.getEpochSecond(), instant.getNano()); diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java index 9679c6299c..93f7ea0f59 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java @@ -16,8 +16,12 @@ package com.google.cloud.testing; +import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration; +import static com.google.api.gax.util.TimeConversionUtils.toThreetenDuration; + import com.google.api.core.CurrentMillisClock; import com.google.api.core.InternalApi; +import com.google.api.core.ObsoleteApi; import com.google.cloud.ExceptionHandler; import com.google.cloud.RetryHelper; import com.google.cloud.ServiceOptions; @@ -56,7 +60,6 @@ import java.util.logging.Logger; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; -import org.threeten.bp.Duration; /** Utility class to start and stop a local service which is used by unit testing. */ @InternalApi @@ -112,14 +115,21 @@ protected final void startProcess(String blockUntilOutput) } } + /** This method is obsolete. Use {@link #waitForProcessDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use waitForProcessDuration(java.time.Duration) instead") + protected final int waitForProcess(org.threeten.bp.Duration timeout) + throws IOException, InterruptedException, TimeoutException { + return waitForProcessDuration(toJavaTimeDuration(timeout)); + } + /** * Waits for the local service's subprocess to terminate, and stop any possible thread listening * for its output. */ - protected final int waitForProcess(Duration timeout) + protected final int waitForProcessDuration(java.time.Duration timeout) throws IOException, InterruptedException, TimeoutException { if (activeRunner != null) { - int exitCode = activeRunner.waitFor(timeout); + int exitCode = activeRunner.waitForDuration(timeout); activeRunner = null; return exitCode; } @@ -130,7 +140,7 @@ protected final int waitForProcess(Duration timeout) return 0; } - private static int waitForProcess(final Process process, Duration timeout) + private static int waitForProcess(final Process process, java.time.Duration timeout) throws InterruptedException, TimeoutException { if (process == null) { return 0; @@ -180,10 +190,17 @@ public String getProjectId() { /** Starts the local emulator. */ public abstract void start() throws IOException, InterruptedException; - /** Stops the local emulator. */ - public abstract void stop(Duration timeout) + /** This method is obsolete. Use {@link #stopDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use stopDuration() instead") + public abstract void stop(org.threeten.bp.Duration timeout) throws IOException, InterruptedException, TimeoutException; + /** Stops the local emulator. */ + public void stopDuration(java.time.Duration timeout) + throws IOException, InterruptedException, TimeoutException { + stop(toThreetenDuration(timeout)); + } + /** Resets the internal state of the emulator. */ public abstract void reset() throws IOException; @@ -226,8 +243,15 @@ protected interface EmulatorRunner { /** Starts the emulator associated to this runner. */ void start() throws IOException; + /** This method is obsolete. Use {@link #waitForDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use waitForDuration() instead") + int waitFor(org.threeten.bp.Duration timeout) throws InterruptedException, TimeoutException; + /** Wait for the emulator associated to this runner to terminate, returning the exit status. */ - int waitFor(Duration timeout) throws InterruptedException, TimeoutException; + default int waitForDuration(java.time.Duration timeout) + throws InterruptedException, TimeoutException { + return waitFor(toThreetenDuration(timeout)); + } /** Returns the process associated to the emulator, if any. */ Process getProcess(); @@ -263,9 +287,17 @@ public void start() throws IOException { log.fine("Starting emulator via Google Cloud SDK"); process = CommandWrapper.create().setCommand(commandText).setRedirectErrorStream().start(); } + /** This method is obsolete. Use {@link #waitForDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use waitForDuration() instead") + @Override + public int waitFor(org.threeten.bp.Duration timeout) + throws InterruptedException, TimeoutException { + return waitForDuration(toJavaTimeDuration(timeout)); + } @Override - public int waitFor(Duration timeout) throws InterruptedException, TimeoutException { + public int waitForDuration(java.time.Duration timeout) + throws InterruptedException, TimeoutException { return waitForProcess(process, timeout); } @@ -374,8 +406,16 @@ public Path call() throws IOException { .start(); } + /** This method is obsolete. Use {@link #waitForDuration(java.time.Duration)} instead */ + @ObsoleteApi("Use waitForDuration() instead") @Override - public int waitFor(Duration timeout) throws InterruptedException, TimeoutException { + public int waitFor(org.threeten.bp.Duration timeout) + throws InterruptedException, TimeoutException { + return waitForDuration(toJavaTimeDuration(timeout)); + } + + public int waitForDuration(java.time.Duration timeout) + throws InterruptedException, TimeoutException { return waitForProcess(process, timeout); } diff --git a/java-core/google-cloud-core/src/test/java/com/google/cloud/RetryOptionTest.java b/java-core/google-cloud-core/src/test/java/com/google/cloud/RetryOptionTest.java index a458d31f67..192cc21f5f 100644 --- a/java-core/google-cloud-core/src/test/java/com/google/cloud/RetryOptionTest.java +++ b/java-core/google-cloud-core/src/test/java/com/google/cloud/RetryOptionTest.java @@ -20,27 +20,30 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import com.google.api.gax.retrying.RetrySettings; +import java.time.Duration; import org.junit.jupiter.api.Test; -import org.threeten.bp.Duration; class RetryOptionTest { + private static final long TOTAL_TIMEOUT_MILLIS = 420l; + private static final long INITIAL_RETRY_DELAY_MILLIS = 42l; + private static final long MAX_RETRY_DELAY_MILLIS = 100l; private static final RetryOption TOTAL_TIMEOUT = - RetryOption.totalTimeout(Duration.ofMillis(420L)); + RetryOption.totalTimeoutDuration(Duration.ofMillis(TOTAL_TIMEOUT_MILLIS)); private static final RetryOption INITIAL_RETRY_DELAY = - RetryOption.initialRetryDelay(Duration.ofMillis(42L)); + RetryOption.initialRetryDelayDuration(Duration.ofMillis(INITIAL_RETRY_DELAY_MILLIS)); private static final RetryOption RETRY_DELAY_MULTIPLIER = RetryOption.retryDelayMultiplier(1.5); private static final RetryOption MAX_RETRY_DELAY = - RetryOption.maxRetryDelay(Duration.ofMillis(100)); + RetryOption.maxRetryDelayDuration(Duration.ofMillis(MAX_RETRY_DELAY_MILLIS)); private static final RetryOption MAX_ATTEMPTS = RetryOption.maxAttempts(100); private static final RetryOption JITTERED = RetryOption.jittered(false); private static final RetrySettings retrySettings = RetrySettings.newBuilder() - .setTotalTimeout(Duration.ofMillis(420L)) - .setInitialRetryDelay(Duration.ofMillis(42L)) + .setTotalTimeoutDuration(Duration.ofMillis(420L)) + .setInitialRetryDelayDuration(Duration.ofMillis(42L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(100)) + .setMaxRetryDelayDuration(Duration.ofMillis(100)) .setMaxAttempts(100) .setJittered(false) .build(); @@ -61,10 +64,10 @@ void testEqualsAndHashCode() { assertNotEquals(MAX_ATTEMPTS, MAX_RETRY_DELAY); assertNotEquals(JITTERED, MAX_ATTEMPTS); - RetryOption totalTimeout = RetryOption.totalTimeout(Duration.ofMillis(420L)); - RetryOption initialRetryDelay = RetryOption.initialRetryDelay(Duration.ofMillis(42L)); + RetryOption totalTimeout = RetryOption.totalTimeoutDuration(Duration.ofMillis(420L)); + RetryOption initialRetryDelay = RetryOption.initialRetryDelayDuration(Duration.ofMillis(42L)); RetryOption retryDelayMultiplier = RetryOption.retryDelayMultiplier(1.5); - RetryOption maxRetryDelay = RetryOption.maxRetryDelay(Duration.ofMillis(100)); + RetryOption maxRetryDelay = RetryOption.maxRetryDelayDuration(Duration.ofMillis(100)); RetryOption maxAttempts = RetryOption.maxAttempts(100); RetryOption jittered = RetryOption.jittered(false); @@ -101,17 +104,17 @@ void testMergeToSettings() { assertEquals(retrySettings, mergedRetrySettings); defRetrySettings = - defRetrySettings.toBuilder().setTotalTimeout(Duration.ofMillis(420L)).build(); + defRetrySettings.toBuilder().setTotalTimeoutDuration(Duration.ofMillis(420L)).build(); mergedRetrySettings = RetryOption.mergeToSettings(defRetrySettings, TOTAL_TIMEOUT); assertEquals(defRetrySettings, mergedRetrySettings); defRetrySettings = - defRetrySettings.toBuilder().setMaxRetryDelay(Duration.ofMillis(100)).build(); + defRetrySettings.toBuilder().setMaxRetryDelayDuration(Duration.ofMillis(100)).build(); mergedRetrySettings = RetryOption.mergeToSettings(defRetrySettings, MAX_RETRY_DELAY); assertEquals(defRetrySettings, mergedRetrySettings); defRetrySettings = - defRetrySettings.toBuilder().setInitialRetryDelay(Duration.ofMillis(42L)).build(); + defRetrySettings.toBuilder().setInitialRetryDelayDuration(Duration.ofMillis(42L)).build(); mergedRetrySettings = RetryOption.mergeToSettings(defRetrySettings, INITIAL_RETRY_DELAY); assertEquals(defRetrySettings, mergedRetrySettings); @@ -127,4 +130,20 @@ void testMergeToSettings() { mergedRetrySettings = RetryOption.mergeToSettings(defRetrySettings, JITTERED); assertEquals(defRetrySettings, mergedRetrySettings); } + + @Test + public void threetenMethods_producesEquivalentJavaTimeRetryOptions() { + + final RetryOption totalTimeoutThreeten = + RetryOption.totalTimeout(org.threeten.bp.Duration.ofMillis(TOTAL_TIMEOUT_MILLIS)); + final RetryOption initialRetryDelayThreeten = + RetryOption.initialRetryDelay( + org.threeten.bp.Duration.ofMillis(INITIAL_RETRY_DELAY_MILLIS)); + final RetryOption maxRetryDelayThreeten = + RetryOption.maxRetryDelay(org.threeten.bp.Duration.ofMillis(MAX_RETRY_DELAY_MILLIS)); + + assertEquals(TOTAL_TIMEOUT, totalTimeoutThreeten); + assertEquals(INITIAL_RETRY_DELAY, initialRetryDelayThreeten); + assertEquals(MAX_RETRY_DELAY, maxRetryDelayThreeten); + } } diff --git a/java-core/google-cloud-core/src/test/java/com/google/cloud/SerializationTest.java b/java-core/google-cloud-core/src/test/java/com/google/cloud/SerializationTest.java index 6c35c665b5..f591578f11 100644 --- a/java-core/google-cloud-core/src/test/java/com/google/cloud/SerializationTest.java +++ b/java-core/google-cloud-core/src/test/java/com/google/cloud/SerializationTest.java @@ -23,7 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.io.Serializable; -import org.threeten.bp.Duration; +import java.time.Duration; public class SerializationTest extends BaseSerializationTest { @@ -37,7 +37,7 @@ public class SerializationTest extends BaseSerializationTest { private static final Role SOME_ROLE = Role.viewer(); private static final Policy SOME_IAM_POLICY = Policy.newBuilder().build(); private static final RetryOption CHECKING_PERIOD = - RetryOption.initialRetryDelay(Duration.ofSeconds(42)); + RetryOption.initialRetryDelayDuration(Duration.ofSeconds(42)); private static final LabelDescriptor LABEL_DESCRIPTOR = new LabelDescriptor("project_id", ValueType.STRING, "The project id"); private static final MonitoredResourceDescriptor MONITORED_RESOURCE_DESCRIPTOR = diff --git a/java-core/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/java-core/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index ba2ad5b701..6d6340b417 100644 --- a/java-core/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/java-core/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.testing.EqualsTester; +import java.time.format.DateTimeParseException; import java.util.Calendar; import java.util.Date; import java.util.GregorianCalendar; @@ -237,14 +238,67 @@ void parseTimestampWithoutTimeZoneOffset() { @Test void parseTimestampWithTimeZoneOffset() { - assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00-00:00")) + // Max values + assertThat(Timestamp.parseTimestampDuration("0001-01-01T00:00:00-00:00")) .isEqualTo(Timestamp.MIN_VALUE); - assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999-00:00")) + assertThat(Timestamp.parseTimestampDuration("9999-12-31T23:59:59.999999999-00:00")) .isEqualTo(Timestamp.MAX_VALUE); - assertThat(Timestamp.parseTimestamp("2020-12-06T19:21:12.123+05:30")) + // Extreme values (close to min/max) + assertThat(Timestamp.parseTimestampDuration("0001-01-01T00:00:00.000000001Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(Timestamp.MIN_VALUE.getSeconds(), 1)); + assertThat(Timestamp.parseTimestampDuration("9999-12-31T23:59:59.999999998Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(Timestamp.MAX_VALUE.getSeconds(), 999999998)); + // Common use cases + assertThat(Timestamp.parseTimestampDuration("2020-07-10T14:03:00.123-07:00")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1594414980, 123000000)); + assertThat(Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123+05:30")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1607262672, 123000000)); - assertThat(Timestamp.parseTimestamp("2020-07-10T14:03:00-07:00")) + // We also confirm that parsing a timestamp with nano precision will behave the same as the + // threeten counterpart + assertThat(Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123+05:30")) + .isEqualTo(Timestamp.parseTimestamp("2020-12-06T19:21:12.123+05:30")); + // Timestamps with fractional seconds at nanosecond level + assertThat(Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123456789+05:30")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1607262672, 123456789)); + // Fractional seconds beyond nanos should throw an exception + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123456789321+05:30")); + // Missing components (should throw exceptions) + assertThrows( + DateTimeParseException.class, () -> Timestamp.parseTimestampDuration("2020-12-06")); + // Whitespace should not be supported + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration(" 2020-12-06T19:21:12.123+05:30 ")); + // It should be case-insensitive + assertThat(Timestamp.parseTimestampDuration("2020-07-10t14:03:00-07:00")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1594414980, 0)); + // Invalid time zone offsets + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123+25:00")); + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123-25:00")); + // Int values for SecondOfMinute should be between 0 and 59 + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("2016-12-31T23:59:60Z")); + } + + @Test + void parseTimestampWithZoneString() { + // Valid RFC 3339 timestamps with time zone names + assertThat(Timestamp.parseTimestampDuration("2020-12-06T08:51:12.123America/Toronto")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1607262672, 123000000)); + assertThat(Timestamp.parseTimestampDuration("2023-04-10T22:42:10.123456789Europe/London")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(1681162930, 123456789)); + + // Invalid time zone names + assertThrows( + DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("2020-12-06T19:21:12.123Invalid/TimeZone")); } @Test @@ -281,4 +335,14 @@ void comparable() { void serialization() { reserializeAndAssert(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999Z")); } + + @Test + void parseInvalidTimestampThreetenThrowsThreetenException() { + assertThrows( + org.threeten.bp.format.DateTimeParseException.class, + () -> Timestamp.parseTimestamp("00x1-01-01T00:00:00")); + assertThrows( + java.time.format.DateTimeParseException.class, + () -> Timestamp.parseTimestampDuration("00x1-01-01T00:00:00")); + } } diff --git a/java-core/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java b/java-core/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java index 79b58a83c9..bfc2666216 100644 --- a/java-core/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java +++ b/java-core/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java @@ -28,12 +28,12 @@ import java.net.URL; import java.net.URLConnection; import java.net.URLStreamHandler; +import java.time.Duration; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; import org.easymock.EasyMock; import org.junit.jupiter.api.Test; -import org.threeten.bp.Duration; class BaseEmulatorHelperTest { @@ -71,10 +71,18 @@ public void start() throws IOException, InterruptedException { } @Override - public void stop(Duration timeout) throws IOException, InterruptedException, TimeoutException { + public void stop(org.threeten.bp.Duration timeout) + throws IOException, InterruptedException, TimeoutException { + // we call the threeten method directly to confirm behavior waitForProcess(timeout); } + @Override + public void stopDuration(Duration timeout) + throws IOException, InterruptedException, TimeoutException { + super.stopDuration(timeout); + } + @Override public void reset() throws IOException { // do nothing @@ -91,13 +99,33 @@ void testEmulatorHelper() throws IOException, InterruptedException, TimeoutExcep emulatorRunner.start(); EasyMock.expectLastCall(); EasyMock.expect(emulatorRunner.getProcess()).andReturn(process); - emulatorRunner.waitFor(Duration.ofMinutes(1)); + emulatorRunner.waitForDuration(Duration.ofMinutes(1)); + EasyMock.expectLastCall().andReturn(0); + EasyMock.replay(process, emulatorRunner); + TestEmulatorHelper helper = + new TestEmulatorHelper(ImmutableList.of(emulatorRunner), BLOCK_UNTIL); + helper.start(); + helper.stopDuration(Duration.ofMinutes(1)); + EasyMock.verify(); + } + + @Test + void testEmulatorHelperThreeten() throws IOException, InterruptedException, TimeoutException { + Process process = EasyMock.createStrictMock(Process.class); + InputStream stream = new ByteArrayInputStream(BLOCK_UNTIL.getBytes(Charsets.UTF_8)); + EmulatorRunner emulatorRunner = EasyMock.createStrictMock(EmulatorRunner.class); + EasyMock.expect(process.getInputStream()).andReturn(stream); + EasyMock.expect(emulatorRunner.isAvailable()).andReturn(true); + emulatorRunner.start(); + EasyMock.expectLastCall(); + EasyMock.expect(emulatorRunner.getProcess()).andReturn(process); + emulatorRunner.waitForDuration(java.time.Duration.ofMinutes(1)); EasyMock.expectLastCall().andReturn(0); EasyMock.replay(process, emulatorRunner); TestEmulatorHelper helper = new TestEmulatorHelper(ImmutableList.of(emulatorRunner), BLOCK_UNTIL); helper.start(); - helper.stop(Duration.ofMinutes(1)); + helper.stop(org.threeten.bp.Duration.ofMinutes(1)); EasyMock.verify(); } @@ -157,13 +185,13 @@ void testEmulatorHelperMultipleRunners() secondRunner.start(); EasyMock.expectLastCall(); EasyMock.expect(secondRunner.getProcess()).andReturn(process); - secondRunner.waitFor(Duration.ofMinutes(1)); + secondRunner.waitForDuration(Duration.ofMinutes(1)); EasyMock.expectLastCall().andReturn(0); EasyMock.replay(process, secondRunner); TestEmulatorHelper helper = new TestEmulatorHelper(ImmutableList.of(firstRunner, secondRunner), BLOCK_UNTIL); helper.start(); - helper.stop(Duration.ofMinutes(1)); + helper.stopDuration(Duration.ofMinutes(1)); EasyMock.verify(); }