From b9c40648d077c99799d7fa35b7820dce3ec7989d Mon Sep 17 00:00:00 2001 From: Shicong Huang Date: Thu, 13 Feb 2020 16:00:21 -0500 Subject: [PATCH] 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 --- .../registry/model/ImmutableObject.java | 11 +- .../registry/model/eppcommon/Address.java | 57 +++++++--- .../CurrencyToBillingMapUserType.java | 28 +++-- .../schema/registrar/RegistrarDao.java | 73 ++++++++++++ .../tools/CreateOrUpdateRegistrarCommand.java | 26 +++++ .../tools/CreateRegistrarCommand.java | 6 + .../registry/tools/MutatingCommand.java | 18 ++- .../tools/UpdateRegistrarCommand.java | 6 + .../registrar/RegistrarSettingsAction.java | 23 ++-- .../registry/model/eppcommon/AddressTest.java | 79 +++++++++++++ .../integration/SqlIntegrationTestSuite.java | 6 + .../schema/registrar/RegistrarDaoTest.java | 104 ++++++++++++++++++ .../tools/CreateRegistrarCommandTest.java | 33 ++++++ .../tools/UpdateRegistrarCommandTest.java | 20 ++++ 14 files changed, 442 insertions(+), 48 deletions(-) create mode 100644 core/src/main/java/google/registry/schema/registrar/RegistrarDao.java create mode 100644 core/src/test/java/google/registry/model/eppcommon/AddressTest.java create mode 100644 core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java diff --git a/core/src/main/java/google/registry/model/ImmutableObject.java b/core/src/main/java/google/registry/model/ImmutableObject.java index ac8e5fc8c..b17af27ea 100644 --- a/core/src/main/java/google/registry/model/ImmutableObject.java +++ b/core/src/main/java/google/registry/model/ImmutableObject.java @@ -162,7 +162,10 @@ public abstract class ImmutableObject implements Cloneable { // values. Map result = new LinkedHashMap<>(); for (Entry 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 toDiffableFieldMap() { diff --git a/core/src/main/java/google/registry/model/eppcommon/Address.java b/core/src/main/java/google/registry/model/eppcommon/Address.java index d3cd34d5d..a12e93522 100644 --- a/core/src/main/java/google/registry/model/eppcommon/Address.java +++ b/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 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. + * + *

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 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. + * + *

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()); + } } diff --git a/core/src/main/java/google/registry/persistence/CurrencyToBillingMapUserType.java b/core/src/main/java/google/registry/persistence/CurrencyToBillingMapUserType.java index 072ec63d3..bbbd2d269 100644 --- a/core/src/main/java/google/registry/persistence/CurrencyToBillingMapUserType.java +++ b/core/src/main/java/google/registry/persistence/CurrencyToBillingMapUserType.java @@ -29,20 +29,26 @@ public class CurrencyToBillingMapUserType extends MapUserType { @Override public Object toEntityTypeMap(Map 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 toDbSupportedMap(Object map) { - return ((Map) map) - .entrySet().stream() - .collect( - toImmutableMap( - entry -> entry.getKey().getCode(), entry -> entry.getValue().getAccountId())); + return map == null + ? null + : ((Map) map) + .entrySet().stream() + .collect( + toImmutableMap( + entry -> entry.getKey().getCode(), + entry -> entry.getValue().getAccountId())); } } diff --git a/core/src/main/java/google/registry/schema/registrar/RegistrarDao.java b/core/src/main/java/google/registry/schema/registrar/RegistrarDao.java new file mode 100644 index 000000000..3b80ab7a6 --- /dev/null +++ b/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 load(String clientId) { + checkArgumentNotNull(clientId, "clientId must be specified"); + return Optional.ofNullable( + jpaTm().transact(() -> jpaTm().getEntityManager().find(Registrar.class, clientId))); + } +} diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 8d5c819bf..8f3b9ccf7 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/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); } diff --git a/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java index 67a7203fa..bce9d9253 100644 --- a/core/src/main/java/google/registry/tools/CreateRegistrarCommand.java +++ b/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) { diff --git a/core/src/main/java/google/registry/tools/MutatingCommand.java b/core/src/main/java/google/registry/tools/MutatingCommand.java index 4cf4e8e1c..1ec5d125b 100644 --- a/core/src/main/java/google/registry/tools/MutatingCommand.java +++ b/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> getCollatedEntityChangeBatches() { ImmutableSet.Builder> 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 getChangedEntities() { + return changedEntitiesMap.values().stream() + .map(entityChange -> entityChange.newEntity) + .collect(toImmutableList()); + } } diff --git a/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java index 04beffd87..2af8c5744 100644 --- a/core/src/main/java/google/registry/tools/UpdateRegistrarCommand.java +++ b/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); + } } diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 6334f6b93..040e4a2e1 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/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 args = (Map) - Optional.ofNullable(input.get(ARGS_PARAM)).orElse(ImmutableMap.of()); + Map args = + (Map) + Optional.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 *

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 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. diff --git a/core/src/test/java/google/registry/model/eppcommon/AddressTest.java b/core/src/test/java/google/registry/model/eppcommon/AddressTest.java new file mode 100644 index 000000000..a85029a86 --- /dev/null +++ b/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(); + } +} diff --git a/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java b/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java index 038541a8a..a1a622bbb 100644 --- a/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java +++ b/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 { diff --git a/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java b/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java new file mode 100644 index 000000000..bd803edd6 --- /dev/null +++ b/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()); + } +} diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index 24725a989..4cfee6946 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/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 { + 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 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( diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 9612402ef..39c185de3 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/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 { + 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();