Add ReservedList to CloudSQL in unit tests (#992)

* Add ReservedList to CloudSQL in unit tests

* Remove unnecessary changes

* Change databasehelper to use DualDatabaseDao

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

@@ -248,7 +248,7 @@ public final class ReservedList
                 @Override
                 public ReservedList load(String listName) {
                   return tm().isOfy()
-                      ? ReservedListDualWriteDao.getLatestRevision(listName).orElse(null)
+                      ? ReservedListDualDatabaseDao.getLatestRevision(listName).orElse(null)
                       : ReservedListSqlDao.getLatestRevision(listName).orElse(null);
                 }
               });

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

@@ -23,6 +23,7 @@ import com.google.common.collect.MapDifference.ValueDifference;
 import com.google.common.collect.Maps;
 import com.google.common.flogger.FluentLogger;
 import com.googlecode.objectify.Key;
+import google.registry.model.DatabaseMigrationUtils;
 import google.registry.model.registry.label.ReservedList.ReservedListEntry;
 import google.registry.persistence.VKey;
 import java.util.Map;
@@ -35,24 +36,22 @@ import java.util.Optional;
  * <p>TODO(b/160993806): Delete this DAO and switch to use the SQL only DAO after migrating to Cloud
  * SQL.
  */
-public class ReservedListDualWriteDao {
+public class ReservedListDualDatabaseDao {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  private ReservedListDualWriteDao() {}
+  private ReservedListDualDatabaseDao() {}
 
   /** Persist a new reserved list to Cloud SQL. */
   public static void save(ReservedList reservedList) {
     ofyTm().transact(() -> ofyTm().put(reservedList));
-    try {
       logger.atInfo().log("Saving reserved list %s to Cloud SQL", reservedList.getName());
-      ReservedListSqlDao.save(reservedList);
+    DatabaseMigrationUtils.suppressExceptionUnlessInTest(
+        () -> ReservedListSqlDao.save(reservedList),
+        "Error saving the reserved list to Cloud SQL.");
       logger.atInfo().log(
           "Saved reserved list %s with %d entries to Cloud SQL",
           reservedList.getName(), reservedList.getReservedListEntries().size());
-    } catch (Throwable t) {
-      logger.atSevere().withCause(t).log("Error saving the reserved list to Cloud SQL.");
-    }
   }
 
   /**
@@ -66,12 +65,10 @@ public class ReservedListDualWriteDao {
                 VKey.createOfy(
                     ReservedList.class,
                     Key.create(getCrossTldKey(), ReservedList.class, reservedListName)));
-    try {
     // Also load the list from Cloud SQL, compare the two lists, and log if different.
-      maybeDatastoreList.ifPresent(ReservedListDualWriteDao::loadAndCompareCloudSqlList);
+    DatabaseMigrationUtils.suppressExceptionUnlessInTest(
-    } catch (Throwable t) {
+        () -> maybeDatastoreList.ifPresent(ReservedListDualDatabaseDao::loadAndCompareCloudSqlList),
-      logger.atSevere().withCause(t).log("Error comparing reserved lists.");
+        "Error comparing reserved lists.");
-    }
     return maybeDatastoreList;
   }
 
@@ -95,7 +92,7 @@ public class ReservedListDualWriteDao {
           Maps.difference(datastoreLabelsToReservations, cloudSqlList.reservedListMap);
       if (!diff.areEqual()) {
         if (diff.entriesDiffering().size() > 10) {
-          logger.atWarning().log(
+          throw new IllegalStateException(
               String.format(
                   "Unequal reserved lists detected, Cloud SQL list with revision"
                       + " id %d has %d different records than the current"
@@ -114,11 +111,11 @@ public class ReservedListDualWriteDao {
                                 + " %s in Cloud SQL.\n",
                             label, valueDiff.leftValue(), valueDiff.rightValue()));
                   });
-          logger.atWarning().log(diffMessage.toString());
+          throw new IllegalStateException(diffMessage.toString());
         }
       }
     } else {
-      logger.atWarning().log("Reserved list in Cloud SQL is empty.");
+      throw new IllegalStateException("Reserved list in Cloud SQL is empty.");
     }
   }
 }

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

@@ -17,7 +17,7 @@ package google.registry.tools;
 import com.beust.jcommander.Parameter;
 import com.google.common.flogger.FluentLogger;
 import google.registry.model.registry.label.ReservedList;
-import google.registry.model.registry.label.ReservedListDualWriteDao;
+import google.registry.model.registry.label.ReservedListDualDatabaseDao;
 import google.registry.tools.params.PathParameter;
 import java.nio.file.Path;
 import javax.annotation.Nullable;
@@ -61,7 +61,7 @@ public abstract class CreateOrUpdateReservedListCommand extends MutatingCommand
             name, reservedList.getReservedListEntries().size());
     try {
       logger.atInfo().log("Saving reserved list for TLD %s", name);
-      ReservedListDualWriteDao.save(reservedList);
+      ReservedListDualDatabaseDao.save(reservedList);
       logger.atInfo().log(message);
     } catch (Throwable e) {
       message = "Unexpected error saving reserved list from nomulus tool command";

core/src/test/java/google/registry/model/registry/RegistryTest.java

@@ -33,6 +33,7 @@ import static org.joda.money.CurrencyUnit.EUR;
 import static org.joda.money.CurrencyUnit.USD;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.googlecode.objectify.Key;
@@ -135,9 +136,23 @@ public class RegistryTest extends EntityTestCase {
   @TestOfyAndSql
   void testSetReservedList_doesntMutateExistingRegistry() {
     ReservedList rl15 =
-        persistReservedList("tld-reserved15", "potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED");
+        persistReservedList(
+            new ReservedList.Builder()
+                .setName("tld-reserved15")
+                .setReservedListMapFromLines(
+                    ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
+                .setShouldPublish(true)
+                .setLastUpdateTime(fakeClock.nowUtc())
+                .build());
     ReservedList rl16 =
-        persistReservedList("tld-reserved16", "port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED");
+        persistReservedList(
+            new ReservedList.Builder()
+                .setName("tld-reserved16")
+                .setReservedListMapFromLines(
+                    ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
+                .setShouldPublish(true)
+                .setLastUpdateTime(fakeClock.nowUtc())
+                .build());
     Registry registry1 =
         newRegistry("propter", "PROPTER")
             .asBuilder()
@@ -173,9 +188,23 @@ public class RegistryTest extends EntityTestCase {
   @TestOfyAndSql
   void testSetReservedLists() {
     ReservedList rl5 =
-        persistReservedList("tld-reserved5", "lol,FULLY_BLOCKED", "cat,FULLY_BLOCKED");
+        persistReservedList(
+            new ReservedList.Builder()
+                .setName("tld-reserved5")
+                .setReservedListMapFromLines(
+                    ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
+                .setShouldPublish(true)
+                .setLastUpdateTime(fakeClock.nowUtc())
+                .build());
     ReservedList rl6 =
-        persistReservedList("tld-reserved6", "hammock,FULLY_BLOCKED", "mouse,FULLY_BLOCKED");
+        persistReservedList(
+            new ReservedList.Builder()
+                .setName("tld-reserved6")
+                .setReservedListMapFromLines(
+                    ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
+                .setShouldPublish(true)
+                .setLastUpdateTime(fakeClock.nowUtc())
+                .build());
     Registry r =
         Registry.get("tld").asBuilder().setReservedLists(ImmutableSet.of(rl5, rl6)).build();
     assertThat(r.getReservedLists().stream().map(Key::getName))
@@ -186,15 +215,29 @@ public class RegistryTest extends EntityTestCase {
 
   @TestOfyAndSql
   void testSetReservedListsByName() {
-    persistReservedList("tld-reserved24", "lol,FULLY_BLOCKED", "cat,FULLY_BLOCKED");
+    persistReservedList(
-    persistReservedList("tld-reserved25", "mit,FULLY_BLOCKED", "tim,FULLY_BLOCKED");
+        new ReservedList.Builder()
+            .setName("tld-reserved15")
+            .setReservedListMapFromLines(
+                ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
+            .setShouldPublish(true)
+            .setLastUpdateTime(fakeClock.nowUtc())
+            .build());
+    persistReservedList(
+        new ReservedList.Builder()
+            .setName("tld-reserved16")
+            .setReservedListMapFromLines(
+                ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
+            .setShouldPublish(true)
+            .setLastUpdateTime(fakeClock.nowUtc())
+            .build());
     Registry r =
         Registry.get("tld")
             .asBuilder()
-            .setReservedListsByName(ImmutableSet.of("tld-reserved24", "tld-reserved25"))
+            .setReservedListsByName(ImmutableSet.of("tld-reserved15", "tld-reserved16"))
             .build();
     assertThat(r.getReservedLists().stream().map(Key::getName))
-        .containsExactly("tld-reserved24", "tld-reserved25");
+        .containsExactly("tld-reserved15", "tld-reserved16");
     r = Registry.get("tld").asBuilder().setReservedListsByName(ImmutableSet.of()).build();
     assertThat(r.getReservedLists()).isEmpty();
   }

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

@@ -105,6 +105,7 @@ import google.registry.model.registry.label.PremiumList.PremiumListEntry;
 import google.registry.model.registry.label.PremiumList.PremiumListRevision;
 import google.registry.model.registry.label.PremiumListDualDao;
 import google.registry.model.registry.label.ReservedList;
+import google.registry.model.registry.label.ReservedListDualDatabaseDao;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.ContactTransferData;
 import google.registry.model.transfer.DomainTransferData;
@@ -345,6 +346,13 @@ public class DatabaseHelper {
     return persistReservedList(listName, true, lines);
   }
 
+  public static ReservedList persistReservedList(ReservedList reservedList) {
+    ReservedListDualDatabaseDao.save(reservedList);
+    maybeAdvanceClock();
+    tm().clearSessionCache();
+    return reservedList;
+  }
+
   public static ReservedList persistReservedList(
       String listName, boolean shouldPublish, String... lines) {
     ReservedList reservedList =
@@ -354,13 +362,7 @@ public class DatabaseHelper {
             .setShouldPublish(shouldPublish)
             .setLastUpdateTime(DateTime.now(DateTimeZone.UTC))
             .build();
-    return tm().isOfy()
+    return persistReservedList(reservedList);
-        ? persistResource(reservedList)
-        : tm().transact(
-                () -> {
-                  tm().insert(reservedList);
-                  return reservedList;
-                });
   }
 
   /**

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

@@ -17,7 +17,7 @@ package google.registry.tools;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
-import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.testing.DatabaseHelper.persistReservedList;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -35,11 +35,11 @@ class UpdateReservedListCommandTest
 
   @BeforeEach
   void beforeEach() {
-    populateInitialReservedListInDatastore(true);
+    populateInitialReservedListInDatabase(true);
   }
 
-  private void populateInitialReservedListInDatastore(boolean shouldPublish) {
+  private void populateInitialReservedListInDatabase(boolean shouldPublish) {
-    persistResource(
+    persistReservedList(
         new ReservedList.Builder()
             .setName("xn--q9jyb4c_common-reserved")
             .setReservedListMapFromLines(ImmutableList.of("helicopter,FULLY_BLOCKED"))
@@ -89,7 +89,7 @@ class UpdateReservedListCommandTest
 
   @Test
   void testSuccess_shouldPublish_doesntOverrideFalseIfNotSpecified() throws Exception {
-    populateInitialReservedListInDatastore(false);
+    populateInitialReservedListInDatabase(false);
     runCommandForced("--input=" + reservedTermsPath);
     assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
     ReservedList reservedList = ReservedList.get("xn--q9jyb4c_common-reserved").get();