Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

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

Merged
merged 4 commits into from Mar 12, 2020
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e10e4c0
fix: Timestamp.of(java.sql.Timestamp) pre-epoch on exact second
olavloite Mar 12, 2020
9bdd7f1
fix: review comments
olavloite Mar 12, 2020
7fb96d8
fix: review comments
olavloite Mar 12, 2020
b9e2161
fix: make all testOf method consistent
olavloite Mar 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions google-cloud-core/src/main/java/com/google/cloud/Timestamp.java
View file
Open in desktop
Expand Up @@ -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 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
olavloite marked this conversation as resolved.
Show resolved Hide resolved
// -2. This causes any pre-epoch timestamp to be off by 1 second - fix this by adjusting the
olavloite marked this conversation as resolved.
Show resolved Hide resolved
// seconds value by 1 if the timestamp < 0.
// seconds value by 1 if the timestamp < 0 and the division by 1000 has a remainder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be technical, a non-zero remainder. Suggest rephrasing as "the timestamp is less than zero and is not divisible by 1000."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to 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;
}

Expand Down
16 changes: 16 additions & 0 deletions google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java
View file
Open in desktop
Expand Up @@ -88,6 +88,14 @@ public void ofSqlTimestamp() {
assertThat(timestamp.toString()).isEqualTo(expectedTimestampString);
}

@Test
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);
olavloite marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void ofSqlTimestampPreEpoch() {
String expectedTimestampString = "1969-12-31T23:59:47.655000000Z";
Expand All @@ -104,6 +112,14 @@ public void ofSqlTimestampOnEpoch() {
assertThat(timestamp.toString()).isEqualTo(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);
assertThat(timestamp.toString()).isEqualTo(expectedTimestampString);
olavloite marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void ofDatePreEpoch() {
Timestamp timestamp = Timestamp.of(TEST_DATE_PRE_EPOCH);
Expand Down