Modify DeleteReservedListCommand to delete from both databases (#1025)

* Modify DeleteReservedListCommand to use both databases

* switch to confirming command

* fix typo

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

@@ -31,6 +31,11 @@ public class ReservedListDatastoreDao {
     ofyTm().transact(() -> ofyTm().put(reservedList));
   }
 
+  /** Delete a reserved list from Datastore. */
+  public static void delete(ReservedList reservedList) {
+    ofyTm().transact(() -> ofyTm().delete(reservedList));
+  }
+
   /**
    * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
    * exists.

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

@@ -51,6 +51,21 @@ public class ReservedListDualDatabaseDao {
     }
   }
 
+  /** Delete a reserved list from both databases. */
+  public static void delete(ReservedList reservedList) {
+    if (isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
+      ReservedListDatastoreDao.delete(reservedList);
+      DatabaseMigrationUtils.suppressExceptionUnlessInTest(
+          () -> ReservedListSqlDao.delete(reservedList),
+          "Error deleting the reserved list from Cloud SQL.");
+    } else {
+      ReservedListSqlDao.delete(reservedList);
+      DatabaseMigrationUtils.suppressExceptionUnlessInTest(
+          () -> ReservedListDatastoreDao.delete(reservedList),
+          "Error deleting the reserved list from Datastore.");
+    }
+  }
+
   /**
    * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
    * exists.

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

@@ -41,6 +41,11 @@ public class ReservedListSqlDao {
         reservedList.getName(), reservedList.getReservedListEntries().size());
   }
 
+  /** Deletes a reserved list from Cloud SQL. */
+  public static void delete(ReservedList reservedList) {
+    jpaTm().transact(() -> jpaTm().delete(reservedList));
+  }
+
   /**
    * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
    * exists.

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

@@ -21,13 +21,14 @@ import com.beust.jcommander.Parameters;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.registry.label.ReservedList;
+import google.registry.model.registry.label.ReservedListDualDatabaseDao;
 
 /**
- * Command to delete a {@link ReservedList} in Datastore. This command will fail if the reserved
+ * Command to delete a {@link ReservedList} from the database. This command will fail if the
- * list is currently in use on a tld.
+ * reserved list is currently in use on a tld.
  */
-@Parameters(separators = " =", commandDescription = "Deletes a ReservedList in Datastore.")
+@Parameters(separators = " =", commandDescription = "Deletes a ReservedList from the database.")
-final class DeleteReservedListCommand extends MutatingCommand {
+final class DeleteReservedListCommand extends ConfirmingCommand {
 
   @Parameter(
       names = {"-n", "--name"},
@@ -47,6 +48,12 @@ final class DeleteReservedListCommand extends MutatingCommand {
         tldsUsedOn.isEmpty(),
         "Cannot delete reserved list because it is used on these tld(s): %s",
         Joiner.on(", ").join(tldsUsedOn));
-    stageEntityChange(existing, null);
+  }
+
+  @Override
+  protected String execute() {
+    ReservedList existing = ReservedList.get(name).get();
+    ReservedListDualDatabaseDao.delete(existing);
+    return String.format("Deleted reserved list: %s", name);
   }
 }

core/src/test/java/google/registry/model/registry/label/ReservedListDatastoreDaoTest.java

@@ -72,6 +72,21 @@ public class ReservedListDatastoreDaoTest {
     assertThat(savedList.get()).isEqualTo(reservedList);
   }
 
+  @Test
+  void delete_worksSuccessfully() {
+    ReservedListDatastoreDao.save(reservedList);
+    assertThat(ReservedListDatastoreDao.getLatestRevision("testlist").isPresent()).isTrue();
+    ReservedListDatastoreDao.delete(reservedList);
+    assertThat(ReservedListDatastoreDao.getLatestRevision("testlist").isPresent()).isFalse();
+  }
+
+  @Test
+  void delete_listNotInDatastore() {
+    assertThat(ReservedListDatastoreDao.getLatestRevision("testlist").isPresent()).isFalse();
+    ReservedListDatastoreDao.delete(reservedList);
+    assertThat(ReservedListDatastoreDao.getLatestRevision("testlist").isPresent()).isFalse();
+  }
+
   @Test
   void getLatestRevision_worksSuccessfully() {
     assertThat(ReservedListDatastoreDao.getLatestRevision("testlist").isPresent()).isFalse();

core/src/test/java/google/registry/model/registry/label/ReservedListDualDatabaseDaoTest.java

@@ -90,7 +90,7 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase {
   }
 
   @TestOfyAndSql
-  void testSave_CloudSqlPrimary_success() {
+  void testSave_cloudSqlPrimary_success() {
     fakeClock.advanceBy(Duration.standardDays(5));
     ReservedListDualDatabaseDao.save(reservedList);
     Optional<ReservedList> savedList =
@@ -98,6 +98,27 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase {
     assertThat(savedList.get()).isEqualTo(reservedList);
   }
 
+  @TestOfyAndSql
+  void testDelete_datastorePrimary_success() {
+    ReservedListDualDatabaseDao.save(reservedList);
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent())
+        .isTrue();
+    ReservedListDualDatabaseDao.delete(reservedList);
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent())
+        .isFalse();
+  }
+
+  @TestOfyAndSql
+  void testDelete_cloudSqlPrimary_success() {
+    fakeClock.advanceBy(Duration.standardDays(5));
+    ReservedListDualDatabaseDao.save(reservedList);
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent())
+        .isTrue();
+    ReservedListDualDatabaseDao.delete(reservedList);
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent())
+        .isFalse();
+  }
+
   @TestOfyAndSql
   void testSaveAndLoad_datastorePrimary_emptyList() {
     ReservedList list =

core/src/test/java/google/registry/model/registry/label/ReservedListSqlDaoTest.java

@@ -80,6 +80,21 @@ public class ReservedListSqlDaoTest {
             });
   }
 
+  @Test
+  void delete_worksSuccessfully() {
+    ReservedListSqlDao.save(testReservedList);
+    assertThat(ReservedListSqlDao.checkExists("testlist")).isTrue();
+    ReservedListSqlDao.delete(testReservedList);
+    assertThat(ReservedListSqlDao.checkExists("testlist")).isFalse();
+  }
+
+  @Test
+  void delete_listNotInDatabase() {
+    assertThat(ReservedListSqlDao.checkExists("testlist")).isFalse();
+    ReservedListSqlDao.delete(testReservedList);
+    assertThat(ReservedListSqlDao.checkExists("testlist")).isFalse();
+  }
+
   @Test
   void checkExists_worksSuccessfully() {
     assertThat(ReservedListSqlDao.checkExists("testlist")).isFalse();

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

@@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.ReservedList;
+import google.registry.model.registry.label.ReservedListDualDatabaseDao;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -40,7 +41,7 @@ class DeleteReservedListCommandTest extends CommandTestCase<DeleteReservedListCo
   void testSuccess() throws Exception {
     assertThat(reservedList.getReservedListEntries()).hasSize(1);
     runCommandForced("--name=common");
-    assertThat(ReservedList.get("common")).isEmpty();
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())).isEmpty();
   }
 
   @Test
@@ -62,7 +63,7 @@ class DeleteReservedListCommandTest extends CommandTestCase<DeleteReservedListCo
         assertThrows(
             IllegalArgumentException.class,
             () -> runCommandForced("--name=" + reservedList.getName()));
-    assertThat(ReservedList.get(reservedList.getName())).isPresent();
+    assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())).isPresent();
     assertThat(thrown)
         .hasMessageThat()
         .isEqualTo("Cannot delete reserved list because it is used on these tld(s): xn--q9jyb4c");