Add dual write for Registrar (#474)

* Add dual write for Registrar

* Use @AlsoLoad to set street field for Cloud SQL

* Change email body to use the new streetLine field

* Refactored the logic to handle street fields

* Simplify postLoad function

* Address comments

* Add a TODO to remove street

* Add test for onLoad and postLoad

* Rebase on master

core/src/main/java/google/registry/model/ImmutableObject.java

@@ -162,7 +162,10 @@ public abstract class ImmutableObject implements Cloneable {
       // values.
       Map<String, Object> result = new LinkedHashMap<>();
       for (Entry<Field, Object> entry : ModelUtils.getFieldValues(o).entrySet()) {
-        result.put(entry.getKey().getName(), toMapRecursive(entry.getValue()));
+        Field field = entry.getKey();
+        if (!field.isAnnotationPresent(IgnoredInDiffableMap.class)) {
+          result.put(field.getName(), toMapRecursive(entry.getValue()));
+        }
       }
       return result;
     } else if (o instanceof Map) {
@@ -191,6 +194,12 @@ public abstract class ImmutableObject implements Cloneable {
     }
   }
 
+  /** Marker to indicate that this filed should be ignored by {@link #toDiffableFieldMap}. */
+  @Documented
+  @Retention(RUNTIME)
+  @Target(FIELD)
+  protected @interface IgnoredInDiffableMap {}
+
   /** Returns a map of all object fields (including sensitive data) that's used to produce diffs. */
   @SuppressWarnings("unchecked")
   public Map<String, Object> toDiffableFieldMap() {

core/src/main/java/google/registry/model/eppcommon/Address.java

@@ -16,20 +16,24 @@ package google.registry.model.eppcommon;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.nullToEmpty;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.googlecode.objectify.annotation.AlsoLoad;
 import com.googlecode.objectify.annotation.Ignore;
-import com.googlecode.objectify.annotation.OnLoad;
 import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.model.JsonMapBuilder;
 import google.registry.model.Jsonifiable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.stream.Stream;
 import javax.persistence.Embeddable;
 import javax.persistence.MappedSuperclass;
+import javax.persistence.PostLoad;
 import javax.persistence.Transient;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlTransient;
@@ -53,15 +57,17 @@ import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 public class Address extends ImmutableObject implements Jsonifiable {
 
   /** The schema validation will enforce that this has 3 lines at most. */
+  // TODO(shicong): Remove this field after migration. We need to figure out how to generate same
+  // XML from streetLine[1,2,3].
   @XmlJavaTypeAdapter(NormalizedStringAdapter.class)
   @Transient
   List<String> street;
 
-  @Ignore String streetLine1;
+  @Ignore @XmlTransient @IgnoredInDiffableMap String streetLine1;
 
-  @Ignore String streetLine2;
+  @Ignore @XmlTransient @IgnoredInDiffableMap String streetLine2;
 
-  @Ignore String streetLine3;
+  @Ignore @XmlTransient @IgnoredInDiffableMap String streetLine3;
 
   @XmlJavaTypeAdapter(NormalizedStringAdapter.class)
   String city;
@@ -86,18 +92,6 @@ public class Address extends ImmutableObject implements Jsonifiable {
     }
   }
 
-  public String getStreetLine1() {
-    return streetLine1;
-  }
-
-  public String getStreetLine2() {
-    return streetLine2;
-  }
-
-  public String getStreetLine13() {
-    return streetLine3;
-  }
-
   public String getCity() {
     return city;
   }
@@ -144,6 +138,9 @@ public class Address extends ImmutableObject implements Jsonifiable {
           street == null || (!street.isEmpty() && street.size() <= 3),
           "Street address must have [1-3] lines: %s", street);
       getInstance().street = street;
+      getInstance().streetLine1 = street.get(0);
+      getInstance().streetLine2 = street.size() >= 2 ? street.get(1) : null;
+      getInstance().streetLine3 = street.size() == 3 ? street.get(2) : null;
       return this;
     }
 
@@ -171,8 +168,14 @@ public class Address extends ImmutableObject implements Jsonifiable {
     }
   }
 
-  @OnLoad
-  void setStreetForCloudSql() {
+  /**
+   * Sets {@link #streetLine1}, {@link #streetLine2} and {@link #streetLine3} after loading the
+   * entity from Datastore.
+   *
+   * <p>This callback method is used by Objectify to set streetLine[1,2,3] fields as they are not
+   * persisted in the Datastore. TODO(shicong): Delete this method after database migration.
+   */
+  void onLoad(@AlsoLoad("street") List<String> street) {
     if (street == null || street.size() == 0) {
       return;
     }
@@ -180,4 +183,22 @@ public class Address extends ImmutableObject implements Jsonifiable {
     streetLine2 = street.size() >= 2 ? street.get(1) : null;
     streetLine3 = street.size() >= 3 ? street.get(2) : null;
   }
+
+  /**
+   * Sets {@link #street} after loading the entity from Cloud SQL.
+   *
+   * <p>This callback method is used by Hibernate to set {@link #street} field as it is not
+   * persisted in Cloud SQL. We are doing this because the street list field is exposed by Address
+   * class and is used everywhere in our code base. Also, setting/reading a list of strings is more
+   * convenient.
+   */
+  @PostLoad
+  void postLoad() {
+    street =
+        streetLine1 == null
+            ? null
+            : Stream.of(streetLine1, streetLine2, streetLine3)
+                .filter(Objects::nonNull)
+                .collect(toImmutableList());
+  }
 }

core/src/main/java/google/registry/persistence/CurrencyToBillingMapUserType.java

@@ -29,20 +29,26 @@ public class CurrencyToBillingMapUserType extends MapUserType {
 
   @Override
   public Object toEntityTypeMap(Map<String, String> map) {
-    return map.entrySet().stream()
-        .collect(
-            toImmutableMap(
-                entry -> CurrencyUnit.of(entry.getKey()),
-                entry ->
-                    new BillingAccountEntry(CurrencyUnit.of(entry.getKey()), entry.getValue())));
+    return map == null
+        ? null
+        : map.entrySet().stream()
+            .collect(
+                toImmutableMap(
+                    entry -> CurrencyUnit.of(entry.getKey()),
+                    entry ->
+                        new BillingAccountEntry(
+                            CurrencyUnit.of(entry.getKey()), entry.getValue())));
   }
 
   @Override
   public Map<String, String> toDbSupportedMap(Object map) {
-    return ((Map<CurrencyUnit, BillingAccountEntry>) map)
-        .entrySet().stream()
-            .collect(
-                toImmutableMap(
-                    entry -> entry.getKey().getCode(), entry -> entry.getValue().getAccountId()));
+    return map == null
+        ? null
+        : ((Map<CurrencyUnit, BillingAccountEntry>) map)
+            .entrySet().stream()
+                .collect(
+                    toImmutableMap(
+                        entry -> entry.getKey().getCode(),
+                        entry -> entry.getValue().getAccountId()));
   }
 }

core/src/main/java/google/registry/schema/registrar/RegistrarDao.java

@@ -0,0 +1,73 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.registrar;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+
+import google.registry.model.registrar.Registrar;
+import java.util.Optional;
+
+/** Data access object for {@link Registrar}. */
+public class RegistrarDao {
+
+  private RegistrarDao() {}
+
+  /** Persists a new or updates an existing registrar in Cloud SQL. */
+  public static void saveNew(Registrar registrar) {
+    checkArgumentNotNull(registrar, "registrar must be specified");
+    jpaTm().transact(() -> jpaTm().getEntityManager().persist(registrar));
+  }
+
+  /** Updates an existing registrar in Cloud SQL, throws excpetion if it does not exist. */
+  public static void update(Registrar registrar) {
+    checkArgumentNotNull(registrar, "registrar must be specified");
+    jpaTm()
+        .transact(
+            () -> {
+              checkArgument(
+                  checkExists(registrar.getClientId()),
+                  "A registrar of this id does not exist: %s.",
+                  registrar.getClientId());
+              jpaTm().getEntityManager().merge(registrar);
+            });
+  }
+
+  /** Returns whether the registrar of the given id exists. */
+  public static boolean checkExists(String clientId) {
+    checkArgumentNotNull(clientId, "clientId must be specified");
+    return jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                        .getEntityManager()
+                        .createQuery(
+                            "SELECT 1 FROM Registrar WHERE clientIdentifier = :clientIdentifier",
+                            Integer.class)
+                        .setParameter("clientIdentifier", clientId)
+                        .setMaxResults(1)
+                        .getResultList()
+                        .size()
+                    > 0);
+  }
+
+  /** Loads the registrar by its id, returns empty if it doesn't exist. */
+  public static Optional<Registrar> load(String clientId) {
+    checkArgumentNotNull(clientId, "clientId must be specified");
+    return Optional.ofNullable(
+        jpaTm().transact(() -> jpaTm().getEntityManager().find(Registrar.class, clientId)));
+  }
+}

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

@@ -18,6 +18,7 @@ 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;
@@ -28,6 +29,7 @@ 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.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registry.Registry;
@@ -53,6 +55,8 @@ 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();
+
   @Parameter(
       description = "Client identifier of the registrar account",
       required = true)
@@ -458,4 +462,26 @@ 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

@@ -32,6 +32,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
 import google.registry.config.RegistryEnvironment;
 import google.registry.model.registrar.Registrar;
+import google.registry.schema.registrar.RegistrarDao;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -69,6 +70,11 @@ final class CreateRegistrarCommand extends CreateOrUpdateRegistrarCommand
     registrarState = Optional.ofNullable(registrarState).orElse(ACTIVE);
   }
 
+  @Override
+  void saveToCloudSql(Registrar registrar) {
+    RegistrarDao.saveNew(registrar);
+  }
+
   @Nullable
   @Override
   Registrar getOldRegistrar(final String clientId) {

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

@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.emptyToNull;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.DatastoreServiceUtils.getNameOrId;
@@ -45,9 +46,9 @@ import javax.annotation.Nullable;
 public abstract class MutatingCommand extends ConfirmingCommand implements CommandWithRemoteApi {
 
   /**
-   * A mutation of a specific entity, represented by an old and a new version of the entity.
-   * Storing the old version is necessary to enable checking that the existing entity has not been
-   * modified when applying a mutation that was created outside the same transaction.
+   * A mutation of a specific entity, represented by an old and a new version of the entity. Storing
+   * the old version is necessary to enable checking that the existing entity has not been modified
+   * when applying a mutation that was created outside the same transaction.
    */
   private static class EntityChange {
 
@@ -169,8 +170,8 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma
   }
 
   /**
-   * Returns a set of lists of EntityChange actions to commit.  Each list should be executed in
-   * order inside a single transaction.
+   * Returns a set of lists of EntityChange actions to commit. Each list should be executed in order
+   * inside a single transaction.
    */
   private ImmutableSet<ImmutableList<EntityChange>> getCollatedEntityChangeBatches() {
     ImmutableSet.Builder<ImmutableList<EntityChange>> batches = new ImmutableSet.Builder<>();
@@ -221,4 +222,11 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma
         ? "No entity changes to apply."
         : changedEntitiesMap.values().stream().map(Object::toString).collect(joining("\n"));
   }
+
+  /** Returns the collection of the new entity in the {@link EntityChange}. */
+  protected ImmutableList<ImmutableObject> getChangedEntities() {
+    return changedEntitiesMap.values().stream()
+        .map(entityChange -> entityChange.newEntity)
+        .collect(toImmutableList());
+  }
 }

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

@@ -20,6 +20,7 @@ import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import com.beust.jcommander.Parameters;
 import google.registry.config.RegistryEnvironment;
 import google.registry.model.registrar.Registrar;
+import google.registry.schema.registrar.RegistrarDao;
 import javax.annotation.Nullable;
 
 /** Command to update a Registrar. */
@@ -49,4 +50,9 @@ final class UpdateRegistrarCommand extends CreateOrUpdateRegistrarCommand {
               + " contact.");
     }
   }
+
+  @Override
+  void saveToCloudSql(Registrar registrar) {
+    RegistrarDao.update(registrar);
+  }
 }

core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java

@@ -119,8 +119,9 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
     // handler, registrar-settings really only supports read and update.
     String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read");
     @SuppressWarnings("unchecked")
-    Map<String, ?> args = (Map<String, Object>)
-        Optional.<Object>ofNullable(input.get(ARGS_PARAM)).orElse(ImmutableMap.of());
+    Map<String, ?> args =
+        (Map<String, Object>)
+            Optional.<Object>ofNullable(input.get(ARGS_PARAM)).orElse(ImmutableMap.of());
 
     logger.atInfo().log("Received request '%s' on registrar '%s' with args %s", op, clientId, args);
     String status = "SUCCESS";
@@ -296,8 +297,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
         .ifPresent(builder::setEmailAddress);
     builder.setPhoneNumber(
         RegistrarFormFields.PHONE_NUMBER_FIELD.extractUntyped(args).orElse(null));
-    builder.setFaxNumber(
-        RegistrarFormFields.FAX_NUMBER_FIELD.extractUntyped(args).orElse(null));
+    builder.setFaxNumber(RegistrarFormFields.FAX_NUMBER_FIELD.extractUntyped(args).orElse(null));
     builder.setLocalizedAddress(
         RegistrarFormFields.L10N_ADDRESS_FIELD.extractUntyped(args).orElse(null));
 
@@ -357,9 +357,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
    * <p>On success, returns {@code builder.build()}.
    */
   private Registrar checkNotChangedUnlessAllowed(
-      Registrar.Builder builder,
-      Registrar originalRegistrar,
-      Role allowedRole) {
+      Registrar.Builder builder, Registrar originalRegistrar, Role allowedRole) {
     Registrar updatedRegistrar =  builder.build();
     if (updatedRegistrar.equals(originalRegistrar)) {
       return updatedRegistrar;
@@ -374,9 +372,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
     }
     Map<?, ?> diffs =
         DiffUtils.deepDiff(
-            originalRegistrar.toDiffableFieldMap(),
-            updatedRegistrar.toDiffableFieldMap(),
-            true);
+            originalRegistrar.toDiffableFieldMap(), updatedRegistrar.toDiffableFieldMap(), true);
     throw new ForbiddenException(
         String.format("Unauthorized: only %s can change fields %s", allowedRole, diffs.keySet()));
   }
@@ -400,9 +396,10 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
     Set<String> emails = new HashSet<>();
     for (RegistrarContact contact : updatedContacts) {
       if (!emails.add(contact.getEmailAddress())) {
-        throw new ContactRequirementException(String.format(
-            "One email address (%s) cannot be used for multiple contacts",
-            contact.getEmailAddress()));
+        throw new ContactRequirementException(
+            String.format(
+                "One email address (%s) cannot be used for multiple contacts",
+                contact.getEmailAddress()));
       }
     }
     // Check that required contacts don't go away, once they are set.

core/src/test/java/google/registry/model/eppcommon/AddressTest.java

@@ -0,0 +1,79 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model.eppcommon;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link Address}. */
+@RunWith(JUnit4.class)
+public class AddressTest {
+  @Test
+  public void onLoad_setsIndividualStreetLinesSuccessfully() {
+    Address address = new Address();
+    address.onLoad(ImmutableList.of("line1", "line2", "line3"));
+    assertThat(address.streetLine1).isEqualTo("line1");
+    assertThat(address.streetLine2).isEqualTo("line2");
+    assertThat(address.streetLine3).isEqualTo("line3");
+  }
+
+  @Test
+  public void onLoad_setsOnlyNonNullStreetLines() {
+    Address address = new Address();
+    address.onLoad(ImmutableList.of("line1", "line2"));
+    assertThat(address.streetLine1).isEqualTo("line1");
+    assertThat(address.streetLine2).isEqualTo("line2");
+    assertThat(address.streetLine3).isNull();
+  }
+
+  @Test
+  public void onLoad_doNothingIfInputIsNull() {
+    Address address = new Address();
+    address.onLoad(null);
+    assertThat(address.streetLine1).isNull();
+    assertThat(address.streetLine2).isNull();
+    assertThat(address.streetLine3).isNull();
+  }
+
+  @Test
+  public void postLoad_setsStreetListSuccessfully() {
+    Address address = new Address();
+    address.streetLine1 = "line1";
+    address.streetLine2 = "line2";
+    address.streetLine3 = "line3";
+    address.postLoad();
+    assertThat(address.street).containsExactly("line1", "line2", "line3");
+  }
+
+  @Test
+  public void postLoad_setsOnlyNonNullStreetLines() {
+    Address address = new Address();
+    address.streetLine1 = "line1";
+    address.streetLine2 = "line2";
+    address.postLoad();
+    assertThat(address.street).containsExactly("line1", "line2");
+  }
+
+  @Test
+  public void postLoad_doNothingIfInputIsNull() {
+    Address address = new Address();
+    address.postLoad();
+    assertThat(address.street).isNull();
+  }
+}

core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java

@@ -19,14 +19,17 @@ import google.registry.model.domain.DomainBaseSqlTest;
 import google.registry.model.registry.RegistryLockDaoTest;
 import google.registry.persistence.transaction.JpaEntityCoverage;
 import google.registry.schema.cursor.CursorDaoTest;
+import google.registry.schema.registrar.RegistrarDaoTest;
 import google.registry.schema.server.LockDaoTest;
 import google.registry.schema.tld.PremiumListDaoTest;
 import google.registry.schema.tld.ReservedListDaoTest;
 import google.registry.schema.tmch.ClaimsListDaoTest;
+import google.registry.tools.CreateRegistrarCommandTest;
 import google.registry.tools.CreateReservedListCommandTest;
 import google.registry.tools.DomainLockUtilsTest;
 import google.registry.tools.LockDomainCommandTest;
 import google.registry.tools.UnlockDomainCommandTest;
+import google.registry.tools.UpdateRegistrarCommandTest;
 import google.registry.tools.UpdateReservedListCommandTest;
 import google.registry.tools.server.CreatePremiumListActionTest;
 import google.registry.tools.server.UpdatePremiumListActionTest;
@@ -54,6 +57,7 @@ import org.junit.runners.Suite.SuiteClasses;
 @SuiteClasses({
   ClaimsListDaoTest.class,
   CreatePremiumListActionTest.class,
+  CreateRegistrarCommandTest.class,
   CreateReservedListCommandTest.class,
   CursorDaoTest.class,
   DomainLockUtilsTest.class,
@@ -61,12 +65,14 @@ import org.junit.runners.Suite.SuiteClasses;
   LockDomainCommandTest.class,
   DomainBaseSqlTest.class,
   PremiumListDaoTest.class,
+  RegistrarDaoTest.class,
   RegistryLockDaoTest.class,
   RegistryLockGetActionTest.class,
   RegistryLockVerifyActionTest.class,
   ReservedListDaoTest.class,
   UnlockDomainCommandTest.class,
   UpdatePremiumListActionTest.class,
+  UpdateRegistrarCommandTest.class,
   UpdateReservedListCommandTest.class
 })
 public class SqlIntegrationTestSuite {

core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java

@@ -0,0 +1,104 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.registrar;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import google.registry.model.EntityTestCase;
+import google.registry.model.registrar.Registrar;
+import google.registry.model.registrar.RegistrarAddress;
+import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
+import google.registry.testing.FakeClock;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit tests for {@link RegistrarDao}. */
+@RunWith(JUnit4.class)
+public class RegistrarDaoTest extends EntityTestCase {
+  private final FakeClock fakeClock = new FakeClock();
+
+  @Rule
+  public final JpaIntegrationWithCoverageRule jpaRule =
+      new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationWithCoverageRule();
+
+  private Registrar testRegistrar;
+
+  @Before
+  public void setUp() {
+    testRegistrar =
+        new Registrar.Builder()
+            .setType(Registrar.Type.TEST)
+            .setClientId("registrarId")
+            .setRegistrarName("registrarName")
+            .setLocalizedAddress(
+                new RegistrarAddress.Builder()
+                    .setStreet(ImmutableList.of("123 Example Boulevard."))
+                    .setCity("Williamsburg")
+                    .setState("NY")
+                    .setZip("11211")
+                    .setCountryCode("US")
+                    .build())
+            .build();
+  }
+
+  @Test
+  public void saveNew_worksSuccessfully() {
+    assertThat(RegistrarDao.checkExists("registrarId")).isFalse();
+    RegistrarDao.saveNew(testRegistrar);
+    assertThat(RegistrarDao.checkExists("registrarId")).isTrue();
+  }
+
+  @Test
+  public void update_worksSuccessfully() {
+    RegistrarDao.saveNew(testRegistrar);
+    Registrar persisted = RegistrarDao.load("registrarId").get();
+    assertThat(persisted.getRegistrarName()).isEqualTo("registrarName");
+    RegistrarDao.update(persisted.asBuilder().setRegistrarName("changedRegistrarName").build());
+    persisted = RegistrarDao.load("registrarId").get();
+    assertThat(persisted.getRegistrarName()).isEqualTo("changedRegistrarName");
+  }
+
+  @Test
+  public void update_throwsExceptionWhenEntityDoesNotExist() {
+    assertThat(RegistrarDao.checkExists("registrarId")).isFalse();
+    assertThrows(IllegalArgumentException.class, () -> RegistrarDao.update(testRegistrar));
+  }
+
+  @Test
+  public void load_worksSuccessfully() {
+    assertThat(RegistrarDao.checkExists("registrarId")).isFalse();
+    RegistrarDao.saveNew(testRegistrar);
+    Registrar persisted = RegistrarDao.load("registrarId").get();
+
+    assertThat(persisted.getClientId()).isEqualTo("registrarId");
+    assertThat(persisted.getRegistrarName()).isEqualTo("registrarName");
+    assertThat(persisted.getCreationTime()).isEqualTo(fakeClock.nowUtc());
+    assertThat(persisted.getLocalizedAddress())
+        .isEqualTo(
+            new RegistrarAddress.Builder()
+                .setStreet(ImmutableList.of("123 Example Boulevard."))
+                .setCity("Williamsburg")
+                .setState("NY")
+                .setZip("11211")
+                .setCountryCode("US")
+                .build());
+  }
+}

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

@@ -33,12 +33,17 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Range;
 import com.google.common.net.MediaType;
 import google.registry.model.registrar.Registrar;
+import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
+import google.registry.schema.registrar.RegistrarDao;
 import google.registry.testing.CertificateSamples;
+import google.registry.testing.FakeClock;
 import java.io.IOException;
 import java.util.Optional;
 import org.joda.money.CurrencyUnit;
 import org.joda.time.DateTime;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
@@ -46,6 +51,12 @@ import org.mockito.Mock;
 /** Unit tests for {@link CreateRegistrarCommand}. */
 public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand> {
 
+  private final FakeClock fakeClock = new FakeClock();
+
+  @Rule
+  public final JpaIntegrationWithCoverageRule jpaRule =
+      new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationWithCoverageRule();
+
   @Mock private AppEngineConnection connection;
 
   @Before
@@ -100,6 +111,28 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
             eq(new byte[0]));
   }
 
+  @Test
+  public 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(RegistrarDao.checkExists("clientz")).isTrue();
+  }
+
   @Test
   public void testSuccess_quotedPassword() throws Exception {
     runCommandForced(

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

@@ -32,16 +32,36 @@ import com.google.common.collect.ImmutableSet;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
+import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
+import google.registry.schema.registrar.RegistrarDao;
 import google.registry.testing.AppEngineRule;
+import google.registry.testing.FakeClock;
 import google.registry.util.CidrAddressBlock;
 import java.util.Optional;
 import org.joda.money.CurrencyUnit;
 import org.joda.time.DateTime;
+import org.junit.Rule;
 import org.junit.Test;
 
 /** Unit tests for {@link UpdateRegistrarCommand}. */
 public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand> {
 
+  private final FakeClock fakeClock = new FakeClock();
+
+  @Rule
+  public final JpaIntegrationWithCoverageRule jpaRule =
+      new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationWithCoverageRule();
+
+  @Test
+  public void testSuccess_alsoUpdateInCloudSql() throws Exception {
+    assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isFalse();
+    RegistrarDao.saveNew(loadRegistrar("NewRegistrar"));
+    runCommand("--password=some_password", "--force", "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isTrue();
+    assertThat(RegistrarDao.load("NewRegistrar").get().verifyPassword("some_password")).isTrue();
+  }
+
   @Test
   public void testSuccess_password() throws Exception {
     assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isFalse();