From 45c8b8182374a15bea06f34d908da7f7918e8f9f Mon Sep 17 00:00:00 2001 From: gbrodman Date: Wed, 15 Jan 2025 14:55:34 -0500 Subject: [PATCH] Map token renewal behavior directly onto BillingRecurrence (#2635) Instead of using a separate RenewalPriceInfo object, just map the behavior (if it exists) onto the BillingRecurrence with a special carve-out, as always, for anchor tenants (note: this shouldn't matter much since anchor tenants *should* use NONPREMIUM renewal tokens anyway, but just in case, double-check). This also fixes DomainPricingLogic to treat a multiyear create as a one-year-create + n-minus-1-year-renewal for cases where either the creation or the renewal (or both) are nonpremium. --- .../flows/domain/DomainCreateFlow.java | 58 ++----- .../flows/domain/DomainPricingLogic.java | 31 +++- .../flows/domain/DomainCreateFlowTest.java | 154 +++++++++--------- .../flows/domain/DomainPricingLogicTest.java | 90 +++++++++- 4 files changed, 203 insertions(+), 130 deletions(-) diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index ea18605da..bf4f3569c 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -14,7 +14,6 @@ package google.registry.flows.domain; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.persistEntityChanges; @@ -120,9 +119,7 @@ import google.registry.model.tmch.ClaimsList; import google.registry.model.tmch.ClaimsListDao; import google.registry.tmch.LordnTaskUtils.LordnPhase; import java.util.Optional; -import javax.annotation.Nullable; import javax.inject.Inject; -import org.joda.money.Money; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -363,9 +360,7 @@ public final class DomainCreateFlow implements MutatingFlow { // Create a new autorenew billing event and poll message starting at the expiration time. BillingRecurrence autorenewBillingEvent = createAutorenewBillingEvent( - domainHistoryId, - registrationExpirationTime, - getRenewalPriceInfo(isAnchorTenant, allocationToken)); + domainHistoryId, registrationExpirationTime, isAnchorTenant, allocationToken); PollMessage.Autorenew autorenewPollMessage = createAutorenewPollMessage(domainHistoryId, registrationExpirationTime); ImmutableSet.Builder entitiesToSave = new ImmutableSet.Builder<>(); @@ -625,7 +620,17 @@ public final class DomainCreateFlow implements MutatingFlow { private BillingRecurrence createAutorenewBillingEvent( HistoryEntryId domainHistoryId, DateTime registrationExpirationTime, - RenewalPriceInfo renewalpriceInfo) { + boolean isAnchorTenant, + Optional allocationToken) { + // Non-standard renewal behaviors can occur for anchor tenants (always NONPREMIUM pricing) or if + // explicitly configured in the token (either NONPREMIUM or directly SPECIFIED). Use DEFAULT if + // none is configured. + RenewalPriceBehavior renewalPriceBehavior = + isAnchorTenant + ? RenewalPriceBehavior.NONPREMIUM + : allocationToken + .map(AllocationToken::getRenewalPriceBehavior) + .orElse(RenewalPriceBehavior.DEFAULT); return new BillingRecurrence.Builder() .setReason(Reason.RENEW) .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) @@ -634,8 +639,8 @@ public final class DomainCreateFlow implements MutatingFlow { .setEventTime(registrationExpirationTime) .setRecurrenceEndTime(END_OF_TIME) .setDomainHistoryId(domainHistoryId) - .setRenewalPriceBehavior(renewalpriceInfo.renewalPriceBehavior()) - .setRenewalPrice(renewalpriceInfo.renewalPrice()) + .setRenewalPriceBehavior(renewalPriceBehavior) + .setRenewalPrice(allocationToken.flatMap(AllocationToken::getRenewalPrice).orElse(null)) .build(); } @@ -679,41 +684,6 @@ public final class DomainCreateFlow implements MutatingFlow { .build(); } - /** - * Determines the {@link RenewalPriceBehavior} and the renewal price that needs be stored in the - * {@link BillingRecurrence} billing events. - * - *

By default, the renewal price is calculated during the process of renewal. Renewal price - * should be the createCost if and only if the renewal price behavior in the {@link - * AllocationToken} is 'SPECIFIED'. - */ - static RenewalPriceInfo getRenewalPriceInfo( - boolean isAnchorTenant, Optional allocationToken) { - if (isAnchorTenant) { - allocationToken.ifPresent( - token -> - checkArgument( - token.getRenewalPriceBehavior() != RenewalPriceBehavior.SPECIFIED, - "Renewal price behavior cannot be SPECIFIED for anchor tenant")); - return RenewalPriceInfo.create(RenewalPriceBehavior.NONPREMIUM, null); - } else if (allocationToken.isPresent() - && allocationToken.get().getRenewalPriceBehavior() == RenewalPriceBehavior.SPECIFIED) { - return RenewalPriceInfo.create( - RenewalPriceBehavior.SPECIFIED, allocationToken.get().getRenewalPrice().get()); - } else { - return RenewalPriceInfo.create(RenewalPriceBehavior.DEFAULT, null); - } - } - - /** A record to store renewal info used in {@link BillingRecurrence} billing events. */ - public record RenewalPriceInfo( - RenewalPriceBehavior renewalPriceBehavior, @Nullable Money renewalPrice) { - static RenewalPriceInfo create( - RenewalPriceBehavior renewalPriceBehavior, @Nullable Money renewalPrice) { - return new RenewalPriceInfo(renewalPriceBehavior, renewalPrice); - } - } - private static ImmutableList createResponseExtensions( Optional feeCreate, FeesAndCredits feesAndCredits) { return feeCreate diff --git a/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java b/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java index 9b55e4a12..55d4fb8ba 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java +++ b/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java @@ -85,14 +85,10 @@ public final class DomainPricingLogic { createFee = Fee.create(zeroInCurrency(currency), FeeType.CREATE, false); } else { DomainPrices domainPrices = getPricesForDomainName(domainName, dateTime); - if (allocationToken.isPresent() - && allocationToken - .get() - .getRegistrationBehavior() - .equals(RegistrationBehavior.NONPREMIUM_CREATE)) { + if (allocationToken.isPresent()) { + // Handle any special NONPREMIUM / SPECIFIED cases configured in the token domainPrices = - DomainPrices.create( - false, tld.getCreateBillingCost(dateTime), domainPrices.getRenewCost()); + applyTokenToDomainPrices(domainPrices, tld, dateTime, years, allocationToken.get()); } Money domainCreateCost = getDomainCreateCostWithDiscount(domainPrices, years, allocationToken, tld); @@ -357,6 +353,27 @@ public final class DomainPricingLogic { return totalDomainFlowCost; } + private DomainPrices applyTokenToDomainPrices( + DomainPrices domainPrices, Tld tld, DateTime dateTime, int years, AllocationToken token) { + // Convert to nonpremium iff no premium charges are included (either in create or any renewal) + boolean convertToNonPremium = + token.getRegistrationBehavior().equals(RegistrationBehavior.NONPREMIUM_CREATE) + && (years == 1 + || !token.getRenewalPriceBehavior().equals(RenewalPriceBehavior.DEFAULT)); + boolean isPremium = domainPrices.isPremium() && !convertToNonPremium; + Money createCost = + token.getRegistrationBehavior().equals(RegistrationBehavior.NONPREMIUM_CREATE) + ? tld.getCreateBillingCost(dateTime) + : domainPrices.getCreateCost(); + Money renewCost = + token.getRenewalPriceBehavior().equals(RenewalPriceBehavior.NONPREMIUM) + ? tld.getStandardRenewCost(dateTime) + : token.getRenewalPriceBehavior().equals(RenewalPriceBehavior.SPECIFIED) + ? token.getRenewalPrice().get() + : domainPrices.getRenewCost(); + return DomainPrices.create(isPremium, createCost, renewCost); + } + /** An allocation token was provided that is invalid for premium domains. */ public static class AllocationTokenInvalidForPremiumNameException extends CommandUseErrorException { diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index 9928d2809..8b03320d4 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -22,7 +22,6 @@ import static google.registry.flows.FlowTestCase.UserPrivileges.SUPERUSER; import static google.registry.model.billing.BillingBase.Flag.ANCHOR_TENANT; import static google.registry.model.billing.BillingBase.Flag.RESERVED; import static google.registry.model.billing.BillingBase.Flag.SUNRISE; -import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.DEFAULT; import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.NONPREMIUM; import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.SPECIFIED; import static google.registry.model.common.FeatureFlag.FeatureName.MINIMUM_DATASET_CONTACTS_OPTIONAL; @@ -87,7 +86,6 @@ import google.registry.flows.domain.DomainCreateFlow.BulkDomainRegisteredForTooM import google.registry.flows.domain.DomainCreateFlow.MustHaveSignedMarksInCurrentPhaseException; import google.registry.flows.domain.DomainCreateFlow.NoGeneralRegistrationsInCurrentPhaseException; import google.registry.flows.domain.DomainCreateFlow.NoTrademarkedRegistrationsBeforeSunriseException; -import google.registry.flows.domain.DomainCreateFlow.RenewalPriceInfo; import google.registry.flows.domain.DomainCreateFlow.SignedMarksOnlyDuringSunriseException; import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkExpiredException; import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkNotYetValidException; @@ -303,10 +301,8 @@ class DomainCreateFlowTest extends ResourceFlowTestCase expectedBillingEvents = @@ -3187,85 +3210,62 @@ class DomainCreateFlowTest extends ResourceFlowTestCase - DomainCreateFlow.getRenewalPriceInfo( - true, - Optional.of( - persistResource( - new AllocationToken.Builder() - .setToken("abc123") - .setTokenType(SINGLE_USE) - .setRenewalPriceBehavior(SPECIFIED) - .setRenewalPrice(Money.of(USD, 0)) - .build())))); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Renewal price behavior cannot be SPECIFIED for anchor tenant"); + void testSuccess_nonAnchorTenant_nonPremiumRenewal() throws Exception { + createTld("example"); + AllocationToken token = + persistResource( + new AllocationToken.Builder() + .setToken("abc123") + .setTokenType(SINGLE_USE) + .setDomainName("rich.example") + .setRenewalPriceBehavior(NONPREMIUM) + .build()); + persistContactsAndHosts(); + // Creation is still $100 but it'll create a NONPREMIUM renewal + setEppInput( + "domain_create_premium_allocationtoken.xml", + ImmutableMap.of("YEARS", "2", "FEE", "111.00")); + runFlow(); + assertSuccessfulCreate("example", ImmutableSet.of(), token); } @Test - void testGetRenewalPriceInfo_withInvalidRenewalPriceBehavior_throwsError() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> - DomainCreateFlow.getRenewalPriceInfo( - true, - Optional.of( - persistResource( - new AllocationToken.Builder() - .setToken("abc123") - .setTokenType(SINGLE_USE) - .setRenewalPriceBehavior(RenewalPriceBehavior.valueOf("INVALID")) - .build())))); - assertThat(thrown) - .hasMessageThat() - .isEqualTo( - "No enum constant" - + " google.registry.model.billing.BillingBase.RenewalPriceBehavior.INVALID"); + void testSuccess_specifiedRenewalPriceToken_specifiedRecurrencePrice() throws Exception { + createTld("example"); + AllocationToken token = + persistResource( + new AllocationToken.Builder() + .setToken("abc123") + .setTokenType(SINGLE_USE) + .setDomainName("rich.example") + .setRenewalPriceBehavior(SPECIFIED) + .setRenewalPrice(Money.of(USD, 1)) + .build()); + persistContactsAndHosts(); + // Creation is still $100 but it'll create a $1 renewal + setEppInput( + "domain_create_premium_allocationtoken.xml", + ImmutableMap.of("YEARS", "2", "FEE", "101.00")); + runFlow(); + assertSuccessfulCreate("example", ImmutableSet.of(), token); } @Test diff --git a/core/src/test/java/google/registry/flows/domain/DomainPricingLogicTest.java b/core/src/test/java/google/registry/flows/domain/DomainPricingLogicTest.java index 5b358294d..c676539e8 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainPricingLogicTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainPricingLogicTest.java @@ -1127,7 +1127,7 @@ public class DomainPricingLogicTest { .setCurrency(USD) .addFeeOrCredit(Fee.create(new BigDecimal("13.00"), CREATE, false)) .build()); - // Two-year create should be 13 (standard price) + 100 (premium price) + // Two-year create should be 13 (standard price) + 100 (premium price), and it's premium assertThat( domainPricingLogic.getCreatePrice( tld, @@ -1140,7 +1140,7 @@ public class DomainPricingLogicTest { .isEqualTo( new FeesAndCredits.Builder() .setCurrency(USD) - .addFeeOrCredit(Fee.create(new BigDecimal("113.00"), CREATE, false)) + .addFeeOrCredit(Fee.create(new BigDecimal("113.00"), CREATE, true)) .build()); assertThat( domainPricingLogic.getRenewPrice( @@ -1156,4 +1156,90 @@ public class DomainPricingLogicTest { .addFeeOrCredit(Fee.create(new BigDecimal("100.00"), RENEW, true)) .build()); } + + @Test + void testGetDomainCreatePrice_premium_multiYear_nonpremiumCreateAndRenewal() throws Exception { + AllocationToken allocationToken = + persistResource( + new AllocationToken.Builder() + .setToken("abc123") + .setTokenType(SINGLE_USE) + .setDomainName("premium.example") + .setRegistrationBehavior(AllocationToken.RegistrationBehavior.NONPREMIUM_CREATE) + .setRenewalPriceBehavior(NONPREMIUM) + .build()); + // Two-year create should be standard create (13) + renewal (10) because both create and renewal + // are standard + assertThat( + domainPricingLogic.getCreatePrice( + tld, + "premium.example", + clock.nowUtc(), + 2, + false, + false, + Optional.of(allocationToken))) + .isEqualTo( + new FeesAndCredits.Builder() + .setCurrency(USD) + .addFeeOrCredit(Fee.create(new BigDecimal("23.00"), CREATE, false)) + .build()); + // Similarly, 3 years should be 13 + 10 + 10 + assertThat( + domainPricingLogic.getCreatePrice( + tld, + "premium.example", + clock.nowUtc(), + 3, + false, + false, + Optional.of(allocationToken))) + .isEqualTo( + new FeesAndCredits.Builder() + .setCurrency(USD) + .addFeeOrCredit(Fee.create(new BigDecimal("33.00"), CREATE, false)) + .build()); + } + + @Test + void testGetDomainCreatePrice_premium_multiYear_onlyNonpremiumRenewal() throws Exception { + AllocationToken allocationToken = + persistResource( + new AllocationToken.Builder() + .setToken("abc123") + .setTokenType(SINGLE_USE) + .setDomainName("premium.example") + .setRenewalPriceBehavior(NONPREMIUM) + .build()); + // Two-year create should be 100 (premium 1st year) plus 10 (nonpremium 2nd year) + assertThat( + domainPricingLogic.getCreatePrice( + tld, + "premium.example", + clock.nowUtc(), + 2, + false, + false, + Optional.of(allocationToken))) + .isEqualTo( + new FeesAndCredits.Builder() + .setCurrency(USD) + .addFeeOrCredit(Fee.create(new BigDecimal("110.00"), CREATE, true)) + .build()); + // Similarly, 3 years should be 100 + 10 + 10 + assertThat( + domainPricingLogic.getCreatePrice( + tld, + "premium.example", + clock.nowUtc(), + 3, + false, + false, + Optional.of(allocationToken))) + .isEqualTo( + new FeesAndCredits.Builder() + .setCurrency(USD) + .addFeeOrCredit(Fee.create(new BigDecimal("120.00"), CREATE, true)) + .build()); + } }