Remove dual-write of registrar from tool commands (#952)

* Remove dual-write of registrar from tool commands

As discussed, we're keeping registrar in the "replicated" category.

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

@@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Predicates.isNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
 import static google.registry.util.RegistrarUtils.normalizeRegistrarName;
 import static java.nio.charset.StandardCharsets.US_ASCII;
@@ -29,7 +28,6 @@ import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
-import com.google.common.flogger.FluentLogger;
 import google.registry.flows.certs.CertificateChecker;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarAddress;
@@ -57,8 +55,6 @@ import org.joda.time.DateTime;
 /** Shared base class for commands to create or update a {@link Registrar}. */
 abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
 
-  static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
   @Inject CertificateChecker certificateChecker;
 
   @Parameter(description = "Client identifier of the registrar account", required = true)
@@ -457,26 +453,4 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       stageEntityChange(oldRegistrar, newRegistrar);
     }
   }
-
-  @Override
-  protected String execute() throws Exception {
-    // Save registrar to Datastore and output its response
-    logger.atInfo().log(super.execute());
-
-    String cloudSqlMessage;
-    try {
-      jpaTm()
-          .transact(
-              () ->
-                  getChangedEntities().forEach(newEntity -> saveToCloudSql((Registrar) newEntity)));
-      cloudSqlMessage =
-          String.format("Updated %d entities in Cloud SQL.\n", getChangedEntities().size());
-    } catch (Throwable t) {
-      cloudSqlMessage = "Unexpected error saving registrar to Cloud SQL from nomulus tool command";
-      logger.atSevere().withCause(t).log(cloudSqlMessage);
-    }
-    return cloudSqlMessage;
-  }
-
-  abstract void saveToCloudSql(Registrar registrar);
 }

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

@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static google.registry.model.registrar.Registrar.State.ACTIVE;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.tools.RegistryToolEnvironment.PRODUCTION;
 import static google.registry.tools.RegistryToolEnvironment.SANDBOX;
 import static google.registry.tools.RegistryToolEnvironment.UNITTEST;
@@ -70,11 +69,6 @@ final class CreateRegistrarCommand extends CreateOrUpdateRegistrarCommand
     registrarState = Optional.ofNullable(registrarState).orElse(ACTIVE);
   }
 
-  @Override
-  void saveToCloudSql(Registrar registrar) {
-    jpaTm().insert(registrar);
-  }
-
   @Nullable
   @Override
   Registrar getOldRegistrar(final String clientId) {

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

@@ -14,7 +14,6 @@
 
 package google.registry.tools;
 
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
@@ -50,9 +49,4 @@ final class UpdateRegistrarCommand extends CreateOrUpdateRegistrarCommand {
               + " contact.");
     }
   }
-
-  @Override
-  void saveToCloudSql(Registrar registrar) {
-    jpaTm().update(registrar);
-  }
 }

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

@@ -17,7 +17,6 @@ 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.ofy.ObjectifyService.ofy;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
@@ -113,28 +112,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
             eq(new byte[0]));
   }
 
-  @Test
-  void testSuccess_alsoSaveToCloudSql() throws Exception {
-    runCommandForced(
-        "--name=blobio",
-        "--password=\"some_password\"",
-        "--registrar_type=REAL",
-        "--iana_id=8",
-        "--passcode=01234",
-        "--icann_referral_email=foo@bar.test",
-        "--street=\"123 Fake St\"",
-        "--city Fakington",
-        "--state MA",
-        "--zip 00351",
-        "--cc US",
-        "clientz");
-
-    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isPresent();
-    assertThat(registrar.get().verifyPassword("some_password")).isTrue();
-    assertThat(jpaTm().transact(() -> jpaTm().exists(registrar.get()))).isTrue();
-  }
-
   @Test
   void testSuccess_quotedPassword() throws Exception {
     runCommandForced(

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

@@ -16,7 +16,6 @@ package google.registry.tools;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
@@ -37,7 +36,6 @@ import google.registry.flows.certs.CertificateChecker.InsecureCertificateExcepti
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
-import google.registry.persistence.VKey;
 import google.registry.testing.AppEngineExtension;
 import google.registry.util.CidrAddressBlock;
 import java.util.Optional;
@@ -60,19 +58,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
             fakeClock);
   }
 
-  @Test
-  void testSuccess_alsoUpdateInCloudSql() throws Exception {
-    assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isFalse();
-    jpaTm().transact(() -> jpaTm().insert(loadRegistrar("NewRegistrar")));
-    runCommand("--password=some_password", "--force", "NewRegistrar");
-    assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isTrue();
-    assertThat(
-            jpaTm()
-                .transact(() -> jpaTm().loadByKey(VKey.createSql(Registrar.class, "NewRegistrar")))
-                .verifyPassword("some_password"))
-        .isTrue();
-  }
-
   @Test
   void testSuccess_password() throws Exception {
     assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isFalse();