Use transaction time for deletion time cache ticker (#2848)

Basically, what happened is that the cache's expireAfterWrite was being
called some number of milliseconds (say, 50-100) after the transaction
was started. That method used the transaction time instead of the
current time, so as a result the entries were sticking around 50-100ms
longer in the cache than they should have been.

This fix contains two parts, each of which I believe would be sufficient
on their own to fix the issue:
1. Use the currentTime passed in in Expiry::expireAfterCreate
2. Use the transaction time in the cache's Ticker. This keeps everything
   on the same schedule.

Author: gbrodman

core/src/main/java/google/registry/flows/domain/DomainDeletionTimeCache.java

@@ -20,9 +20,11 @@
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.github.benmanes.caffeine.cache.Expiry;
 import com.github.benmanes.caffeine.cache.LoadingCache;
+import com.github.benmanes.caffeine.cache.Ticker;
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.ForeignKeyUtils;
 import google.registry.model.domain.Domain;
+import google.registry.util.DateTimeUtils;
 import java.util.Optional;
 import org.joda.time.DateTime;
 
@@ -55,9 +57,9 @@
 public class DomainDeletionTimeCache {
 
   // Max expiry time is ten minutes
-  private static final int MAX_EXPIRY_MILLIS = 10 * 60 * 1000;
+  private static final long NANOS_IN_ONE_MILLISECOND = 100000L;
+  private static final long MAX_EXPIRY_NANOS = 10L * 60L * 1000L * NANOS_IN_ONE_MILLISECOND;
   private static final int MAX_ENTRIES = 500;
-  private static final int NANOS_IN_ONE_MILLISECOND = 100000;
 
   /**
    * Expire after the max duration, or after the domain is set to drop (whichever comes first).
@@ -71,9 +73,13 @@ public class DomainDeletionTimeCache {
       new Expiry<>() {
         @Override
         public long expireAfterCreate(String key, DateTime value, long currentTime) {
-          long millisUntilDeletion = value.getMillis() - tm().getTransactionTime().getMillis();
-          return NANOS_IN_ONE_MILLISECOND
-              * Math.max(0L, Math.min(MAX_EXPIRY_MILLIS, millisUntilDeletion));
+          // Watch out for Long overflow
+          long deletionTimeNanos =
+              value.equals(DateTimeUtils.END_OF_TIME)
+                  ? Long.MAX_VALUE
+                  : value.getMillis() * NANOS_IN_ONE_MILLISECOND;
+          long nanosUntilDeletion = deletionTimeNanos - currentTime;
+          return Math.max(0L, Math.min(MAX_EXPIRY_NANOS, nanosUntilDeletion));
         }
 
         /** Reset the time entirely on update, as if we were creating the entry anew. */
@@ -101,9 +107,14 @@ public long expireAfterRead(
         return mostRecentResource == null ? null : mostRecentResource.deletionTime();
       };
 
+  // Unfortunately, maintenance tasks aren't necessarily already in a transaction
+  private static final Ticker TRANSACTION_TIME_TICKER =
+      () -> tm().reTransact(() -> tm().getTransactionTime().getMillis() * NANOS_IN_ONE_MILLISECOND);
+
   public static DomainDeletionTimeCache create() {
     return new DomainDeletionTimeCache(
         Caffeine.newBuilder()
+            .ticker(TRANSACTION_TIME_TICKER)
             .expireAfter(EXPIRY_POLICY)
             .maximumSize(MAX_ENTRIES)
             .build(CACHE_LOADER));

core/src/test/java/google/registry/flows/domain/DomainDeletionTimeCacheTest.java

@@ -26,6 +26,7 @@
 import google.registry.testing.FakeClock;
 import java.util.Optional;
 import org.joda.time.DateTime;
+import org.joda.time.Duration;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -67,7 +68,8 @@ void testCache_returnsOldData() {
     Domain domain = persistActiveDomain("domain.tld");
     assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
     persistDomainAsDeleted(domain, clock.nowUtc().plusDays(1));
-    // Without intervention, the cache should have the old data
+    // Without intervention, the cache should have the old data, even if a few minutes have passed
+    clock.advanceBy(Duration.standardMinutes(5));
     assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
   }
 
@@ -79,6 +81,16 @@ void testCache_returnsNewDataAfterDomainCreate() {
     assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(clock.nowUtc().plusDays(1));
   }
 
+  @Test
+  void testCache_expires() {
+    Domain domain = persistActiveDomain("domain.tld");
+    assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
+    DateTime elevenMinutesFromNow = clock.nowUtc().plusMinutes(11);
+    persistDomainAsDeleted(domain, elevenMinutesFromNow);
+    clock.advanceBy(Duration.standardMinutes(30));
+    assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(elevenMinutesFromNow);
+  }
+
   private Optional<DateTime> getDeletionTimeFromCache(String domainName) {
     return tm().transact(() -> cache.getDeletionTimeForDomain(domainName));
   }