From e10e4c03e3d124a97f627853300914ddd19aa775 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 12 Mar 2020 08:35:29 +0100 Subject: [PATCH 1/4] fix: Timestamp.of(java.sql.Timestamp) pre-epoch on exact second --- .../main/java/com/google/cloud/Timestamp.java | 6 +++--- .../java/com/google/cloud/TimestampTest.java | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index df111d3503..94e7f4da68 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -121,13 +121,13 @@ public static Timestamp now() { public static Timestamp of(java.sql.Timestamp timestamp) { int nanos = timestamp.getNanos(); - // A pre-epoch timestamp will be off by one second because of the way that integer division + // A pre-epoch timestamp could be off by one second because of the way that integer division // works. For example, -1001 / 1000 == -1. In this case of timestamps, we want this result to be // -2. This causes any pre-epoch timestamp to be off by 1 second - fix this by adjusting the - // seconds value by 1 if the timestamp < 0. + // seconds value by 1 if the timestamp < 0 and the division by 1000 has a remainder. // TODO: replace with Math.floorDiv when we drop Java 7 support long seconds = timestamp.getTime() / 1000; - if (seconds < 0) { + if (seconds < 0 && timestamp.getTime() % 1000 != 0) { --seconds; } diff --git a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 12e13ef5b9..078ef5bcf2 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -88,6 +88,14 @@ public void ofSqlTimestamp() { assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); } + @Test + public void ofSqlTimestampOnExactSecond() { + String expectedTimestampString = "1970-01-01T00:00:12Z"; + java.sql.Timestamp input = new java.sql.Timestamp(12000); + Timestamp timestamp = Timestamp.of(input); + assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + } + @Test public void ofSqlTimestampPreEpoch() { String expectedTimestampString = "1969-12-31T23:59:47.655000000Z"; @@ -104,6 +112,14 @@ public void ofSqlTimestampOnEpoch() { assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); } + @Test + public void ofSqlTimestampPreEpochOnExactSecond() { + String expectedTimestampString = "1969-12-31T23:59:59Z"; + java.sql.Timestamp input = new java.sql.Timestamp(-1000); + Timestamp timestamp = Timestamp.of(input); + assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + } + @Test public void ofDatePreEpoch() { Timestamp timestamp = Timestamp.of(TEST_DATE_PRE_EPOCH); From 9bdd7f1d968c7a65057a5b5b82fb064daad25c8f Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 12 Mar 2020 13:35:38 +0100 Subject: [PATCH 2/4] fix: review comments --- .../src/main/java/com/google/cloud/Timestamp.java | 2 +- .../src/test/java/com/google/cloud/TimestampTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 94e7f4da68..6948800a21 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -121,7 +121,7 @@ public static Timestamp now() { public static Timestamp of(java.sql.Timestamp timestamp) { int nanos = timestamp.getNanos(); - // A pre-epoch timestamp could be off by one second because of the way that integer division + // A pre-epoch timestamp can be off by one second because of the way that integer division // works. For example, -1001 / 1000 == -1. In this case of timestamps, we want this result to be // -2. This causes any pre-epoch timestamp to be off by 1 second - fix this by adjusting the // seconds value by 1 if the timestamp < 0 and the division by 1000 has a remainder. diff --git a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 078ef5bcf2..b88ade1946 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -89,7 +89,7 @@ public void ofSqlTimestamp() { } @Test - public void ofSqlTimestampOnExactSecond() { + public void testOf_exactSecond() { String expectedTimestampString = "1970-01-01T00:00:12Z"; java.sql.Timestamp input = new java.sql.Timestamp(12000); Timestamp timestamp = Timestamp.of(input); @@ -113,7 +113,7 @@ public void ofSqlTimestampOnEpoch() { } @Test - public void ofSqlTimestampPreEpochOnExactSecond() { + public void testOf_preEpochExactSecond() { String expectedTimestampString = "1969-12-31T23:59:59Z"; java.sql.Timestamp input = new java.sql.Timestamp(-1000); Timestamp timestamp = Timestamp.of(input); From 7fb96d8c67d25bbdf8c3d3637ffce1a57d09a396 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 12 Mar 2020 17:10:21 +0100 Subject: [PATCH 3/4] fix: review comments --- .../src/main/java/com/google/cloud/Timestamp.java | 6 +++--- .../src/test/java/com/google/cloud/TimestampTest.java | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 6948800a21..c2cf20a3e2 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -122,9 +122,9 @@ public static Timestamp of(java.sql.Timestamp timestamp) { int nanos = timestamp.getNanos(); // A pre-epoch timestamp can be off by one second because of the way that integer division - // works. For example, -1001 / 1000 == -1. In this case of timestamps, we want this result to be - // -2. This causes any pre-epoch timestamp to be off by 1 second - fix this by adjusting the - // seconds value by 1 if the timestamp < 0 and the division by 1000 has a remainder. + // works. For example, -1001 / 1000 == -1. In this case, we want this result to be -2. This + // causes any pre-epoch timestamp to be off by 1 second - fix this by subtracting 1 from the + // seconds value if the seconds value is less than zero and is not divisible by 1000. // TODO: replace with Math.floorDiv when we drop Java 7 support long seconds = timestamp.getTime() / 1000; if (seconds < 0 && timestamp.getTime() % 1000 != 0) { diff --git a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index b88ade1946..9d673a3134 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -18,6 +18,7 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import com.google.common.testing.EqualsTester; @@ -93,7 +94,7 @@ public void testOf_exactSecond() { String expectedTimestampString = "1970-01-01T00:00:12Z"; java.sql.Timestamp input = new java.sql.Timestamp(12000); Timestamp timestamp = Timestamp.of(input); - assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + assertEquals(timestamp.toString(), expectedTimestampString); } @Test @@ -109,7 +110,7 @@ public void ofSqlTimestampOnEpoch() { String expectedTimestampString = "1970-01-01T00:00:00Z"; java.sql.Timestamp input = new java.sql.Timestamp(0); Timestamp timestamp = Timestamp.of(input); - assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + assertEquals(timestamp.toString(), expectedTimestampString); } @Test From b9e21613e0a87b67da9a10187794e1d63016233a Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 12 Mar 2020 17:49:29 +0100 Subject: [PATCH 4/4] fix: make all testOf method consistent --- .../test/java/com/google/cloud/TimestampTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 9d673a3134..26bf9f2f8d 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -82,11 +82,11 @@ public void ofDate() { } @Test - public void ofSqlTimestamp() { + public void testOf() { String expectedTimestampString = "1970-01-01T00:00:12.345000000Z"; java.sql.Timestamp input = new java.sql.Timestamp(12345); Timestamp timestamp = Timestamp.of(input); - assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + assertEquals(timestamp.toString(), expectedTimestampString); } @Test @@ -98,15 +98,15 @@ public void testOf_exactSecond() { } @Test - public void ofSqlTimestampPreEpoch() { + public void testOf_preEpoch() { String expectedTimestampString = "1969-12-31T23:59:47.655000000Z"; java.sql.Timestamp input = new java.sql.Timestamp(-12345); Timestamp timestamp = Timestamp.of(input); - assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + assertEquals(timestamp.toString(), expectedTimestampString); } @Test - public void ofSqlTimestampOnEpoch() { + public void testOf_onEpoch() { String expectedTimestampString = "1970-01-01T00:00:00Z"; java.sql.Timestamp input = new java.sql.Timestamp(0); Timestamp timestamp = Timestamp.of(input); @@ -118,7 +118,7 @@ public void testOf_preEpochExactSecond() { String expectedTimestampString = "1969-12-31T23:59:59Z"; java.sql.Timestamp input = new java.sql.Timestamp(-1000); Timestamp timestamp = Timestamp.of(input); - assertThat(timestamp.toString()).isEqualTo(expectedTimestampString); + assertEquals(timestamp.toString(), expectedTimestampString); } @Test