fix: fix Timestamp.of(java.sql.Timestamp) pre-epoch on exact second (#…

…179)

* fix: Timestamp.of(java.sql.Timestamp) pre-epoch on exact second

* fix: review comments

* fix: review comments

* fix: make all testOf method consistent

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
-    // 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.
+    // 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, 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) {
+    if (seconds < 0 && timestamp.getTime() % 1000 != 0) {
       --seconds;
     }
 

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;
@@ -81,27 +82,43 @@ 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
-  public void ofSqlTimestampPreEpoch() {
+  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);
+    assertEquals(timestamp.toString(), expectedTimestampString);
+  }
+
+  @Test
+  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);
-    assertThat(timestamp.toString()).isEqualTo(expectedTimestampString);
+    assertEquals(timestamp.toString(), expectedTimestampString);
+  }
+
+  @Test
+  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);
+    assertEquals(timestamp.toString(), expectedTimestampString);
   }
 
   @Test