From 7d3cb3d426f34bdcd1b3c17effd8dc76b10693ec Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 22 Jun 2018 16:04:36 -0700 Subject: [PATCH] Make our Clock util Serializable It doesn't entirely make semantic sense, since the actual state of the SystemClock isn't being preserved, but it makes injection into serializable classes (e.g. mapreduces) much simpler, so it's worth doing. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=201755949 --- .../request/lock/LockHandlerImpl.java | 20 +++++-------------- java/google/registry/util/Clock.java | 12 +++++++++-- java/google/registry/util/SystemClock.java | 2 ++ .../google/registry/testing/FakeClock.java | 3 +-- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/java/google/registry/request/lock/LockHandlerImpl.java b/java/google/registry/request/lock/LockHandlerImpl.java index a3b344a33..197c7e546 100644 --- a/java/google/registry/request/lock/LockHandlerImpl.java +++ b/java/google/registry/request/lock/LockHandlerImpl.java @@ -27,8 +27,6 @@ import google.registry.model.server.Lock; import google.registry.util.AppEngineTimeLimiter; import google.registry.util.Clock; import google.registry.util.RequestStatusChecker; -import google.registry.util.SystemClock; -import java.io.Serializable; import java.util.HashSet; import java.util.Optional; import java.util.Set; @@ -42,16 +40,16 @@ import org.joda.time.DateTime; import org.joda.time.Duration; /** Implementation of {@link LockHandler} that uses the datastore lock. */ -public class LockHandlerImpl implements LockHandler, Serializable { +public class LockHandlerImpl implements LockHandler { - private static final long serialVersionUID = 5162259753801400985L; + private static final long serialVersionUID = 5746905970040002524L; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); /** Fudge factor to make sure we kill threads before a lock actually expires. */ private static final Duration LOCK_TIMEOUT_FUDGE = Duration.standardSeconds(5); private final RequestStatusChecker requestStatusChecker; - @Nullable private transient Clock clock; + private final Clock clock; @Inject public LockHandlerImpl(RequestStatusChecker requestStatusChecker, Clock clock) { @@ -59,14 +57,6 @@ public class LockHandlerImpl implements LockHandler, Serializable { this.clock = clock; } - private synchronized Clock getClock() { - // Re-set the clock on first use after de-serialization - if (clock == null) { - clock = new SystemClock(); - } - return clock; - } - /** * Acquire one or more locks and execute a Void {@link Callable}. * @@ -83,7 +73,7 @@ public class LockHandlerImpl implements LockHandler, Serializable { @Nullable String tld, Duration leaseLength, String... lockNames) { - DateTime startTime = getClock().nowUtc(); + DateTime startTime = clock.nowUtc(); String sanitizedTld = Strings.emptyToNull(tld); try { return AppEngineTimeLimiter.create() @@ -100,7 +90,7 @@ public class LockHandlerImpl implements LockHandler, Serializable { "Execution on locks '%s' for TLD '%s' timed out after %s; started at %s", Joiner.on(", ").join(lockNames), Optional.ofNullable(sanitizedTld).orElse("(null)"), - new Duration(startTime, getClock().nowUtc()), + new Duration(startTime, clock.nowUtc()), startTime), cause); } diff --git a/java/google/registry/util/Clock.java b/java/google/registry/util/Clock.java index 4e53cd0e8..78372cba4 100644 --- a/java/google/registry/util/Clock.java +++ b/java/google/registry/util/Clock.java @@ -14,12 +14,20 @@ package google.registry.util; +import java.io.Serializable; import javax.annotation.concurrent.ThreadSafe; import org.joda.time.DateTime; -/** A clock that tells the current time in milliseconds or nanoseconds. */ +/** + * A clock that tells the current time in milliseconds or nanoseconds. + * + *

Clocks are technically serializable because they are either a stateless wrapper around the + * system clock, or for testing, are just a wrapper around a DateTime. This means that if you + * serialize a clock and deserialize it elsewhere, you won't necessarily get the same time or time + * zone -- what you will get is a functioning clock. + */ @ThreadSafe -public interface Clock { +public interface Clock extends Serializable { /** Returns current time in UTC timezone. */ DateTime nowUtc(); diff --git a/java/google/registry/util/SystemClock.java b/java/google/registry/util/SystemClock.java index b50c9f894..ec6693025 100644 --- a/java/google/registry/util/SystemClock.java +++ b/java/google/registry/util/SystemClock.java @@ -25,6 +25,8 @@ import org.joda.time.DateTime; @ThreadSafe public class SystemClock implements Clock { + private static final long serialVersionUID = 5165372013848947515L; + /** Returns the current time. */ @Override public DateTime nowUtc() { diff --git a/javatests/google/registry/testing/FakeClock.java b/javatests/google/registry/testing/FakeClock.java index 24d20456b..24431c557 100644 --- a/javatests/google/registry/testing/FakeClock.java +++ b/javatests/google/registry/testing/FakeClock.java @@ -19,7 +19,6 @@ import static org.joda.time.DateTimeZone.UTC; import static org.joda.time.Duration.millis; import google.registry.util.Clock; -import java.io.Serializable; import java.util.concurrent.atomic.AtomicLong; import javax.annotation.concurrent.ThreadSafe; import org.joda.time.DateTime; @@ -28,7 +27,7 @@ import org.joda.time.ReadableInstant; /** A mock clock for testing purposes that supports telling, setting, and advancing the time. */ @ThreadSafe -public final class FakeClock implements Clock, Serializable { +public final class FakeClock implements Clock { private static final long serialVersionUID = 675054721685304599L;