Add caches to ClaimsListDao and ClaimsList (#1731)

We cache the ClaimsList Java object for six hours (we don't expect it to
change frequently, and the cron job to update it only runs every twelve
hours). Subsequent calls to ClaimsListDao::get will return the cached
value.

Within the ClaimsList Java object itself, we cache any labels that we
have retrieved. While we already have a form of a cache here in the
"labelsToKeys" map, that only handles situations where we've loaded the
entire map from the database. We want to have a non-guaranteed cache in
order to make repeated calls to getClaimKey fast.

core/src/main/java/google/registry/config/RegistryConfig.java

@@ -1427,6 +1427,11 @@ public static int getEppResourceMaxCachedEntries() {
     return CONFIG_SETTINGS.get().caching.eppResourceMaxCachedEntries;
   }
 
+  /** Returns the amount of time that a particular claims list should be cached. */
+  public static java.time.Duration getClaimsListCacheDuration() {
+    return java.time.Duration.ofSeconds(CONFIG_SETTINGS.get().caching.claimsListCachingSeconds);
+  }
+
   /** Returns the email address that outgoing emails from the app are sent from. */
   public static InternetAddress getGSuiteOutgoingEmailAddress() {
     return parseEmailAddress(CONFIG_SETTINGS.get().gSuite.outgoingEmailAddress);

core/src/main/java/google/registry/config/RegistryConfigSettings.java

@@ -155,6 +155,7 @@ public static class Caching {
     public boolean eppResourceCachingEnabled;
     public int eppResourceCachingSeconds;
     public int eppResourceMaxCachedEntries;
+    public int claimsListCachingSeconds;
   }
 
   /** Configuration for ICANN monthly reporting. */

core/src/main/java/google/registry/config/files/default-config.yaml

@@ -294,6 +294,10 @@ caching:
   # have to be very large to achieve the vast majority of possible gains.
   eppResourceMaxCachedEntries: 500
 
+  # Length of time that a claims list will be cached after retrieval. A fairly
+  # long duration is acceptable because claims lists don't change frequently.
+  claimsListCachingSeconds: 21600 # six hours
+
 oAuth:
   # OAuth scopes to detect on access tokens. Superset of requiredOauthScopes.
   availableOauthScopes:

core/src/main/java/google/registry/model/tld/label/PremiumListDao.java

@@ -73,7 +73,7 @@ public static LoadingCache<String, Optional<PremiumList>> createPremiumListCache
   }
 
   /**
-   * In-memory price cache for for a given premium list revision and domain label.
+   * In-memory price cache for a given premium list revision and domain label.
    *
    * <p>Note that premium list revision ids are globally unique, so this cache is specific to a
    * given premium list. Premium list entries might not be present, as indicated by the Optional

core/src/main/java/google/registry/model/tld/label/ReservedList.java

@@ -90,12 +90,13 @@ void preRemove() {
    *
    * <p>We need to persist the list entries, but only on the initial insert (not on update) since
    * the entries themselves never get changed, so we only annotate it with {@link PostPersist}, not
-   * {@link PostUpdate}.
+   * PostUpdate.
    */
   @PostPersist
   void postPersist() {
     if (reservedListMap != null) {
-      reservedListMap.values().stream()
+      reservedListMap
+          .values()
           .forEach(
               entry -> {
                 // We can safely change the revision id since it's "Insignificant".

core/src/main/java/google/registry/model/tmch/ClaimsList.java

@@ -20,7 +20,10 @@
 import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 
+import com.github.benmanes.caffeine.cache.LoadingCache;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import google.registry.model.CacheUtils;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
 import java.util.Map;
@@ -81,9 +84,26 @@ public class ClaimsList extends ImmutableObject {
    * <p>This field requires special treatment since we want to lazy load it. We have to remove it
    * from the immutability contract so we can modify it after construction and we have to handle the
    * database processing on our own so we can detach it after load.
+   *
+   * <p>Unlike the cache below, this is guaranteed to contain all mappings from labels to claim keys
+   * if it is not null.
    */
   @Insignificant @Transient ImmutableMap<String, String> labelsToKeys;
 
+  /**
+   * A not-necessarily-complete cache of labels to claim keys.
+   *
+   * <p>At any point in time this might or might not contain none, some, or all of the mappings from
+   * labels to claim keys. Exists so that repeated calls to {@link #getClaimKey(String)} can be
+   * quick.
+   *
+   * <p>Note: this cache has no expiration because while claims list revisions can be added over
+   * time, each instance of a claims list is immutable.
+   */
+  @Insignificant @Transient @VisibleForTesting
+  final LoadingCache<String, Optional<String>> claimKeyCache =
+      CacheUtils.newCacheBuilder().build(this::getClaimKeyUncached);
+
   @PreRemove
   void preRemove() {
     jpaTm()
@@ -139,18 +159,7 @@ public DateTime getCreationTimestamp() {
    * entries and cache them locally.
    */
   public Optional<String> getClaimKey(String label) {
-    if (labelsToKeys != null) {
-      return Optional.ofNullable(labelsToKeys.get(label));
-    }
-    return jpaTm()
-        .transact(
-            () ->
-                jpaTm()
-                    .createQueryComposer(ClaimsEntry.class)
-                    .where("revisionId", EQ, revisionId)
-                    .where("domainLabel", EQ, label)
-                    .first()
-                    .map(ClaimsEntry::getClaimKey));
+    return claimKeyCache.get(label);
   }
 
   /**
@@ -192,6 +201,27 @@ public long size() {
     return labelsToKeys.size();
   }
 
+  /**
+   * Returns the claim key for a given domain if there is one, empty otherwise.
+   *
+   * <p>This attempts to load from the base {@link #labelsToKeys} if possible, otherwise it will
+   * query the database for the entry requested.
+   */
+  private Optional<String> getClaimKeyUncached(String label) {
+    if (labelsToKeys != null) {
+      return Optional.ofNullable(labelsToKeys.get(label));
+    }
+    return jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                    .createQueryComposer(ClaimsEntry.class)
+                    .where("revisionId", EQ, revisionId)
+                    .where("domainLabel", EQ, label)
+                    .first()
+                    .map(ClaimsEntry::getClaimKey));
+  }
+
   public static ClaimsList create(
       DateTime tmdbGenerationTime, ImmutableMap<String, String> labelsToKeys) {
     ClaimsList instance = new ClaimsList();

core/src/main/java/google/registry/model/tmch/ClaimsListDao.java

@@ -14,25 +14,57 @@
 
 package google.registry.model.tmch;
 
+import static google.registry.config.RegistryConfig.getClaimsListCacheDuration;
 import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 
+import com.github.benmanes.caffeine.cache.LoadingCache;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import google.registry.model.CacheUtils;
+import java.time.Duration;
+import java.util.Optional;
 
 /** Data access object for {@link ClaimsList}. */
 public class ClaimsListDao {
 
+  /**
+   * Cache of the {@link ClaimsList} instance.
+   *
+   * <p>The key is meaningless since we only have one active claims list, this is essentially a
+   * memoizing Supplier that can be reset.
+   */
+  @VisibleForTesting
+  static LoadingCache<Class<ClaimsListDao>, ClaimsList> CACHE =
+      createCache(getClaimsListCacheDuration());
+
+  @VisibleForTesting
+  public static void setCacheForTest(Optional<Duration> expiry) {
+    Duration effectiveExpiry = expiry.orElse(getClaimsListCacheDuration());
+    CACHE = createCache(effectiveExpiry);
+  }
+
+  private static LoadingCache<Class<ClaimsListDao>, ClaimsList> createCache(Duration expiry) {
+    return CacheUtils.newCacheBuilder(expiry).build(ignored -> ClaimsListDao.getUncached());
+  }
+
   /** Saves the given {@link ClaimsList} to Cloud SQL. */
   public static void save(ClaimsList claimsList) {
     jpaTm().transact(() -> jpaTm().insert(claimsList));
+    CACHE.put(ClaimsListDao.class, claimsList);
+  }
+
+  /** Returns the most recent revision of the {@link ClaimsList} from the cache. */
+  public static ClaimsList get() {
+    return CACHE.get(ClaimsListDao.class);
   }
 
   /**
    * Returns the most recent revision of the {@link ClaimsList} in SQL or an empty list if it
    * doesn't exist.
    */
-  public static ClaimsList get() {
+  private static ClaimsList getUncached() {
     return jpaTm()
         .transact(
             () -> {

core/src/main/java/google/registry/tools/GetClaimsListCommand.java

@@ -30,8 +30,8 @@
 /**
  * A command to download the current claims list.
  *
- * <p>This is not the original file we fetched from TMCH. It is just a representation of what we
- * are currently storing in Datastore.
+ * <p>This is not the original file we fetched from TMCH. It is just a representation of what we are
+ * currently storing in SQL.
  */
 @Parameters(separators = " =", commandDescription = "Download the current claims list")
 final class GetClaimsListCommand implements CommandWithRemoteApi {

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

@@ -41,13 +41,20 @@
 import google.registry.model.domain.Domain;
 import google.registry.model.tld.Registry;
 import google.registry.model.tld.Registry.TldState;
+import google.registry.testing.TestCacheExtension;
+import java.time.Duration;
 import org.joda.money.Money;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /** Unit tests for {@link DomainClaimsCheckFlow}. */
 public class DomainClaimsCheckFlowTest extends ResourceFlowTestCase<DomainClaimsCheckFlow, Domain> {
 
+  @RegisterExtension
+  public final TestCacheExtension testCacheExtension =
+      new TestCacheExtension.Builder().withClaimsListCache(Duration.ofHours(6)).build();
+
   DomainClaimsCheckFlowTest() {
     setEppInput("domain_check_claims.xml");
   }

core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java

@@ -15,12 +15,16 @@
 package google.registry.model.tmch;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.truth.Truth8;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.FakeClock;
+import google.registry.testing.TestCacheExtension;
+import java.time.Duration;
 import javax.persistence.PersistenceException;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -37,6 +41,11 @@ public class ClaimsListDaoTest {
           .withoutCannedData()
           .buildIntegrationWithCoverageExtension();
 
+  // Set long persist times on the cache so it can be tested (cache times default to 0 in tests).
+  @RegisterExtension
+  public final TestCacheExtension testCacheExtension =
+      new TestCacheExtension.Builder().withClaimsListCache(Duration.ofHours(6)).build();
+
   @Test
   void save_insertsClaimsListSuccessfully() {
     ClaimsList claimsList =
@@ -83,6 +92,40 @@ void getCurrent_returnsLatestClaims() {
     assertClaimsListEquals(newClaimsList, ClaimsListDao.get());
   }
 
+  @Test
+  void testDaoCaching_savesAndUpdates() {
+    assertThat(ClaimsListDao.CACHE.getIfPresent(ClaimsListDao.class)).isNull();
+    ClaimsList oldList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListDao.save(oldList);
+    assertThat(ClaimsListDao.CACHE.getIfPresent(ClaimsListDao.class)).isEqualTo(oldList);
+    ClaimsList newList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4"));
+    ClaimsListDao.save(newList);
+    assertThat(ClaimsListDao.CACHE.getIfPresent(ClaimsListDao.class)).isEqualTo(newList);
+  }
+
+  @Test
+  void testEntryCaching_savesAndUpdates() {
+    ClaimsList claimsList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    // Bypass the DAO to avoid the cache
+    jpaTm().transact(() -> jpaTm().insert(claimsList));
+    ClaimsList fromDatabase = ClaimsListDao.get();
+    // At first, we haven't loaded any entries
+    assertThat(fromDatabase.claimKeyCache.getIfPresent("label1")).isNull();
+    Truth8.assertThat(fromDatabase.getClaimKey("label1")).hasValue("key1");
+    // After retrieval, the key exists
+    Truth8.assertThat(fromDatabase.claimKeyCache.getIfPresent("label1")).hasValue("key1");
+    assertThat(fromDatabase.claimKeyCache.getIfPresent("label2")).isNull();
+    // Loading labels-to-keys should still work
+    assertThat(fromDatabase.getLabelsToKeys()).containsExactly("label1", "key1", "label2", "key2");
+    // We should also cache nonexistent values
+    assertThat(fromDatabase.claimKeyCache.getIfPresent("nonexistent")).isNull();
+    Truth8.assertThat(fromDatabase.getClaimKey("nonexistent")).isEmpty();
+    Truth8.assertThat(fromDatabase.claimKeyCache.getIfPresent("nonexistent")).isEmpty();
+  }
+
   private void assertClaimsListEquals(ClaimsList left, ClaimsList right) {
     assertThat(left.getRevisionId()).isEqualTo(right.getRevisionId());
     assertThat(left.getTmdbGenerationTime()).isEqualTo(right.getTmdbGenerationTime());

core/src/test/java/google/registry/testing/TestCacheExtension.java

@@ -19,6 +19,7 @@
 import google.registry.model.EppResource;
 import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.tld.label.PremiumListDao;
+import google.registry.model.tmch.ClaimsListDao;
 import java.time.Duration;
 import java.util.Map;
 import java.util.Optional;
@@ -75,6 +76,12 @@ public Builder withPremiumListsCache(Duration expiry) {
       return this;
     }
 
+    public Builder withClaimsListCache(Duration expiry) {
+      cacheHandlerMap.put(
+          "ClaimsListDao.CACHE", new TestCacheHandler(ClaimsListDao::setCacheForTest, expiry));
+      return this;
+    }
+
     public TestCacheExtension build() {
       return new TestCacheExtension(ImmutableList.copyOf(cacheHandlerMap.values()));
     }

core/src/test/java/google/registry/tools/EppLifecycleToolsTest.java

@@ -29,7 +29,9 @@
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.reporting.HistoryEntry.Type;
 import google.registry.testing.AppEngineExtension;
+import google.registry.testing.TestCacheExtension;
 import google.registry.util.Clock;
+import java.time.Duration;
 import java.util.List;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -44,6 +46,10 @@ class EppLifecycleToolsTest extends EppTestCase {
   final AppEngineExtension appEngine =
       AppEngineExtension.builder().withClock(clock).withCloudSql().withTaskQueue().build();
 
+  @RegisterExtension
+  public final TestCacheExtension testCacheExtension =
+      new TestCacheExtension.Builder().withClaimsListCache(Duration.ofHours(6)).build();
+
   @BeforeEach
   void beforeEach() {
     createTlds("example", "tld");