From d77511c36d882911eeef7682edf33dcbd48c87bd Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Fri, 5 Feb 2021 11:06:14 -0500 Subject: [PATCH] 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. --- .../tools/CreateOrUpdateRegistrarCommand.java | 26 ------------------- .../tools/CreateRegistrarCommand.java | 6 ----- .../tools/UpdateRegistrarCommand.java | 6 ----- .../tools/CreateRegistrarCommandTest.java | 23 ---------------- .../tools/UpdateRegistrarCommandTest.java | 15 ----------- 5 files changed, 76 deletions(-) diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 39dda3776..e2e304fc4 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/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); } diff --git a/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java index 210dbce22..67a7203fa 100644 --- a/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java +++ b/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) { diff --git a/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java index c31b665f6..04beffd87 100644 --- a/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java +++ b/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); - } } diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index 29d5defd5..78b6f2d4f 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/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 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.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( diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 06ec102fe..d09da3866 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/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 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();