Add dual read for ReservedList (#423)

* Add dual read for ReservedList

* Extract loading cloud sql list to a method

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

@@ -16,6 +16,7 @@ package google.registry.model.registry.label;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
@@ -31,6 +32,10 @@ import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.MapDifference;
+import com.google.common.collect.MapDifference.ValueDifference;
+import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Embed;
@@ -40,6 +45,8 @@ import com.googlecode.objectify.mapper.Mapper;
 import google.registry.model.Buildable;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.DomainLabelMetrics.MetricsReservedListMatch;
+import google.registry.schema.tld.ReservedList.ReservedEntry;
+import google.registry.schema.tld.ReservedListDao;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -54,6 +61,8 @@ import org.joda.time.DateTime;
 public final class ReservedList
     extends BaseDomainLabelList<ReservationType, ReservedList.ReservedListEntry> {
 
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   @Mapify(ReservedListEntry.LabelMapper.class)
   Map<String, ReservedListEntry> reservedListMap;
 
@@ -222,13 +231,67 @@ public final class ReservedList
               new CacheLoader<String, ReservedList>() {
                 @Override
                 public ReservedList load(String listName) {
-                  return ofy()
+                  ReservedList datastoreList =
-                      .load()
+                      ofy()
-                      .type(ReservedList.class)
+                          .load()
-                      .parent(getCrossTldKey())
+                          .type(ReservedList.class)
-                      .id(listName)
+                          .parent(getCrossTldKey())
-                      .now();
+                          .id(listName)
-                }});
+                          .now();
+                  // Also load the list from Cloud SQL, compare the two lists, and log if different.
+                  try {
+                    loadAndCompareCloudSqlList(datastoreList);
+                  } catch (Throwable t) {
+                    logger.atSevere().withCause(t).log("Error comparing reserved lists.");
+                  }
+                  return datastoreList;
+                }
+              });
+
+  private static final void loadAndCompareCloudSqlList(ReservedList datastoreList) {
+    Optional<google.registry.schema.tld.ReservedList> maybeCloudSqlList =
+        ReservedListDao.getLatestRevision(datastoreList.getName());
+    if (maybeCloudSqlList.isPresent()) {
+      Map<String, ReservedEntry> datastoreLabelsToReservations =
+          datastoreList.reservedListMap.entrySet().parallelStream()
+              .collect(
+                  toImmutableMap(
+                      entry -> entry.getKey(),
+                      entry ->
+                          ReservedEntry.create(
+                              entry.getValue().reservationType, entry.getValue().comment)));
+
+      google.registry.schema.tld.ReservedList cloudSqlList = maybeCloudSqlList.get();
+      MapDifference<String, ReservedEntry> diff =
+          Maps.difference(datastoreLabelsToReservations, cloudSqlList.getLabelsToReservations());
+      if (!diff.areEqual()) {
+        if (diff.entriesDiffering().size() > 10) {
+          logger.atWarning().log(
+              String.format(
+                  "Unequal reserved lists detected, Cloud SQL list with revision"
+                      + " id %d has %d different records than the current"
+                      + " Datastore list.",
+                  cloudSqlList.getRevisionId(), diff.entriesDiffering().size()));
+        } else {
+          StringBuilder diffMessage = new StringBuilder("Unequal reserved lists detected:\n");
+          diff.entriesDiffering().entrySet().stream()
+              .forEach(
+                  entry -> {
+                    String label = entry.getKey();
+                    ValueDifference<ReservedEntry> valueDiff = entry.getValue();
+                    diffMessage.append(
+                        String.format(
+                            "Domain label %s has entry %s in Datastore and entry"
+                                + " %s in Cloud SQL.\n",
+                            label, valueDiff.leftValue(), valueDiff.rightValue()));
+                  });
+          logger.atWarning().log(diffMessage.toString());
+        }
+      }
+    } else {
+      logger.atWarning().log("Reserved list in Cloud SQL is empty.");
+    }
+  }
 
   /**
    * Gets the {@link ReservationType} of a label in a single ReservedList, or returns an absent

core/src/main/java/google/registry/schema/tld/ReservedListCache.java

@@ -0,0 +1,54 @@
+// Copyright 2019 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.tld;
+
+import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import google.registry.util.NonFinalForTesting;
+import java.util.Optional;
+import org.joda.time.Duration;
+
+/** Caching utils for {@link ReservedList} */
+public class ReservedListCache {
+
+  /**
+   * In-memory cache for reserved lists.
+   *
+   * <p>This is cached for a shorter duration because we need to periodically reload from the DB to
+   * check if a new revision has been published, and if so, then use that.
+   */
+  @NonFinalForTesting
+  static LoadingCache<String, Optional<ReservedList>> cacheReservedLists =
+      createCacheReservedLists(getDomainLabelListCacheDuration());
+
+  @VisibleForTesting
+  static LoadingCache<String, Optional<ReservedList>> createCacheReservedLists(
+      Duration cachePersistDuration) {
+    return CacheBuilder.newBuilder()
+        .expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
+        .build(
+            new CacheLoader<String, Optional<ReservedList>>() {
+              @Override
+              public Optional<ReservedList> load(String reservedListName) {
+                return ReservedListDao.getLatestRevision(reservedListName);
+              }
+            });
+  }
+}

core/src/main/java/google/registry/schema/tld/ReservedListDao.java

@@ -16,6 +16,10 @@ package google.registry.schema.tld;
 
 import static google.registry.model.transaction.TransactionManagerFactory.jpaTm;
 
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+
 /** Data access object class for {@link ReservedList} */
 public class ReservedListDao {
 
@@ -43,5 +47,38 @@ public class ReservedListDao {
                     > 0);
   }
 
+  /**
+   * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
+   * exists. TODO(shicong): Change this method to package level access after dual-read phase.
+   */
+  public static Optional<ReservedList> getLatestRevision(String reservedListName) {
+    return jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                    .getEntityManager()
+                    .createQuery(
+                        "FROM ReservedList rl LEFT JOIN FETCH rl.labelsToReservations WHERE"
+                            + " rl.revisionId IN (SELECT MAX(revisionId) FROM ReservedList subrl"
+                            + " WHERE subrl.name = :name)",
+                        ReservedList.class)
+                    .setParameter("name", reservedListName)
+                    .getResultStream()
+                    .findFirst());
+  }
+
+  /**
+   * Returns the most recent revision of the {@link ReservedList} with the specified name, from
+   * cache.
+   */
+  public static Optional<ReservedList> getLatestRevisionCached(String reservedListName) {
+    try {
+      return ReservedListCache.cacheReservedLists.get(reservedListName);
+    } catch (ExecutionException e) {
+      throw new UncheckedExecutionException(
+          "Could not retrieve reserved list named " + reservedListName, e);
+    }
+  }
+
   private ReservedListDao() {}
 }

core/src/test/java/google/registry/schema/tld/ReservedListDaoTest.java

@@ -67,4 +67,44 @@ public class ReservedListDaoTest {
     ReservedListDao.save(ReservedList.create("testlist", false, TEST_RESERVATIONS));
     assertThat(ReservedListDao.checkExists("testlist")).isTrue();
   }
+
+  @Test
+  public void getLatestRevision_worksSuccessfully() {
+    assertThat(ReservedListDao.getLatestRevision("testlist").isPresent()).isFalse();
+    ReservedListDao.save(ReservedList.create("testlist", false, TEST_RESERVATIONS));
+    ReservedList persistedList = ReservedListDao.getLatestRevision("testlist").get();
+    assertThat(persistedList.getRevisionId()).isNotNull();
+    assertThat(persistedList.getCreationTimestamp()).isEqualTo(jpaRule.getTxnClock().nowUtc());
+    assertThat(persistedList.getName()).isEqualTo("testlist");
+    assertThat(persistedList.getShouldPublish()).isFalse();
+    assertThat(persistedList.getLabelsToReservations()).containsExactlyEntriesIn(TEST_RESERVATIONS);
+  }
+
+  @Test
+  public void getLatestRevision_returnsLatestRevision() {
+    ReservedListDao.save(
+        ReservedList.create(
+            "testlist",
+            false,
+            ImmutableMap.of(
+                "old", ReservedEntry.create(ReservationType.RESERVED_FOR_SPECIFIC_USE, null))));
+    ReservedListDao.save(ReservedList.create("testlist", false, TEST_RESERVATIONS));
+    ReservedList persistedList = ReservedListDao.getLatestRevision("testlist").get();
+    assertThat(persistedList.getRevisionId()).isNotNull();
+    assertThat(persistedList.getCreationTimestamp()).isEqualTo(jpaRule.getTxnClock().nowUtc());
+    assertThat(persistedList.getName()).isEqualTo("testlist");
+    assertThat(persistedList.getShouldPublish()).isFalse();
+    assertThat(persistedList.getLabelsToReservations()).containsExactlyEntriesIn(TEST_RESERVATIONS);
+  }
+
+  @Test
+  public void getLatestRevisionCached_worksSuccessfully() {
+    ReservedListDao.save(ReservedList.create("testlist", false, TEST_RESERVATIONS));
+    ReservedList persistedList = ReservedListDao.getLatestRevisionCached("testlist").get();
+    assertThat(persistedList.getRevisionId()).isNotNull();
+    assertThat(persistedList.getCreationTimestamp()).isEqualTo(jpaRule.getTxnClock().nowUtc());
+    assertThat(persistedList.getName()).isEqualTo("testlist");
+    assertThat(persistedList.getShouldPublish()).isFalse();
+    assertThat(persistedList.getLabelsToReservations()).containsExactlyEntriesIn(TEST_RESERVATIONS);
+  }
 }