opendev/gerrit
Code Issues Proposed changes

Switch to Java Date/Time API for TimeUtil

Browse Source 
Instead of adding a mechanism to exchange the provider of the current
time in millis, we should rather use Instant.now(Clock).toEpochMilli()
in combination with Clock#fixed for tests. However, this would require
adjustments which are beyond the scope of this change.

Change-Id: I91e2de5d6cbda5235f54fc7ca8851534d8c85aa0
This commit is contained in:
Alice Kober-Sotzek
2017-10-01 13:42:48 +02:00
parent 1c50562df5
commit 376b1bd2f3
3 changed files with 35 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
gerrit-common/src/main/java/com/google/gerrit/common/TimeUtil.java
View File

@@ -15,14 +15,21 @@
package com.google.gerrit.common;
import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.VisibleForTesting;
import java.sql.Timestamp;
import org.joda.time.DateTimeUtils;
import java.util.function.LongSupplier;
/** Static utility methods for dealing with dates and times. */
@GwtIncompatible("Unemulated org.joda.time.DateTimeUtils")
@GwtIncompatible("Unemulated Java 8 functionalities")
public class TimeUtil {
private static final LongSupplier SYSTEM_CURRENT_MILLIS_SUPPLIER = System::currentTimeMillis;
private static volatile LongSupplier currentMillisSupplier = SYSTEM_CURRENT_MILLIS_SUPPLIER;
public static long nowMs() {
return DateTimeUtils.currentTimeMillis();
// We should rather use Instant.now(Clock).toEpochMilli() instead but this would require some
// changes in our testing code as we wouldn't have clock steps anymore.
return currentMillisSupplier.getAsLong();
}
public static Timestamp nowTs() {
@@ -33,5 +40,15 @@ public class TimeUtil {
return new Timestamp((t.getTime() / 1000) * 1000);
}
@VisibleForTesting
public static void setCurrentMillisSupplier(LongSupplier customCurrentMillisSupplier) {
currentMillisSupplier = customCurrentMillisSupplier;
}
@VisibleForTesting
public static void resetCurrentMillisSupplier() {
currentMillisSupplier = SYSTEM_CURRENT_MILLIS_SUPPLIER;
}
private TimeUtil() {}
}

6
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -1232,7 +1232,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
long startMs = TestTimeUtil.START.getMillis();
long startMs = TestTimeUtil.START.toEpochMilli();
Change change1 = insert(repo, newChange(repo), null, new Timestamp(startMs));
Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
@@ -1259,7 +1259,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
long startMs = TestTimeUtil.START.getMillis();
long startMs = TestTimeUtil.START.toEpochMilli();
Change change1 = insert(repo, newChange(repo), null, new Timestamp(startMs));
Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);
@@ -1281,7 +1281,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
long startMs = TestTimeUtil.START.getMillis();
long startMs = TestTimeUtil.START.toEpochMilli();
Change change1 = insert(repo, newChange(repo), null, new Timestamp(startMs));
Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);

27
gerrit-server/src/test/java/com/google/gerrit/testutil/TestTimeUtil.java
View File

@@ -17,18 +17,21 @@ package com.google.gerrit.testutil;
import static com.google.common.base.Preconditions.checkState;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.gerrit.common.TimeUtil;
import java.sql.Timestamp;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.Month;
import java.time.ZoneOffset;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import org.joda.time.DateTime;
import org.joda.time.DateTimeUtils;
import org.joda.time.DateTimeUtils.MillisProvider;
import org.joda.time.DateTimeZone;
/** Static utility methods for dealing with dates and times in tests. */
public class TestTimeUtil {
public static final DateTime START =
new DateTime(2009, 9, 30, 17, 0, 0, DateTimeZone.forOffsetHours(-4));
public static final Instant START =
LocalDateTime.of(2009, Month.SEPTEMBER, 30, 17, 0, 0)
.atOffset(ZoneOffset.ofHours(-4))
.toInstant();
private static Long clockStepMs;
private static AtomicLong clockMs;
@@ -43,7 +46,7 @@ public class TestTimeUtil {
*/
public static synchronized void resetWithClockStep(long clockStep, TimeUnit clockStepUnit) {
// Set an arbitrary start point so tests are more repeatable.
clockMs = new AtomicLong(START.getMillis());
clockMs = new AtomicLong(START.toEpochMilli());
setClockStep(clockStep, clockStepUnit);
}
@@ -56,13 +59,7 @@ public class TestTimeUtil {
public static synchronized void setClockStep(long clockStep, TimeUnit clockStepUnit) {
checkState(clockMs != null, "call resetWithClockStep first");
clockStepMs = MILLISECONDS.convert(clockStep, clockStepUnit);
DateTimeUtils.setCurrentMillisProvider(
new MillisProvider() {
@Override
public long getMillis() {
return clockMs.getAndAdd(clockStepMs);
}
});
TimeUtil.setCurrentMillisSupplier(() -> clockMs.getAndAdd(clockStepMs));
}
/**
@@ -89,7 +86,7 @@ public class TestTimeUtil {
/** Reset the clock to use the actual system clock. */
public static synchronized void useSystemTime() {
clockMs = null;
DateTimeUtils.setCurrentMillisSystem();
TimeUtil.resetCurrentMillisSupplier();
}
private TestTimeUtil() {}