From 0a5b308cf35d8028da785f539ff2788ef61deb82 Mon Sep 17 00:00:00 2001 From: Hanzhen Yi Date: Mon, 9 Jul 2018 14:57:00 -0700 Subject: [PATCH 1/2] Fix the error when calling Timestamp.of(Date date) when date is pre epoch. --- .../src/main/java/com/google/cloud/Timestamp.java | 4 ++++ .../test/java/com/google/cloud/TimestampTest.java | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 0c2eca786367..7154dca9460e 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -81,6 +81,10 @@ public static Timestamp ofTimeMicroseconds(long microseconds) { long seconds = TimeUnit.MICROSECONDS.toSeconds(microseconds); int nanos = (int) TimeUnit.MICROSECONDS.toNanos(microseconds - TimeUnit.SECONDS.toMicros(seconds)); + if (nanos < 0) { + seconds--; + nanos += TimeUnit.SECONDS.toNanos(1); + } checkArgument( Timestamps.isValid(seconds, nanos), "timestamp out of range: %s, %s", seconds, nanos); return new Timestamp(seconds, nanos); diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 51d2ecb20f18..53235db4e547 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -39,7 +39,9 @@ public class TimestampTest { private static final long TEST_TIME_MICROSECONDS = 10000100L; private static final long TEST_TIME_MILLISECONDS = TimeUnit.SECONDS.toMillis(1444662894L) + TimeUnit.MICROSECONDS.toMillis(1234); + private static final long TEST_TIME_MILLISECONDS_NEGATIVE = -1L; private static final Date TEST_DATE = new Date(TEST_TIME_MILLISECONDS); + private static final Date TEST_DATE_PRE_EPOCH = new Date(TEST_TIME_MILLISECONDS_NEGATIVE); @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -80,6 +82,17 @@ public void ofDate() { assertThat(timestamp.getNanos()).isEqualTo(expectedNanos); } + @Test + public void ofDatePreEpoch() { + Timestamp timestamp = Timestamp.of(TEST_DATE_PRE_EPOCH); + Long expectedSeconds = TimeUnit.MILLISECONDS.toSeconds(TEST_TIME_MILLISECONDS_NEGATIVE) - 1; + Long expectedNanos = + TimeUnit.MILLISECONDS.toNanos(TEST_TIME_MILLISECONDS_NEGATIVE) + - TimeUnit.SECONDS.toNanos(expectedSeconds); + assertThat(timestamp.getSeconds()).isEqualTo(expectedSeconds); + assertThat(timestamp.getNanos()).isEqualTo(expectedNanos); + } + @Test public void toDate() { Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 1234 * 1000); From 963bef748becb6b54dc3cd0bd9b80e03372ef01e Mon Sep 17 00:00:00 2001 From: Hanzhen Yi Date: Tue, 10 Jul 2018 14:19:50 -0700 Subject: [PATCH 2/2] Fix code review issues --- .../src/main/java/com/google/cloud/Timestamp.java | 7 +++---- .../test/java/com/google/cloud/TimestampTest.java | 12 +++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 7154dca9460e..17e26d9c9993 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -78,12 +78,11 @@ public static Timestamp ofTimeSecondsAndNanos(long seconds, int nanos) { * @throws IllegalArgumentException if the timestamp is outside the representable range */ public static Timestamp ofTimeMicroseconds(long microseconds) { - long seconds = TimeUnit.MICROSECONDS.toSeconds(microseconds); - int nanos = - (int) TimeUnit.MICROSECONDS.toNanos(microseconds - TimeUnit.SECONDS.toMicros(seconds)); + long seconds = microseconds / 1_000_000; + int nanos = (int)(microseconds % 1_000_000 * 1000); if (nanos < 0) { seconds--; - nanos += TimeUnit.SECONDS.toNanos(1); + nanos += 1_000_000_000; } checkArgument( Timestamps.isValid(seconds, nanos), "timestamp out of range: %s, %s", seconds, nanos); diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 53235db4e547..c78f54627c77 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -39,7 +39,7 @@ public class TimestampTest { private static final long TEST_TIME_MICROSECONDS = 10000100L; private static final long TEST_TIME_MILLISECONDS = TimeUnit.SECONDS.toMillis(1444662894L) + TimeUnit.MICROSECONDS.toMillis(1234); - private static final long TEST_TIME_MILLISECONDS_NEGATIVE = -1L; + private static final long TEST_TIME_MILLISECONDS_NEGATIVE = -1000L; private static final Date TEST_DATE = new Date(TEST_TIME_MILLISECONDS); private static final Date TEST_DATE_PRE_EPOCH = new Date(TEST_TIME_MILLISECONDS_NEGATIVE); @@ -85,10 +85,12 @@ public void ofDate() { @Test public void ofDatePreEpoch() { Timestamp timestamp = Timestamp.of(TEST_DATE_PRE_EPOCH); - Long expectedSeconds = TimeUnit.MILLISECONDS.toSeconds(TEST_TIME_MILLISECONDS_NEGATIVE) - 1; - Long expectedNanos = - TimeUnit.MILLISECONDS.toNanos(TEST_TIME_MILLISECONDS_NEGATIVE) - - TimeUnit.SECONDS.toNanos(expectedSeconds); + long expectedSeconds = TEST_TIME_MILLISECONDS_NEGATIVE / 1_000; + int expectedNanos = (int)(TEST_TIME_MILLISECONDS_NEGATIVE % 1_000 * 1000_000); + if (expectedNanos < 0) { + expectedSeconds--; + expectedNanos += 1_000_000_000; + } assertThat(timestamp.getSeconds()).isEqualTo(expectedSeconds); assertThat(timestamp.getNanos()).isEqualTo(expectedNanos); }