Create a ClaimsListDualDatabaseDao (#1011)

The dual DAO takes care of switching between databases, comparing the
results of one to the results of the other, and caching the result. All
calls to ClaimsList retrieval or storing should use the
dual-database-DAO.

Previously, calls to comparing the lists were somewhat scattered
throughout the codebase. Now, there is one class for retrieval and
comparison (the dual DAO), one class for retrieval from SQL (the SQL
DAO), and one class for retrieval from Datastore (ClaimsListShard
itself, though the retrieval could be moved in to a separate DAO if we
wished).

In addition, we rename the ClaimsListDao to ClaimsListSqlDao

core/src/test/java/google/registry/flows/ResourceFlowTestCase.java

@@ -18,7 +18,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
-import static google.registry.model.tmch.ClaimsListShardTest.createTestClaimsListShard;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
@@ -39,8 +38,8 @@ import google.registry.model.eppinput.EppInput.ResourceCommandWrapper;
 import google.registry.model.eppinput.ResourceCommand;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
-import google.registry.model.tmch.ClaimsListShard.ClaimsListRevision;
-import google.registry.model.tmch.ClaimsListShard.ClaimsListSingleton;
+import google.registry.model.tmch.ClaimsListDualDatabaseDao;
+import google.registry.model.tmch.ClaimsListShard;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import google.registry.util.TypeUtils.TypeInstantiator;
 import java.util.logging.Level;
@@ -103,22 +102,12 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
   }
 
   private Class<R> getResourceClass() {
-    return new TypeInstantiator<R>(getClass()){}.getExactType();
+    return new TypeInstantiator<R>(getClass()) {}.getExactType();
   }
 
-  /**
-   * Persists a testing claims list to Datastore that contains a single shard.
-   */
+  /** Persists a testing claims list to Datastore that contains a single shard. */
   protected void persistClaimsList(ImmutableMap<String, String> labelsToKeys) {
-    ClaimsListSingleton singleton = new ClaimsListSingleton();
-    Key<ClaimsListRevision> revision = ClaimsListRevision.createKey(singleton);
-    singleton.setActiveRevision(revision);
-    ofy().saveWithoutBackup().entity(singleton).now();
-    if (!labelsToKeys.isEmpty()) {
-      ofy().saveWithoutBackup()
-          .entity(createTestClaimsListShard(clock.nowUtc(), labelsToKeys, revision))
-          .now();
-    }
+    ClaimsListDualDatabaseDao.save(ClaimsListShard.create(clock.nowUtc(), labelsToKeys));
   }
 
   @Test

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

@@ -0,0 +1,143 @@
+// Copyright 2021 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.model.tmch;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
+import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedMap;
+import google.registry.config.RegistryEnvironment;
+import google.registry.model.EntityTestCase;
+import google.registry.model.common.DatabaseTransitionSchedule;
+import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabase;
+import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabaseTransition;
+import google.registry.model.common.DatabaseTransitionSchedule.TransitionId;
+import google.registry.model.common.TimedTransitionProperty;
+import google.registry.testing.SystemPropertyExtension;
+import org.joda.time.Duration;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+/** Unit tests for {@link ClaimsListDualDatabaseDao}. */
+public class ClaimsListDualDatabaseDaoTest extends EntityTestCase {
+
+  @RegisterExtension
+  @Order(value = Integer.MAX_VALUE)
+  final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension();
+
+  @BeforeEach
+  void beforeEach() {
+    DatabaseTransitionSchedule schedule =
+        DatabaseTransitionSchedule.create(
+            TransitionId.CLAIMS_LIST,
+            TimedTransitionProperty.fromValueMap(
+                ImmutableSortedMap.of(
+                    START_OF_TIME,
+                    PrimaryDatabase.DATASTORE,
+                    fakeClock.nowUtc().plusDays(1),
+                    PrimaryDatabase.CLOUD_SQL),
+                PrimaryDatabaseTransition.class));
+    tm().transactNew(() -> ofy().saveWithoutBackup().entity(schedule).now());
+  }
+
+  @Test
+  void testGetList_missingSql() {
+    createClaimsList().saveToDatastore();
+    assertThat(assertThrows(IllegalStateException.class, ClaimsListDualDatabaseDao::get))
+        .hasMessageThat()
+        .isEqualTo("Claims list found in primary DB but not in secondary DB.");
+  }
+
+  @Test
+  void testGetList_missingOfy() {
+    fakeClock.advanceBy(Duration.standardDays(5));
+    ClaimsListSqlDao.save(createClaimsList());
+    assertThat(assertThrows(IllegalStateException.class, ClaimsListDualDatabaseDao::get))
+        .hasMessageThat()
+        .isEqualTo("Claims list found in primary DB but not in secondary DB.");
+  }
+
+  @Test
+  void testGetList_fromOfy_different() {
+    createClaimsList().saveToDatastore();
+    ClaimsListSqlDao.save(
+        ClaimsListShard.create(fakeClock.nowUtc(), ImmutableMap.of("foo", "bar")));
+    assertThat(assertThrows(IllegalStateException.class, ClaimsListDualDatabaseDao::get))
+        .hasMessageThat()
+        .isEqualTo(
+            "Unequal claims lists detected:\n"
+                + "Domain label label1 with key key1 only appears in the primary DB.\n"
+                + "Domain label label2 with key key2 only appears in the primary DB.\n"
+                + "Domain label foo with key bar only appears in the secondary DB.\n");
+  }
+
+  @Test
+  void testGetList_fromSql_different() {
+    fakeClock.advanceBy(Duration.standardDays(5));
+    ClaimsListShard.create(fakeClock.nowUtc(), ImmutableMap.of("foo", "bar")).saveToDatastore();
+    ClaimsListSqlDao.save(createClaimsList());
+    assertThat(assertThrows(IllegalStateException.class, ClaimsListDualDatabaseDao::get))
+        .hasMessageThat()
+        .isEqualTo(
+            "Unequal claims lists detected:\n"
+                + "Domain label label1 with key key1 only appears in the primary DB.\n"
+                + "Domain label label2 with key key2 only appears in the primary DB.\n"
+                + "Domain label foo with key bar only appears in the secondary DB.\n");
+  }
+
+  @Test
+  void testSaveAndGet() {
+    tm().transact(() -> ClaimsListDualDatabaseDao.save(createClaimsList()));
+    assertAboutImmutableObjects()
+        .that(ClaimsListDualDatabaseDao.get())
+        .isEqualExceptFields(createClaimsList(), "id", "revisionId", "creationTimestamp");
+  }
+
+  @Test
+  void testGet_empty() {
+    assertThat(tm().transact(ClaimsListDualDatabaseDao::get).getLabelsToKeys()).isEmpty();
+  }
+
+  @Test
+  void testGetList_missingSql_notInTest() {
+    RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension);
+    createClaimsList().saveToDatastore();
+    // Shouldn't fail in production
+    assertThat(ClaimsListDualDatabaseDao.get().getLabelsToKeys())
+        .isEqualTo(createClaimsList().getLabelsToKeys());
+  }
+
+  @Test
+  void testGetList_missingOfy_notInTest() {
+    RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension);
+    fakeClock.advanceBy(Duration.standardDays(5));
+    ClaimsListSqlDao.save(createClaimsList());
+    // Shouldn't fail in production
+    assertThat(ClaimsListDualDatabaseDao.get().getLabelsToKeys())
+        .isEqualTo(createClaimsList().getLabelsToKeys());
+  }
+
+  private ClaimsListShard createClaimsList() {
+    return ClaimsListShard.create(
+        fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+  }
+}

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

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
-import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -48,8 +47,7 @@ public class ClaimsListShardTest {
     assertThrows(
         UnshardedSaveException.class,
         () ->
-            tm()
-                .transact(
+            tm().transact(
                     () -> {
                       ClaimsListShard claimsList =
                           ClaimsListShard.create(
@@ -62,8 +60,7 @@ public class ClaimsListShardTest {
 
   @Test
   void testGet_safelyLoadsEmptyClaimsList_whenNoShardsExist() {
-    assertThat(ClaimsListShard.get().labelsToKeys).isEmpty();
-    assertThat(ClaimsListShard.get().creationTime).isEqualTo(START_OF_TIME);
+    assertThat(ClaimsListShard.getFromDatastore()).isEmpty();
   }
 
   @Test
@@ -77,11 +74,12 @@ public class ClaimsListShardTest {
     // Save it with sharding, and make sure that reloading it works.
     ClaimsListShard unsharded = ClaimsListShard.create(now, ImmutableMap.copyOf(labelsToKeys));
     unsharded.saveToDatastore(shardSize);
-    assertThat(ClaimsListShard.get().labelsToKeys).isEqualTo(unsharded.labelsToKeys);
+    assertThat(ClaimsListShard.getFromDatastore().get().labelsToKeys)
+        .isEqualTo(unsharded.labelsToKeys);
     List<ClaimsListShard> shards1 = ofy().load().type(ClaimsListShard.class).list();
     assertThat(shards1).hasSize(4);
-    assertThat(ClaimsListShard.get().getClaimKey("1")).hasValue("1");
-    assertThat(ClaimsListShard.get().getClaimKey("a")).isEmpty();
+    assertThat(ClaimsListShard.getFromDatastore().get().getClaimKey("1")).hasValue("1");
+    assertThat(ClaimsListShard.getFromDatastore().get().getClaimKey("a")).isEmpty();
     assertThat(ClaimsListShard.getCurrentRevision()).isEqualTo(shards1.get(0).parent);
 
     // Create a smaller ClaimsList that will need only 2 shards to save.
@@ -92,8 +90,10 @@ public class ClaimsListShardTest {
     unsharded = ClaimsListShard.create(now.plusDays(1), ImmutableMap.copyOf(labelsToKeys));
     unsharded.saveToDatastore(shardSize);
     ofy().clearSessionCache();
-    assertThat(ClaimsListShard.get().labelsToKeys).hasSize(unsharded.labelsToKeys.size());
-    assertThat(ClaimsListShard.get().labelsToKeys).isEqualTo(unsharded.labelsToKeys);
+    assertThat(ClaimsListShard.getFromDatastore().get().labelsToKeys)
+        .hasSize(unsharded.labelsToKeys.size());
+    assertThat(ClaimsListShard.getFromDatastore().get().labelsToKeys)
+        .isEqualTo(unsharded.labelsToKeys);
     List<ClaimsListShard> shards2 = ofy().load().type(ClaimsListShard.class).list();
     assertThat(shards2).hasSize(2);
 

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

@@ -15,18 +15,21 @@
 package google.registry.model.tmch;
 
 import static com.google.common.truth.Truth.assertThat;
+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.JpaTestRules;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
+import javax.persistence.PersistenceException;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-/** Unit tests for {@link ClaimsListDao}. */
-public class ClaimsListDaoTest {
+/** Unit tests for {@link ClaimsListSqlDao}. */
+public class ClaimsListSqlDaoTest {
 
   private final FakeClock fakeClock = new FakeClock();
 
@@ -39,40 +42,40 @@ public class ClaimsListDaoTest {
   final DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension();
 
   @Test
-  void trySave_insertsClaimsListSuccessfully() {
+  void save_insertsClaimsListSuccessfully() {
     ClaimsListShard claimsList =
         ClaimsListShard.create(
             fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
-    ClaimsListDao.trySave(claimsList);
-    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListSqlDao.save(claimsList);
+    ClaimsListShard insertedClaimsList = ClaimsListSqlDao.get().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     assertThat(insertedClaimsList.getCreationTimestamp()).isEqualTo(fakeClock.nowUtc());
   }
 
   @Test
-  void trySave_noExceptionThrownWhenSaveFail() {
+  void save_fail_duplicateId() {
     ClaimsListShard claimsList =
         ClaimsListShard.create(
             fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
-    ClaimsListDao.trySave(claimsList);
-    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListSqlDao.save(claimsList);
+    ClaimsListShard insertedClaimsList = ClaimsListSqlDao.get().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     // Save ClaimsList with existing revisionId should fail because revisionId is the primary key.
-    ClaimsListDao.trySave(insertedClaimsList);
+    assertThrows(PersistenceException.class, () -> ClaimsListSqlDao.save(insertedClaimsList));
   }
 
   @Test
-  void trySave_claimsListWithNoEntries() {
+  void save_claimsListWithNoEntries() {
     ClaimsListShard claimsList = ClaimsListShard.create(fakeClock.nowUtc(), ImmutableMap.of());
-    ClaimsListDao.trySave(claimsList);
-    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListSqlDao.save(claimsList);
+    ClaimsListShard insertedClaimsList = ClaimsListSqlDao.get().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     assertThat(insertedClaimsList.getLabelsToKeys()).isEmpty();
   }
 
   @Test
   void getCurrent_returnsEmptyListIfTableIsEmpty() {
-    assertThat(ClaimsListDao.getLatestRevision().isPresent()).isFalse();
+    Truth8.assertThat(ClaimsListSqlDao.get()).isEmpty();
   }
 
   @Test
@@ -83,9 +86,9 @@ public class ClaimsListDaoTest {
     ClaimsListShard newClaimsList =
         ClaimsListShard.create(
             fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4"));
-    ClaimsListDao.trySave(oldClaimsList);
-    ClaimsListDao.trySave(newClaimsList);
-    assertClaimsListEquals(newClaimsList, ClaimsListDao.getLatestRevision().get());
+    ClaimsListSqlDao.save(oldClaimsList);
+    ClaimsListSqlDao.save(newClaimsList);
+    assertClaimsListEquals(newClaimsList, ClaimsListSqlDao.get().get());
   }
 
   private void assertClaimsListEquals(ClaimsListShard left, ClaimsListShard right) {

core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java

@@ -33,7 +33,7 @@ import google.registry.model.reporting.Spec11ThreatMatchTest;
 import google.registry.model.server.KmsSecretRevisionSqlDaoTest;
 import google.registry.model.server.ServerSecretTest;
 import google.registry.model.smd.SignedMarkRevocationListDaoTest;
-import google.registry.model.tmch.ClaimsListDaoTest;
+import google.registry.model.tmch.ClaimsListSqlDaoTest;
 import google.registry.model.tmch.TmchCrlTest;
 import google.registry.persistence.transaction.JpaEntityCoverageExtension;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
@@ -82,7 +82,7 @@ import org.junit.runner.RunWith;
   BeforeSuiteTest.class,
   AllocationTokenTest.class,
   BillingEventTest.class,
-  ClaimsListDaoTest.class,
+  ClaimsListSqlDaoTest.class,
   ContactHistoryTest.class,
   ContactResourceTest.class,
   CursorTest.class,

core/src/test/java/google/registry/tmch/TmchDnlActionTest.java

@@ -20,6 +20,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import google.registry.model.tmch.ClaimsListDualDatabaseDao;
 import google.registry.model.tmch.ClaimsListShard;
 import java.util.Optional;
 import org.joda.time.DateTime;
@@ -37,7 +38,7 @@ class TmchDnlActionTest extends TmchActionTestCase {
 
   @Test
   void testDnl() throws Exception {
-    assertThat(ClaimsListShard.get().getClaimKey("xn----7sbejwbn3axu3d")).isEmpty();
+    assertThat(ClaimsListDualDatabaseDao.get().getClaimKey("xn----7sbejwbn3axu3d")).isEmpty();
     when(httpResponse.getContent())
         .thenReturn(TmchTestData.loadBytes("dnl-latest.csv").read())
         .thenReturn(TmchTestData.loadBytes("dnl-latest.sig").read());
@@ -49,7 +50,7 @@ class TmchDnlActionTest extends TmchActionTestCase {
         .isEqualTo(MARKSDB_URL + "/dnl/dnl-latest.sig");
 
     // Make sure the contents of testdata/dnl-latest.csv got inserted into the database.
-    ClaimsListShard claimsList = ClaimsListShard.get();
+    ClaimsListShard claimsList = ClaimsListDualDatabaseDao.get();
     assertThat(claimsList.getTmdbGenerationTime())
         .isEqualTo(DateTime.parse("2013-11-24T23:15:37.4Z"));
     assertThat(claimsList.getClaimKey("xn----7sbejwbn3axu3d"))

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

@@ -20,6 +20,7 @@ import static java.nio.file.Files.readAllLines;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableMap;
+import google.registry.model.tmch.ClaimsListDualDatabaseDao;
 import google.registry.model.tmch.ClaimsListShard;
 import java.io.File;
 import java.nio.file.Files;
@@ -31,7 +32,8 @@ class GetClaimsListCommandTest extends CommandTestCase<GetClaimsListCommand> {
 
   @Test
   void testSuccess_getWorks() throws Exception {
-    ClaimsListShard.create(DateTime.now(UTC), ImmutableMap.of("a", "1", "b", "2")).save();
+    ClaimsListDualDatabaseDao.save(
+        ClaimsListShard.create(DateTime.now(UTC), ImmutableMap.of("a", "1", "b", "2")));
     File output = tmpDir.resolve("claims.txt").toFile();
     runCommand("--output=" + output.getAbsolutePath());
     assertThat(readAllLines(output.toPath(), UTF_8)).containsExactly("a,1", "b,2");
@@ -39,7 +41,8 @@ class GetClaimsListCommandTest extends CommandTestCase<GetClaimsListCommand> {
 
   @Test
   void testSuccess_endsWithNewline() throws Exception {
-    ClaimsListShard.create(DateTime.now(UTC), ImmutableMap.of("a", "1")).save();
+    ClaimsListDualDatabaseDao.save(
+        ClaimsListShard.create(DateTime.now(UTC), ImmutableMap.of("a", "1")));
     File output = tmpDir.resolve("claims.txt").toFile();
     runCommand("--output=" + output.getAbsolutePath());
     assertThat(new String(Files.readAllBytes(output.toPath()), UTF_8)).endsWith("\n");

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

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import google.registry.model.tmch.ClaimsListDualDatabaseDao;
 import google.registry.model.tmch.ClaimsListShard;
 import java.io.FileNotFoundException;
 import org.joda.time.DateTime;
@@ -36,7 +37,7 @@ class UploadClaimsListCommandTest extends CommandTestCase<UploadClaimsListComman
       "anotherexample,2013041500/A/C/7/rHdC4wnrWRvPY6nneCVtQhFj0000000003,2011-08-16T12:00:00.0Z");
     runCommand("--force", filename);
 
-    ClaimsListShard claimsList = ClaimsListShard.get();
+    ClaimsListShard claimsList = ClaimsListDualDatabaseDao.get();
     assertThat(claimsList.getTmdbGenerationTime())
         .isEqualTo(DateTime.parse("2012-08-16T00:00:00.0Z"));
     assertThat(claimsList.getClaimKey("example"))