From 1f8b6b4835aaa702ec94bbbde89ed90f519c935a Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Tue, 25 Feb 2020 10:26:31 -0800 Subject: [PATCH] fix: fix conversion for pre-epoch timestamps (#160) * test: failing test for parsing a negative timestamp * fix: fix conversion for pre-epoch timestamps * chore: add comment, rename test variable names --- .../main/java/com/google/cloud/Timestamp.java | 14 ++++++++++- .../java/com/google/cloud/TimestampTest.java | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) 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 618ef9c721..df111d3503 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 @@ -119,7 +119,19 @@ public static Timestamp now() { * @throws IllegalArgumentException if the timestamp is outside the representable range */ public static Timestamp of(java.sql.Timestamp timestamp) { - return ofTimeSecondsAndNanos(timestamp.getTime() / 1000, timestamp.getNanos()); + int nanos = timestamp.getNanos(); + + // A pre-epoch timestamp will 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. + // TODO: replace with Math.floorDiv when we drop Java 7 support + long seconds = timestamp.getTime() / 1000; + if (seconds < 0) { + --seconds; + } + + return Timestamp.ofTimeSecondsAndNanos(seconds, nanos); } /** 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 81ef7ce379..12e13ef5b9 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 @@ -80,6 +80,30 @@ public void ofDate() { assertThat(timestamp.getNanos()).isEqualTo(expectedNanos); } + @Test + public void ofSqlTimestamp() { + 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); + } + + @Test + public void ofSqlTimestampPreEpoch() { + 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); + } + + @Test + 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); + } + @Test public void ofDatePreEpoch() { Timestamp timestamp = Timestamp.of(TEST_DATE_PRE_EPOCH);