From 1e76eeed37efecb759160898f53d877959ef5ad3 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Fri, 22 Apr 2022 16:33:18 -0400 Subject: [PATCH] Validate that a registrar has billing accounts for all its allowed TLDs (#1601) This will require edits to a substantial number of registrars on sandbox (nearly all of them) because almost all of them have access to at least one TLD, but almost none of them have any billing accounts set. Until this is set, any updates to the existing registrars that aren't adding the billing accounts will cause failures. Unfortunately, there wasn't any less invasive foolproof way to implement this change, and we already had one attempt to implement it on create registrar command that wasn't working (because allowed TLDs tend not to be added on initial registrar creation, but rather, afterwards as an update). --- .../registry/model/OteAccountBuilder.java | 14 ++++ .../registry/model/registrar/Registrar.java | 25 +++++-- .../google/registry/model/tld/Registries.java | 4 +- .../google/registry/model/tld/Registry.java | 14 ++-- .../tools/CreateOrUpdateRegistrarCommand.java | 20 ------ .../google/registry/tools/RegistryTool.java | 2 + .../registry/tools/RegistryToolComponent.java | 3 + ...ckfillRegistrarBillingAccountsCommand.java | 56 +++++++++++++++ .../export/sheet/SyncRegistrarsSheetTest.java | 6 +- .../model/registrar/RegistrarTest.java | 72 ++++++++++++++++++- .../registry/model/tld/RegistryTest.java | 2 +- .../registry/testing/AppEngineExtension.java | 2 + .../registry/testing/DatabaseHelper.java | 1 + .../tools/CreateRegistrarCommandTest.java | 29 ++++++-- .../tools/UpdateRegistrarCommandTest.java | 34 ++++++++- 15 files changed, 238 insertions(+), 46 deletions(-) create mode 100644 core/src/main/java/google/registry/tools/javascrap/BackfillRegistrarBillingAccountsCommand.java diff --git a/core/src/main/java/google/registry/model/OteAccountBuilder.java b/core/src/main/java/google/registry/model/OteAccountBuilder.java index d4d2389fa..8d9008ecf 100644 --- a/core/src/main/java/google/registry/model/OteAccountBuilder.java +++ b/core/src/main/java/google/registry/model/OteAccountBuilder.java @@ -38,6 +38,7 @@ import google.registry.model.registrar.RegistrarAddress; import google.registry.model.registrar.RegistrarContact; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; +import google.registry.model.tld.Registry.TldType; import google.registry.model.tld.label.PremiumList; import google.registry.model.tld.label.PremiumListDao; import google.registry.persistence.VKey; @@ -111,6 +112,17 @@ public final class OteAccountBuilder { DateTime.parse("2030-03-01T00:00:00Z"), Money.of(CurrencyUnit.USD, 0)); + /** + * The default billing account map applied to all OT&E registrars. + * + *

This contains dummy values for USD and JPY so that OT&E registrars can be granted access + * to all existing TLDs in sandbox. Note that OT&E is only on sandbox and thus these dummy + * values will never be used in production (the only environment where real invoicing takes + * place). + */ + public static final ImmutableMap DEFAULT_BILLING_ACCOUNT_MAP = + ImmutableMap.of(CurrencyUnit.USD, "123", CurrencyUnit.JPY, "456"); + private final ImmutableMap registrarIdToTld; private final Registry sunriseTld; private final Registry gaTld; @@ -305,6 +317,7 @@ public final class OteAccountBuilder { .setTldStateTransitions(ImmutableSortedMap.of(START_OF_TIME, initialTldState)) .setDnsWriters(ImmutableSet.of("VoidDnsWriter")) .setPremiumList(premiumList.get()) + .setTldType(TldType.TEST) .setRoidSuffix( String.format( "%S%X", @@ -334,6 +347,7 @@ public final class OteAccountBuilder { .setPhoneNumber("+1.2125550100") .setIcannReferralEmail("nightmare@registrar.test") .setState(Registrar.State.ACTIVE) + .setBillingAccountMap(DEFAULT_BILLING_ACCOUNT_MAP) .build(); } diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index 69e71ac44..d06c9a233 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -81,6 +81,7 @@ import google.registry.model.common.EntityGroupRoot; import google.registry.model.registrar.Registrar.BillingAccountEntry.CurrencyMapper; import google.registry.model.replay.DatastoreAndSqlEntity; import google.registry.model.tld.Registry; +import google.registry.model.tld.Registry.TldType; import google.registry.persistence.VKey; import google.registry.util.CidrAddressBlock; import java.security.cert.CertificateParsingException; @@ -798,13 +799,9 @@ public class Registrar extends ImmutableObject } public Builder setBillingAccountMap(@Nullable Map billingAccountMap) { - if (billingAccountMap == null) { - getInstance().billingAccountMap = null; - } else { - getInstance().billingAccountMap = - billingAccountMap.entrySet().stream() - .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new)); - } + getInstance().billingAccountMap = + nullToEmptyImmutableCopy(billingAccountMap).entrySet().stream() + .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new)); return this; } @@ -1015,6 +1012,20 @@ public class Registrar extends ImmutableObject String.format( "Supplied IANA ID is not valid for %s registrar type: %s", getInstance().type, getInstance().ianaIdentifier)); + + // In order to grant access to real TLDs, the registrar must have a corresponding billing + // account ID for that TLD's billing currency. + ImmutableSet nonBillableTlds = + Registry.get(getInstance().getAllowedTlds()).stream() + .filter(r -> r.getTldType() == TldType.REAL) + .filter(r -> !getInstance().getBillingAccountMap().containsKey(r.getCurrency())) + .map(Registry::getTldStr) + .collect(toImmutableSet()); + checkArgument( + nonBillableTlds.isEmpty(), + "Cannot set these allowed, real TLDs because their currency is missing " + + "from the billing account map: %s", + nonBillableTlds); return cloneEmptyToNull(super.build()); } } diff --git a/core/src/main/java/google/registry/model/tld/Registries.java b/core/src/main/java/google/registry/model/tld/Registries.java index 40cadae87..385810dd1 100644 --- a/core/src/main/java/google/registry/model/tld/Registries.java +++ b/core/src/main/java/google/registry/model/tld/Registries.java @@ -72,7 +72,7 @@ public final class Registries { .stream() .map(Key::getName) .collect(toImmutableSet()); - return Registry.getAll(tlds).stream() + return Registry.get(tlds).stream() .map(e -> Maps.immutableEntry(e.getTldStr(), e.getTldType())) .collect(entriesToImmutableMap()); } else { @@ -105,7 +105,7 @@ public final class Registries { /** Returns the Registry entities themselves of the given type loaded fresh from Datastore. */ public static ImmutableSet getTldEntitiesOfType(TldType type) { - return Registry.getAll(filterValues(cache.get(), equalTo(type)).keySet()); + return Registry.get(filterValues(cache.get(), equalTo(type)).keySet()); } /** Pass-through check that the specified TLD exists, otherwise throw an IAE. */ diff --git a/core/src/main/java/google/registry/model/tld/Registry.java b/core/src/main/java/google/registry/model/tld/Registry.java index e695c6028..4a947ca2a 100644 --- a/core/src/main/java/google/registry/model/tld/Registry.java +++ b/core/src/main/java/google/registry/model/tld/Registry.java @@ -142,10 +142,16 @@ public class Registry extends ImmutableObject /** The type of TLD, which determines things like backups and escrow policy. */ public enum TldType { - /** A real, official TLD. */ + /** + * A real, official TLD (but not necessarily only on production). + * + *

Note that, to avoid unnecessary costly DB writes, {@link + * google.registry.model.reporting.DomainTransactionRecord}s are only written out for REAL TLDs + * (these transaction records are only used for ICANN reporting purposes). + */ REAL, - /** A test TLD, for the prober. */ + /** A test TLD, for the prober, OT&E, and other testing purposes. */ TEST } @@ -232,7 +238,7 @@ public class Registry extends ImmutableObject } /** Returns the registry entities for the given TLD strings, throwing if any don't exist. */ - static ImmutableSet getAll(Set tlds) { + public static ImmutableSet get(Set tlds) { try { ImmutableMap> registries = CACHE.getAll(tlds); ImmutableSet missingRegistries = @@ -1000,7 +1006,7 @@ public class Registry extends ImmutableObject instance.renewBillingCostTransitions.checkValidity(); instance.eapFeeSchedule.checkValidity(); // All costs must be in the expected currency. - // TODO(b/21854155): When we move PremiumList into Datastore, verify its currency too. + checkArgumentNotNull(instance.getCurrency(), "Currency must be set"); checkArgument( instance.getStandardCreateCost().getCurrencyUnit().equals(instance.currency), "Create cost must be in the registry's currency"); diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 65863fbce..dc592a5bb 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -24,14 +24,11 @@ import static java.nio.charset.StandardCharsets.US_ASCII; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; -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 google.registry.flows.certs.CertificateChecker; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; -import google.registry.model.tld.Registry; import google.registry.tools.params.KeyValueMapParameter.CurrencyUnitToStringMap; import google.registry.tools.params.OptionalLongParameter; import google.registry.tools.params.OptionalPhoneNumberParameter; @@ -46,7 +43,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import javax.annotation.Nullable; import javax.inject.Inject; import org.joda.money.CurrencyUnit; @@ -433,22 +429,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { // Require a phone passcode. checkArgument( newRegistrar.getPhonePasscode() != null, "--passcode is required for REAL registrars."); - // Check if registrar has billing account IDs for the currency of the TLDs that it is - // allowed to register. - ImmutableSet tldCurrencies = - newRegistrar - .getAllowedTlds() - .stream() - .map(tld -> Registry.get(tld).getCurrency()) - .collect(toImmutableSet()); - Set currenciesWithoutBillingAccountId = - newRegistrar.getBillingAccountMap() == null - ? tldCurrencies - : Sets.difference(tldCurrencies, newRegistrar.getBillingAccountMap().keySet()); - checkArgument( - currenciesWithoutBillingAccountId.isEmpty(), - "Need billing account map entries for currencies: %s", - Joiner.on(' ').join(currenciesWithoutBillingAccountId)); } stageEntityChange(oldRegistrar, newRegistrar); diff --git a/core/src/main/java/google/registry/tools/RegistryTool.java b/core/src/main/java/google/registry/tools/RegistryTool.java index 54e64e459..c24261423 100644 --- a/core/src/main/java/google/registry/tools/RegistryTool.java +++ b/core/src/main/java/google/registry/tools/RegistryTool.java @@ -15,6 +15,7 @@ package google.registry.tools; import com.google.common.collect.ImmutableMap; +import google.registry.tools.javascrap.BackfillRegistrarBillingAccountsCommand; import google.registry.tools.javascrap.CompareEscrowDepositsCommand; import google.registry.tools.javascrap.HardDeleteHostCommand; @@ -30,6 +31,7 @@ public final class RegistryTool { public static final ImmutableMap> COMMAND_MAP = new ImmutableMap.Builder>() .put("ack_poll_messages", AckPollMessagesCommand.class) + .put("backfill_registrar_billing_accounts", BackfillRegistrarBillingAccountsCommand.class) .put("canonicalize_labels", CanonicalizeLabelsCommand.class) .put("check_domain", CheckDomainCommand.class) .put("check_domain_claims", CheckDomainClaimsCommand.class) diff --git a/core/src/main/java/google/registry/tools/RegistryToolComponent.java b/core/src/main/java/google/registry/tools/RegistryToolComponent.java index d882505a2..3945d356b 100644 --- a/core/src/main/java/google/registry/tools/RegistryToolComponent.java +++ b/core/src/main/java/google/registry/tools/RegistryToolComponent.java @@ -43,6 +43,7 @@ import google.registry.request.Modules.UrlConnectionServiceModule; import google.registry.request.Modules.UrlFetchServiceModule; import google.registry.request.Modules.UserServiceModule; import google.registry.tools.AuthModule.LocalCredentialModule; +import google.registry.tools.javascrap.BackfillRegistrarBillingAccountsCommand; import google.registry.tools.javascrap.CompareEscrowDepositsCommand; import google.registry.tools.javascrap.HardDeleteHostCommand; import google.registry.util.UtilsModule; @@ -90,6 +91,8 @@ import javax.inject.Singleton; interface RegistryToolComponent { void inject(AckPollMessagesCommand command); + void inject(BackfillRegistrarBillingAccountsCommand command); + void inject(CheckDomainClaimsCommand command); void inject(CheckDomainCommand command); diff --git a/core/src/main/java/google/registry/tools/javascrap/BackfillRegistrarBillingAccountsCommand.java b/core/src/main/java/google/registry/tools/javascrap/BackfillRegistrarBillingAccountsCommand.java new file mode 100644 index 000000000..24afd47f1 --- /dev/null +++ b/core/src/main/java/google/registry/tools/javascrap/BackfillRegistrarBillingAccountsCommand.java @@ -0,0 +1,56 @@ +// Copyright 2021 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.tools.javascrap; + +import static com.google.common.base.Preconditions.checkState; +import static google.registry.model.OteAccountBuilder.DEFAULT_BILLING_ACCOUNT_MAP; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; + +import com.beust.jcommander.Parameters; +import google.registry.config.RegistryEnvironment; +import google.registry.model.registrar.Registrar; +import google.registry.tools.CommandWithRemoteApi; + +/** + * Backfills the billing account maps on all Registrars that don't have any set. + * + *

This should not (and cannot) be used on production. Its purpose is to backfill these values on + * sandbox, where most registrars have an empty billing account map, including all that were created + * for OT&E. The same default billing account map is used for all registrars, and includes + * values for USD and JPY. The actual values here don't matter as we don't do invoicing on sandbox + * anyway. + */ +@Parameters(separators = " =", commandDescription = "Backfill registrar billing account maps.") +public class BackfillRegistrarBillingAccountsCommand implements CommandWithRemoteApi { + + @Override + public void run() throws Exception { + checkState( + RegistryEnvironment.get() != RegistryEnvironment.PRODUCTION, + "Do not run this on production"); + System.out.println("Populating billing account maps on all registrars missing them ..."); + tm().transact( + () -> + tm().loadAllOfStream(Registrar.class) + .filter(r -> r.getBillingAccountMap().isEmpty()) + .forEach( + r -> + tm().update( + r.asBuilder() + .setBillingAccountMap(DEFAULT_BILLING_ACCOUNT_MAP) + .build()))); + System.out.println("Done!"); + } +} diff --git a/core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java b/core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java index 03149667c..9934d15d4 100644 --- a/core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java +++ b/core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java @@ -336,7 +336,11 @@ public class SyncRegistrarsSheetTest { @TestOfyAndSql void testRun_missingValues_stillWorks() throws Exception { - persistNewRegistrar("SomeRegistrar", "Some Registrar", Registrar.Type.REAL, 8L); + persistResource( + persistNewRegistrar("SomeRegistrar", "Some Registrar", Registrar.Type.REAL, 8L) + .asBuilder() + .setBillingAccountMap(ImmutableMap.of()) + .build()); newSyncRegistrarsSheet().run("foobar"); diff --git a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java index 09cf69081..599eb9b85 100644 --- a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java +++ b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java @@ -30,11 +30,14 @@ import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistSimpleResource; import static google.registry.testing.DatabaseHelper.persistSimpleResources; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.joda.money.CurrencyUnit.JPY; +import static org.joda.money.CurrencyUnit.USD; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.ImmutableSortedSet; import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig; @@ -42,13 +45,17 @@ import google.registry.model.EntityTestCase; import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.Type; import google.registry.model.tld.Registries; +import google.registry.model.tld.Registry; +import google.registry.model.tld.Registry.TldType; import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestOfyAndSql; import google.registry.testing.TestOfyOnly; import google.registry.testing.TestSqlOnly; import google.registry.util.CidrAddressBlock; import google.registry.util.SerializeUtils; +import java.math.BigDecimal; import org.joda.money.CurrencyUnit; +import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; /** Unit tests for {@link Registrar}. */ @@ -60,7 +67,20 @@ class RegistrarTest extends EntityTestCase { @BeforeEach void setUp() { - createTld("xn--q9jyb4c"); + createTld("tld"); + persistResource( + newRegistry("xn--q9jyb4c", "MINNA") + .asBuilder() + .setCurrency(JPY) + .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300))) + .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700))) + .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900))) + .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700))) + .setRenewBillingCostTransitions( + ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100)))) + .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY))) + .setPremiumList(null) + .build()); // Set up a new persisted registrar entity. registrar = cloneAndSetAutoTimestamps( @@ -251,8 +271,10 @@ class RegistrarTest extends EntityTestCase { } @TestOfyAndSql - void testSuccess_clearingBillingAccountMap() { - registrar = registrar.asBuilder().setBillingAccountMap(null).build(); + void testSuccess_clearingBillingAccountMapAndAllowedTlds() { + registrar = + registrar.asBuilder().setAllowedTlds(ImmutableSet.of()).setBillingAccountMap(null).build(); + assertThat(registrar.getAllowedTlds()).isEmpty(); assertThat(registrar.getBillingAccountMap()).isEmpty(); } @@ -677,4 +699,48 @@ class RegistrarTest extends EntityTestCase { assertThrows(IllegalArgumentException.class, () -> Registrar.loadByRegistrarIdCached("")); assertThat(thrown).hasMessageThat().contains("registrarId must be specified"); } + + @TestOfyAndSql + void testFailure_missingCurrenciesFromBillingMap() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + registrar + .asBuilder() + .setBillingAccountMap(null) + .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c")) + .build()); + assertThat(thrown) + .hasMessageThat() + .contains("their currency is missing from the billing account map: [tld, xn--q9jyb4c]"); + } + + @TestOfyAndSql + void testFailure_missingCurrencyFromBillingMap() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + registrar + .asBuilder() + .setBillingAccountMap(ImmutableMap.of(USD, "abc123")) + .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c")) + .build()); + assertThat(thrown) + .hasMessageThat() + .contains("their currency is missing from the billing account map: [xn--q9jyb4c]"); + } + + @TestOfyAndSql + void testSuccess_nonRealTldDoesntNeedEntryInBillingMap() { + persistResource(Registry.get("xn--q9jyb4c").asBuilder().setTldType(TldType.TEST).build()); + // xn--q9jyb4c bills in JPY and we don't have a JPY entry in this billing account map, but it + // should succeed without throwing an error because xn--q9jyb4c is set to be a TEST TLD. + registrar + .asBuilder() + .setBillingAccountMap(ImmutableMap.of(USD, "abc123")) + .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c")) + .build(); + } } diff --git a/core/src/test/java/google/registry/model/tld/RegistryTest.java b/core/src/test/java/google/registry/model/tld/RegistryTest.java index 38b6d3a6e..5db5f8607 100644 --- a/core/src/test/java/google/registry/model/tld/RegistryTest.java +++ b/core/src/test/java/google/registry/model/tld/RegistryTest.java @@ -190,7 +190,7 @@ public final class RegistryTest extends EntityTestCase { @TestOfyAndSql void testGetAll() { createTld("foo"); - assertThat(Registry.getAll(ImmutableSet.of("foo", "tld"))) + assertThat(Registry.get(ImmutableSet.of("foo", "tld"))) .containsExactlyElementsIn( tm().transact( () -> diff --git a/core/src/test/java/google/registry/testing/AppEngineExtension.java b/core/src/test/java/google/registry/testing/AppEngineExtension.java index 0b46155a9..086159828 100644 --- a/core/src/test/java/google/registry/testing/AppEngineExtension.java +++ b/core/src/test/java/google/registry/testing/AppEngineExtension.java @@ -66,6 +66,7 @@ import java.util.Set; import java.util.logging.LogManager; import java.util.stream.Stream; import javax.annotation.Nullable; +import org.joda.money.CurrencyUnit; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -289,6 +290,7 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa .build()) .setPhoneNumber("+1.3334445555") .setPhonePasscode("12345") + .setBillingAccountMap(ImmutableMap.of(CurrencyUnit.USD, "abc123")) .setContactsRequireSyncing(true); } diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 2e6c9a7b9..9f26de4b1 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -762,6 +762,7 @@ public class DatabaseHelper { .setType(type) .setState(State.ACTIVE) .setIanaIdentifier(ianaIdentifier) + .setBillingAccountMap(ImmutableMap.of(USD, "abc123")) .setLocalizedAddress( new RegistrarAddress.Builder() .setStreet(ImmutableList.of("123 Fake St")) diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index 62cb93dce..ba479e71e 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -21,8 +21,11 @@ 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; import static google.registry.testing.DatabaseHelper.createTlds; +import static google.registry.testing.DatabaseHelper.newRegistry; import static google.registry.testing.DatabaseHelper.persistNewRegistrar; +import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.joda.money.CurrencyUnit.JPY; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; @@ -43,8 +46,10 @@ import google.registry.testing.DualDatabaseTest; import google.registry.testing.InjectExtension; import google.registry.testing.TestOfyAndSql; import java.io.IOException; +import java.math.BigDecimal; import java.util.Optional; import org.joda.money.CurrencyUnit; +import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.RegisterExtension; @@ -604,11 +609,11 @@ class CreateRegistrarCommandTest extends CommandTestCase Optional registrar = Registrar.loadByRegistrarId("clientz"); assertThat(registrar).isPresent(); assertThat(registrar.get().getBillingAccountMap()) - .containsExactly(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz"); + .containsExactly(CurrencyUnit.USD, "abc123", JPY, "789xyz"); } @TestOfyAndSql - void testFailure_billingAccountMap_doesNotContainEntryForTldAllowed() { + void testFailure_billingAccountMap_doesNotContainEntryForAllowedTld() { createTlds("foo"); IllegalArgumentException thrown = @@ -632,12 +637,26 @@ class CreateRegistrarCommandTest extends CommandTestCase "--cc US", "--force", "clientz")); - assertThat(thrown).hasMessageThat().contains("USD"); + assertThat(thrown) + .hasMessageThat() + .contains("their currency is missing from the billing account map: [foo]"); } @TestOfyAndSql void testSuccess_billingAccountMap_onlyAppliesToRealRegistrar() throws Exception { - createTlds("foo"); + persistResource( + newRegistry("foo", "FOO") + .asBuilder() + .setCurrency(JPY) + .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300))) + .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700))) + .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900))) + .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700))) + .setRenewBillingCostTransitions( + ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100)))) + .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY))) + .setPremiumList(null) + .build()); runCommandForced( "--name=blobio", @@ -656,7 +675,7 @@ class CreateRegistrarCommandTest extends CommandTestCase Optional registrar = Registrar.loadByRegistrarId("clientz"); assertThat(registrar).isPresent(); - assertThat(registrar.get().getBillingAccountMap()).containsExactly(CurrencyUnit.JPY, "789xyz"); + assertThat(registrar.get().getBillingAccountMap()).containsExactly(JPY, "789xyz"); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 4cb894f44..b149cb6db 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -21,8 +21,10 @@ import static google.registry.testing.CertificateSamples.SAMPLE_CERT3; import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.loadRegistrar; +import static google.registry.testing.DatabaseHelper.newRegistry; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.joda.money.CurrencyUnit.JPY; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -38,8 +40,10 @@ import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.Type; import google.registry.testing.AppEngineExtension; import google.registry.util.CidrAddressBlock; +import java.math.BigDecimal; import java.util.Optional; import org.joda.money.CurrencyUnit; +import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -359,6 +363,8 @@ class UpdateRegistrarCommandTest extends CommandTestCase @Test void testSuccess_billingAccountMap() throws Exception { + persistResource( + loadRegistrar("NewRegistrar").asBuilder().setBillingAccountMap(ImmutableMap.of()).build()); assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty(); runCommand("--billing_account_map=USD=abc123,JPY=789xyz", "--force", "NewRegistrar"); assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()) @@ -366,8 +372,14 @@ class UpdateRegistrarCommandTest extends CommandTestCase } @Test - void testFailure_billingAccountMap_doesNotContainEntryForTldAllowed() { + void testFailure_billingAccountMap_doesNotContainEntryForAllowedTld() { createTlds("foo"); + persistResource( + loadRegistrar("NewRegistrar") + .asBuilder() + .setAllowedTlds(ImmutableSet.of()) + .setBillingAccountMap(ImmutableMap.of()) + .build()); assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty(); IllegalArgumentException thrown = assertThrows( @@ -379,12 +391,28 @@ class UpdateRegistrarCommandTest extends CommandTestCase "--force", "--registrar_type=REAL", "NewRegistrar")); - assertThat(thrown).hasMessageThat().contains("USD"); + assertThat(thrown) + .hasMessageThat() + .contains("their currency is missing from the billing account map: [foo]"); } @Test void testSuccess_billingAccountMap_onlyAppliesToRealRegistrar() throws Exception { - createTlds("foo"); + persistResource( + newRegistry("foo", "FOO") + .asBuilder() + .setCurrency(JPY) + .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300))) + .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700))) + .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900))) + .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700))) + .setRenewBillingCostTransitions( + ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100)))) + .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY))) + .setPremiumList(null) + .build()); + persistResource( + loadRegistrar("NewRegistrar").asBuilder().setBillingAccountMap(ImmutableMap.of()).build()); assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty(); runCommand("--billing_account_map=JPY=789xyz", "--allowed_tlds=foo", "--force", "NewRegistrar"); assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())