diff --git a/core/src/main/java/google/registry/tools/GenerateAllocationTokensCommand.java b/core/src/main/java/google/registry/tools/GenerateAllocationTokensCommand.java index 06fadc0c0..da4985b42 100644 --- a/core/src/main/java/google/registry/tools/GenerateAllocationTokensCommand.java +++ b/core/src/main/java/google/registry/tools/GenerateAllocationTokensCommand.java @@ -18,9 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Sets.difference; import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.DEFAULT; +import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE; import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE; import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.util.CollectionUtils.isNullOrEmpty; import static google.registry.util.CollectionUtils.nullToEmpty; import static google.registry.util.StringGenerator.DEFAULT_PASSWORD_LENGTH; import static java.nio.charset.StandardCharsets.UTF_8; @@ -253,6 +255,22 @@ class GenerateAllocationTokensCommand implements Command { !ImmutableList.of("").equals(allowedTlds), "Either omit --allowed_tlds if all TLDs are allowed, or include a comma-separated list"); + if (!isNullOrEmpty(tokenStatusTransitions)) { + // Don't allow package tokens to be created with a scheduled end time since this could allow + // future domains to be attributed to the package and never be billed. Package promotion + // tokens should only be scheduled to end with a brief time period before the status + // transition occurs so that no new domains are registered using that token between when the + // status is scheduled and when the transition occurs. + // TODO(@sarahbot): Create a cleaner way to handle ending packages once we actually have + // customers using them + boolean hasEnding = + tokenStatusTransitions.containsValue(TokenStatus.ENDED) + || tokenStatusTransitions.containsValue(TokenStatus.CANCELLED); + checkArgument( + !(PACKAGE.equals(tokenType) && hasEnding), + "PACKAGE tokens should not be generated with ENDED or CANCELLED in their transition map"); + } + if (tokenStrings != null) { verifyTokenStringsDoNotExist(); } diff --git a/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java b/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java index f77682a10..20981cefd 100644 --- a/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java @@ -14,6 +14,7 @@ package google.registry.tools; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.partition; @@ -31,6 +32,7 @@ import google.registry.model.billing.BillingEvent.RenewalPriceBehavior; import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.RegistrationBehavior; import google.registry.model.domain.token.AllocationToken.TokenStatus; +import google.registry.model.domain.token.AllocationToken.TokenType; import google.registry.tools.params.TransitionListParameter.TokenStatusTransitions; import java.util.List; import java.util.Map; @@ -114,6 +116,7 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens private static final Joiner JOINER = Joiner.on(", "); private ImmutableSet tokensToSave; + private boolean endToken = false; @Override public void init() { @@ -126,6 +129,12 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens allowedTlds = ImmutableList.of(); } + if (tokenStatusTransitions != null + && (tokenStatusTransitions.containsValue(TokenStatus.ENDED) + || tokenStatusTransitions.containsValue(TokenStatus.CANCELLED))) { + endToken = true; + } + tokensToSave = tm().transact( () -> @@ -157,6 +166,19 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens } private AllocationToken updateToken(AllocationToken original) { + if (endToken && original.getTokenType().equals(TokenType.PACKAGE)) { + Long domainsInPackage = + tm().query("SELECT COUNT(*) FROM Domain WHERE currentPackageToken = :token", Long.class) + .setParameter("token", original.createVKey()) + .getSingleResult(); + + checkArgument( + domainsInPackage == 0, + "Package token %s can not end its promotion because it still has %s domains in the" + + " package", + original.getToken(), + domainsInPackage); + } AllocationToken.Builder builder = original.asBuilder(); Optional.ofNullable(allowedClientIds) .ifPresent(clientIds -> builder.setAllowedRegistrarIds(ImmutableSet.copyOf(clientIds))); diff --git a/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java b/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java index 7eba0703e..253fe9926 100644 --- a/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java +++ b/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java @@ -395,6 +395,26 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase + runCommand( + "--number", + "999", + "--type", + "PACKAGE", + String.format( + "--token_status_transitions=\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"", + START_OF_TIME, fakeClock.nowUtc(), fakeClock.nowUtc().plusDays(1))))) + .hasMessageThat() + .isEqualTo( + "PACKAGE tokens should not be generated with ENDED or CANCELLED in their transition" + + " map"); + } + @Test void testFailure_lengthOfZero() { IllegalArgumentException thrown = diff --git a/core/src/test/java/google/registry/tools/UpdateAllocationTokensCommandTest.java b/core/src/test/java/google/registry/tools/UpdateAllocationTokensCommandTest.java index 57454395d..8ab728e2f 100644 --- a/core/src/test/java/google/registry/tools/UpdateAllocationTokensCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateAllocationTokensCommandTest.java @@ -22,9 +22,12 @@ import static google.registry.model.domain.token.AllocationToken.TokenStatus.CAN import static google.registry.model.domain.token.AllocationToken.TokenStatus.ENDED; import static google.registry.model.domain.token.AllocationToken.TokenStatus.NOT_STARTED; import static google.registry.model.domain.token.AllocationToken.TokenStatus.VALID; +import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE; import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE; import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE; +import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadByEntity; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.DateTimeZone.UTC; @@ -255,7 +258,7 @@ class UpdateAllocationTokensCommandTest extends CommandTestCasenaturalOrder() + .put(START_OF_TIME, NOT_STARTED) + .put(now.minusDays(1), VALID) + .build()) + .build()); + runCommandForced( + "--prefix", + "token", + "--token_status_transitions", + String.format( + "\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"", START_OF_TIME, now.minusDays(1), now)); + token = reloadResource(token); + assertThat(token.getTokenStatusTransitions().toValueMap()) + .containsExactly(START_OF_TIME, NOT_STARTED, now.minusDays(1), VALID, now, ENDED); + } + + @Test + void testUpdateStatusTransitions_endPackageTokenWithActiveDomainsFails() throws Exception { + DateTime now = fakeClock.nowUtc(); + AllocationToken token = + persistResource( + new AllocationToken.Builder() + .setToken("token") + .setTokenType(PACKAGE) + .setRenewalPriceBehavior(SPECIFIED) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setTokenStatusTransitions( + ImmutableSortedMap.naturalOrder() + .put(START_OF_TIME, NOT_STARTED) + .put(now.minusDays(1), VALID) + .build()) + .build()); + createTld("tld"); + persistResource( + persistActiveDomain("example.tld") + .asBuilder() + .setCurrentPackageToken(token.createVKey()) + .build()); + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + runCommandForced( + "--prefix", + "token", + "--token_status_transitions", + String.format( + "\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"", + START_OF_TIME, now.minusDays(1), now))); + assertThat(thrown) + .hasMessageThat() + .isEqualTo( + "Package token token can not end its promotion because it still has 1 domains in the" + + " package"); + } + @Test void testUpdate_onlyWithPrefix() throws Exception { AllocationToken token =