From d28debf149bdc7e56cfcce67ac9bd19bcc5b99d6 Mon Sep 17 00:00:00 2001 From: Shicong Huang Date: Wed, 15 Jan 2020 13:10:50 -0500 Subject: [PATCH] Relax reserved list existence check to allow Cloud SQL migration (#429) --- .../registry/tools/UpdateReservedListCommand.java | 14 ++++++++++---- .../tools/UpdateReservedListCommandTest.java | 9 +++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java index a15797063..752f41887 100644 --- a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java @@ -62,10 +62,16 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand jpaTm() .transact( () -> { - checkArgument( - ReservedListDao.checkExists(cloudSqlReservedList.getName()), - "A reserved list of this name doesn't exist: %s.", - cloudSqlReservedList.getName()); + // This check is currently disabled because, during the Cloud SQL migration, we need + // to be able to update reserved lists in Datastore while simultaneously creating + // their first revision in Cloud SQL (i.e. if they haven't been migrated over yet). + // TODO(shicong): Re-instate this once all reserved lists are migrated to Cloud SQL, + // and add a unit test to verity that an exception will be thrown if + // the reserved list doesn't exist. + // checkArgument( + // ReservedListDao.checkExists(cloudSqlReservedList.getName()), + // "A reserved list of this name doesn't exist: %s.", + // cloudSqlReservedList.getName()); ReservedListDao.save(cloudSqlReservedList); }); } diff --git a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java index fc236fa72..f62c94c00 100644 --- a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java @@ -123,13 +123,14 @@ public class UpdateReservedListCommandTest extends } @Test - public void testSaveToCloudSql_noExceptionThrownWhenSaveFail() throws Exception { - // Note that, during the dual-write phase, we want to make sure that no exception will be - // thrown if saving reserved list to Cloud SQL fails. + public void testSaveToCloudSql_succeedsEvenPreviousListNotExist() throws Exception { + // Note that, during the dual-write phase, we just always save the revered list to + // Cloud SQL (if --also_cloud_sql is set) without checking if there is a list with + // same name. This is to backfill the existing list in Datastore when we update it. populateInitialReservedListInDatastore(true); runCommandForced( "--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath, "--also_cloud_sql"); verifyXnq9jyb4cInDatastore(); - assertThat(ReservedListDao.checkExists("xn--q9jyb4c_common-reserved")).isFalse(); + assertThat(ReservedListDao.checkExists("xn--q9jyb4c_common-reserved")).isTrue(); } }