From 3256fa8cb117600238a7a80ac05a2ddacd65a518 Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Thu, 26 Mar 2026 15:36:39 -0400 Subject: [PATCH] Use the cheapest default token when multiple are available Previously we would just use the first one we found. This is a valid behavior, but we want to change it so that we apply the cheapest default if multiple are available (this way we avoid having to go back after the fact and give refunds). --- .../bsa/persistence/BsaLabelUtils.java | 3 +- .../google/registry/flows/FlowReporter.java | 3 +- .../flows/domain/DomainCheckFlow.java | 11 +- .../flows/domain/DomainCreateFlow.java | 4 +- .../flows/domain/DomainPricingLogic.java | 11 +- .../flows/domain/DomainRenewFlow.java | 7 +- .../token/AllocationTokenFlowUtils.java | 102 +++++++++++----- .../model/tld/label/ReservedList.java | 3 +- .../registry/rdap/RdapJsonFormatter.java | 3 +- .../tools/GetAllocationTokenCommand.java | 3 +- .../tools/server/ListPremiumListsAction.java | 3 +- .../tools/server/ListReservedListsAction.java | 3 +- .../flows/domain/DomainCreateFlowTest.java | 2 +- .../flows/domain/DomainRenewFlowTest.java | 15 ++- .../token/AllocationTokenFlowUtilsTest.java | 113 +++++++++++++++++- .../sql/flyway/FlywayDeadlockTest.java | 3 +- 16 files changed, 215 insertions(+), 74 deletions(-) diff --git a/core/src/main/java/google/registry/bsa/persistence/BsaLabelUtils.java b/core/src/main/java/google/registry/bsa/persistence/BsaLabelUtils.java index 80221e322a9..6ec621739ae 100644 --- a/core/src/main/java/google/registry/bsa/persistence/BsaLabelUtils.java +++ b/core/src/main/java/google/registry/bsa/persistence/BsaLabelUtils.java @@ -100,8 +100,7 @@ public static ImmutableSet getBlockedLabels(ImmutableCollection ImmutableList> queriedLabels = domainLabels.stream().map(BsaLabel::vKey).collect(toImmutableList()); return cacheBsaLabels.getAll(queriedLabels).values().stream() - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .map(BsaLabel::getLabel) .collect(toImmutableSet()); } diff --git a/core/src/main/java/google/registry/flows/FlowReporter.java b/core/src/main/java/google/registry/flows/FlowReporter.java index 19e8e79c438..39c2b1d2914 100644 --- a/core/src/main/java/google/registry/flows/FlowReporter.java +++ b/core/src/main/java/google/registry/flows/FlowReporter.java @@ -100,8 +100,7 @@ private static Optional extractTld(String domainName) { public static ImmutableSet extractTlds(Iterable domainNames) { return Streams.stream(domainNames) .map(FlowReporter::extractTld) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(toImmutableSet()); } diff --git a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java index b77e6596e9e..59f9a523b84 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java @@ -225,7 +225,8 @@ private Optional getMessageForCheck( ImmutableSet bsaBlockedDomainNames, ImmutableMap tldStates, ImmutableMap parsedDomains, - DateTime now) { + DateTime now) + throws EppException { InternetDomainName idn = parsedDomains.get(domainName); Optional token; try { @@ -238,7 +239,9 @@ private Optional getMessageForCheck( eppInput.getSingleExtension(AllocationTokenExtension.class), Tld.get(idn.parent().toString()), domainName, - FeeQueryCommandExtensionItem.CommandName.CREATE); + FeeQueryCommandExtensionItem.CommandName.CREATE, + Optional.empty(), + pricingLogic); } catch (AllocationTokenFlowUtils.NonexistentAllocationTokenException | AllocationTokenFlowUtils.AllocationTokenInvalidException e) { // The provided token was catastrophically invalid in some way @@ -317,7 +320,9 @@ private ImmutableList getResponseExtensions( eppInput.getSingleExtension(AllocationTokenExtension.class), tld, domainName, - feeCheckItem.getCommandName()); + feeCheckItem.getCommandName(), + Optional.empty(), + pricingLogic); } catch (AllocationTokenFlowUtils.NonexistentAllocationTokenException | AllocationTokenFlowUtils.AllocationTokenInvalidException e) { // The provided token was catastrophically invalid in some way 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 2717eba70d9..39fca08651d 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -264,7 +264,9 @@ public EppResponse run() throws EppException { eppInput.getSingleExtension(AllocationTokenExtension.class), tld, command.getDomainName(), - CommandName.CREATE); + CommandName.CREATE, + Optional.of(years), + pricingLogic); boolean defaultTokenUsed = allocationToken.map(t -> t.getTokenType().equals(TokenType.DEFAULT_PROMO)).orElse(false); boolean isAnchorTenant = 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 d3c8a2429f1..9b47261f122 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java +++ b/core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java @@ -67,7 +67,7 @@ public DomainPricingLogic(DomainPricingCustomLogic customLogic) { *

If {@code allocationToken} is present and the domain is non-premium, that discount will be * applied to the first year. */ - FeesAndCredits getCreatePrice( + public FeesAndCredits getCreatePrice( Tld tld, String domainName, DateTime dateTime, @@ -193,8 +193,8 @@ public FeesAndCredits getRenewPrice( } /** Returns a new restore price for the pricer. */ - FeesAndCredits getRestorePrice(Tld tld, String domainName, DateTime dateTime, boolean isExpired) - throws EppException { + public FeesAndCredits getRestorePrice( + Tld tld, String domainName, DateTime dateTime, boolean isExpired) throws EppException { DomainPrices domainPrices = getPricesForDomainName(domainName, dateTime); FeesAndCredits.Builder feesAndCredits = new FeesAndCredits.Builder() @@ -216,7 +216,7 @@ FeesAndCredits getRestorePrice(Tld tld, String domainName, DateTime dateTime, bo } /** Returns a new transfer price for the pricer. */ - FeesAndCredits getTransferPrice( + public FeesAndCredits getTransferPrice( Tld tld, String domainName, DateTime dateTime, @Nullable BillingRecurrence billingRecurrence) throws EppException { FeesAndCredits renewPrice = @@ -239,7 +239,8 @@ FeesAndCredits getTransferPrice( } /** Returns a new update price for the pricer. */ - FeesAndCredits getUpdatePrice(Tld tld, String domainName, DateTime dateTime) throws EppException { + public FeesAndCredits getUpdatePrice(Tld tld, String domainName, DateTime dateTime) + throws EppException { CurrencyUnit currency = tld.getCurrency(); BaseFee feeOrCredit = Fee.create(zeroInCurrency(currency), FeeType.UPDATE, false); return customLogic.customizeUpdatePrice( diff --git a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java index 120316c004f..ec7d982ca77 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java @@ -169,6 +169,8 @@ public EppResponse run() throws EppException { Domain existingDomain = loadAndVerifyExistence(Domain.class, targetId, now); String tldStr = existingDomain.getTld(); Tld tld = Tld.get(tldStr); + int years = command.getPeriod().getValue(); + Optional allocationToken = AllocationTokenFlowUtils.loadTokenFromExtensionOrGetDefault( registrarId, @@ -176,7 +178,9 @@ public EppResponse run() throws EppException { eppInput.getSingleExtension(AllocationTokenExtension.class), tld, existingDomain.getDomainName(), - CommandName.RENEW); + CommandName.RENEW, + Optional.of(years), + pricingLogic); boolean defaultTokenUsed = allocationToken .map(t -> t.getTokenType().equals(AllocationToken.TokenType.DEFAULT_PROMO)) @@ -186,7 +190,6 @@ public EppResponse run() throws EppException { // If client passed an applicable static token this updates the domain existingDomain = maybeApplyBulkPricingRemovalToken(existingDomain, allocationToken); - int years = command.getPeriod().getValue(); DateTime newExpirationTime = leapSafeAddYears(existingDomain.getRegistrationExpirationTime(), years); // Uncapped validateRegistrationPeriod(now, newExpirationTime); diff --git a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java index d22d269eb76..afeddc17490 100644 --- a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java @@ -15,7 +15,6 @@ package google.registry.flows.domain.token; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.pricing.PricingEngineProxy.isDomainPremium; @@ -24,21 +23,25 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.net.InternetDomainName; import google.registry.flows.EppException; import google.registry.flows.EppException.AssociationProhibitsOperationException; import google.registry.flows.EppException.AuthorizationErrorException; import google.registry.flows.EppException.StatusProhibitsOperationException; -import google.registry.model.billing.BillingBase; +import google.registry.flows.domain.DomainPricingLogic; +import google.registry.model.billing.BillingBase.RenewalPriceBehavior; import google.registry.model.billing.BillingRecurrence; import google.registry.model.domain.Domain; import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName; import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.TokenBehavior; +import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.domain.token.AllocationTokenExtension; import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Tld; import google.registry.persistence.VKey; +import java.math.BigDecimal; import java.util.Map; import java.util.Optional; import org.joda.time.DateTime; @@ -91,8 +94,10 @@ public static Optional loadTokenFromExtensionOrGetDefault( Optional extension, Tld tld, String domainName, - CommandName commandName) - throws NonexistentAllocationTokenException, AllocationTokenInvalidException { + CommandName commandName, + Optional years, + DomainPricingLogic pricingLogic) + throws EppException { Optional fromExtension = loadAllocationTokenFromExtension(registrarId, domainName, now, extension); if (fromExtension.isPresent() @@ -100,7 +105,8 @@ && tokenIsValidAgainstDomain( InternetDomainName.from(domainName), fromExtension.get(), commandName, now)) { return fromExtension; } - return checkForDefaultToken(tld, domainName, commandName, registrarId, now); + return checkForDefaultToken( + tld, domainName, commandName, registrarId, now, years, pricingLogic); } /** Verifies that the given domain can have a bulk pricing token removed from it. */ @@ -133,7 +139,7 @@ public static Domain maybeApplyBulkPricingRemovalToken( BillingRecurrence newBillingRecurrence = tm().loadByKey(domain.getAutorenewBillingEvent()) .asBuilder() - .setRenewalPriceBehavior(BillingBase.RenewalPriceBehavior.DEFAULT) + .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT) .setRenewalPrice(null) .build(); @@ -182,37 +188,70 @@ static boolean tokenIsValidAgainstDomain( * token found on the TLD's default token list will be returned. */ private static Optional checkForDefaultToken( - Tld tld, String domainName, CommandName commandName, String registrarId, DateTime now) { + Tld tld, + String domainName, + CommandName commandName, + String registrarId, + DateTime now, + Optional years, + DomainPricingLogic pricingLogic) + throws EppException { ImmutableList> tokensFromTld = tld.getDefaultPromoTokens(); if (isNullOrEmpty(tokensFromTld)) { return Optional.empty(); } - Map, Optional> tokens = - AllocationToken.getAll(tokensFromTld); - checkState( - !isNullOrEmpty(tokens), "Failure while loading default TLD tokens from the database"); - // Iterate over the list to maintain token ordering (since we return the first valid token) ImmutableList tokenList = - tokensFromTld.stream() - .map(tokens::get) - .filter(Optional::isPresent) - .map(Optional::get) + AllocationToken.getAll(tokensFromTld).values().stream() + .flatMap(Optional::stream) + // Filter to tokens that are 1. valid in general 2. valid for this particular request + .filter( + token -> { + try { + validateTokenEntity(token, registrarId, domainName, now); + } catch (AllocationTokenInvalidException e) { + return false; + } + return tokenIsValidAgainstDomain( + InternetDomainName.from(domainName), token, commandName, now); + }) .collect(toImmutableList()); - - // Check if any of the tokens are valid for this domain registration + // We can't compute the costs directly in the stream due to the checked EppException + ImmutableMap.Builder tokenCosts = new ImmutableMap.Builder<>(); for (AllocationToken token : tokenList) { - try { - validateTokenEntity(token, registrarId, domainName, now); - } catch (AllocationTokenInvalidException e) { - // Token is not valid for this registrar, etc. -- continue trying tokens - continue; - } - if (tokenIsValidAgainstDomain(InternetDomainName.from(domainName), token, commandName, now)) { - return Optional.of(token); - } + tokenCosts.put( + token, + getSampleCostWithToken(tld, domainName, token, commandName, now, years, pricingLogic)); } - // No valid default token found - return Optional.empty(); + return tokenCosts.build().entrySet().stream() + .min(Map.Entry.comparingByValue()) + .map(Map.Entry::getKey); + } + + private static BigDecimal getSampleCostWithToken( + Tld tld, + String domainName, + AllocationToken token, + CommandName commandName, + DateTime now, + Optional years, + DomainPricingLogic pricingLogic) + throws EppException { + int yearsForAction = years.orElse(1); + // We only support token discounts on creates or renews + return switch (commandName) { + case CREATE -> + pricingLogic + .getCreatePrice( + tld, domainName, now, yearsForAction, false, false, Optional.of(token)) + .getTotalCost() + .getAmount(); + case RENEW -> + pricingLogic + .getRenewPrice(tld, domainName, now, yearsForAction, null, Optional.of(token)) + .getTotalCost() + .getAmount(); + default -> BigDecimal.ZERO; + }; } /** Loads a given token and validates it against the registrar, time, etc */ @@ -253,8 +292,7 @@ private static void validateTokenEntity( // Tokens without status transitions will just have a single-entry NOT_STARTED map, so only // check the status transitions map if it's non-trivial. if (token.getTokenStatusTransitions().size() > 1 - && !AllocationToken.TokenStatus.VALID.equals( - token.getTokenStatusTransitions().getValueAtTime(now))) { + && !TokenStatus.VALID.equals(token.getTokenStatusTransitions().getValueAtTime(now))) { throw new AllocationTokenNotInPromotionException(); } @@ -303,7 +341,7 @@ public static class AllocationTokenNotValidForDomainException AllocationTokenNotValidForDomainException() { super("Alloc token invalid for domain"); } - } + } /** The allocation token is invalid. */ public static class NonexistentAllocationTokenException extends AuthorizationErrorException { diff --git a/core/src/main/java/google/registry/model/tld/label/ReservedList.java b/core/src/main/java/google/registry/model/tld/label/ReservedList.java index 70a81c64f58..4020254303e 100644 --- a/core/src/main/java/google/registry/model/tld/label/ReservedList.java +++ b/core/src/main/java/google/registry/model/tld/label/ReservedList.java @@ -261,8 +261,7 @@ private static ImmutableSet getReservedListEntries( public static ImmutableSet loadReservedLists( ImmutableSet reservedListNames) { return cache.getAll(reservedListNames).values().stream() - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(toImmutableSet()); } diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index baec62dc01d..131c22f2f54 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -579,8 +579,7 @@ RdapRegistrarEntity createRdapRegistrarEntity( ImmutableList registrarPocs = registrar.getPocsFromReplica().stream() .map(RdapJsonFormatter::makeRdapJsonForRegistrarPoc) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .filter( poc -> outputDataType == OutputDataType.FULL diff --git a/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java b/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java index a6a0f14f0cd..cc257650434 100644 --- a/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java +++ b/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java @@ -89,8 +89,7 @@ private static ImmutableMap, Domain> loadRedeemedDomains( ImmutableList> domainKeys = tokens.stream() .map(AllocationToken::getRedemptionHistoryId) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .map(hi -> tm().loadByKey(VKey.create(DomainHistory.class, hi))) .map(dh -> VKey.create(Domain.class, dh.getRepoId())) .collect(toImmutableList()); diff --git a/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java b/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java index cfeda091a88..29467c68a87 100644 --- a/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java +++ b/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java @@ -54,8 +54,7 @@ public ImmutableSet loadObjects() { tm().loadAllOf(PremiumList.class).stream() .map(PremiumList::getName) .map(PremiumListDao::getLatestRevision) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(toImmutableSortedSet(Comparator.comparing(PremiumList::getName)))); } } diff --git a/core/src/main/java/google/registry/tools/server/ListReservedListsAction.java b/core/src/main/java/google/registry/tools/server/ListReservedListsAction.java index 2692c4779e2..4e940429a38 100644 --- a/core/src/main/java/google/registry/tools/server/ListReservedListsAction.java +++ b/core/src/main/java/google/registry/tools/server/ListReservedListsAction.java @@ -52,8 +52,7 @@ public ImmutableSet loadObjects() { tm().loadAllOf(ReservedList.class).stream() .map(ReservedList::getName) .map(ReservedListDao::getLatestRevision) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(toImmutableSortedSet(Comparator.comparing(ReservedList::getName)))); } } 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 238eb6e17f0..f1ae4f4855f 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -1577,7 +1577,7 @@ void testSuccess_onlyUseFirstValidDefaultToken() throws Exception { persistHosts(); setupDefaultToken("aaaaa", 0, "TheRegistrar"); setupDefaultTokenWithDiscount(); - runTest_defaultToken("aaaaa"); + runTest_defaultToken("bbbbb"); } @Test diff --git a/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java index 8b2f3a4e325..6694e221d74 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java @@ -1243,7 +1243,7 @@ void testSuccess_usesDefaultToken() throws Exception { new AllocationToken.Builder() .setToken("aaaaa") .setTokenType(DEFAULT_PROMO) - .setDiscountFraction(0.5) + .setDiscountFraction(0.9) .setDiscountYears(1) .setAllowedTlds(ImmutableSet.of("tld")) .build()); @@ -1271,8 +1271,8 @@ void testSuccess_usesDefaultToken() throws Exception { assertThat(billingEvent.getTargetId()).isEqualTo("example.tld"); assertThat(billingEvent.getAllocationToken().get().getKey()) .isEqualTo(defaultToken1.getToken()); - // Price is 50% off the first year only. Non-discounted price is $11. - assertThat(billingEvent.getCost()).isEqualTo(Money.of(USD, 16.5)); + // Price is 90% off the first year only. Non-discounted price is $11. + assertThat(billingEvent.getCost()).isEqualTo(Money.of(USD, 12.10)); } @Test @@ -1412,7 +1412,7 @@ void testSuccess_doesNotApplyNonPremiumDefaultTokenToPremiumName_std_v1() throws } @Test - void testSuccess_onlyUsesFirstValidToken() throws Exception { + void testSuccess_usesCheapestValidToken() throws Exception { setEppInput("domain_renew.xml", ImmutableMap.of("DOMAIN", "example.tld", "YEARS", "2")); persistDomain(); AllocationToken defaultToken1 = @@ -1459,10 +1459,9 @@ void testSuccess_onlyUsesFirstValidToken() throws Exception { BillingEvent billingEvent = Iterables.getOnlyElement(DatabaseHelper.loadAllOf(BillingEvent.class)); assertThat(billingEvent.getTargetId()).isEqualTo("example.tld"); - assertThat(billingEvent.getAllocationToken().get().getKey()) - .isEqualTo(defaultToken2.getToken()); - // Price is 50% off the first year only. Non-discounted price is $11. - assertThat(billingEvent.getCost()).isEqualTo(Money.of(USD, 16.5)); + assertThat(billingEvent.getAllocationToken().get().getKey()).isEqualTo("ccccc"); + // Price is 75% off the first year only. Non-discounted price is $11. + assertThat(billingEvent.getCost()).isEqualTo(Money.of(USD, 13.75)); } @Test diff --git a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java index 7e59884ff9b..8c60f92581a 100644 --- a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java +++ b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java @@ -35,6 +35,8 @@ import com.google.common.collect.ImmutableSortedMap; import com.google.common.net.InternetDomainName; import google.registry.flows.EppException; +import google.registry.flows.custom.DomainPricingCustomLogic; +import google.registry.flows.domain.DomainPricingLogic; import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotInPromotionException; import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotValidForRegistrarException; import google.registry.flows.domain.token.AllocationTokenFlowUtils.NonexistentAllocationTokenException; @@ -65,6 +67,9 @@ class AllocationTokenFlowUtilsTest { private final AllocationTokenExtension allocationTokenExtension = mock(AllocationTokenExtension.class); + private final DomainPricingLogic domainPricingLogic = + new DomainPricingLogic(new DomainPricingCustomLogic(null, null, null)); + private Tld tld; @BeforeEach @@ -140,7 +145,9 @@ void testSuccess_loadOrDefault_fromExtensionEvenWhenDefaultPresent() throws Exce Optional.of(allocationTokenExtension), tld, "example.tld", - CommandName.CREATE)) + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)) .hasValue(token); } @@ -154,7 +161,9 @@ void testSuccess_loadOrDefault_defaultWhenNonePresent() throws Exception { Optional.empty(), tld, "example.tld", - CommandName.CREATE)) + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)) .hasValue(defaultToken); } @@ -176,7 +185,9 @@ void testSuccess_loadOrDefault_defaultWhenTokenIsPresentButNotApplicable() throw Optional.of(allocationTokenExtension), tld, "example.tld", - CommandName.CREATE)) + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)) .hasValue(defaultToken); } @@ -299,7 +310,9 @@ void testFailure_loadOrDefault_badTokenProvided() throws Exception { Optional.of(allocationTokenExtension), tld, "example.tld", - CommandName.CREATE)); + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)); } @Test @@ -311,7 +324,9 @@ void testFailure_loadOrDefault_noValidTokens() throws Exception { Optional.empty(), tld, "example.tld", - CommandName.CREATE)) + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)) .isEmpty(); } @@ -329,7 +344,93 @@ void testFailure_loadOrDefault_badDomainName() throws Exception { Optional.of(allocationTokenExtension), tld, "example.tld", - CommandName.CREATE)); + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)); + } + + @Test + void testSuccess_default_cheaperTokenUsed() throws Exception { + AllocationToken cheaperToken = + persistResource( + new AllocationToken.Builder() + .setToken("cheaperToken") + .setDiscountFraction(0.5) + .setAllowedTlds(ImmutableSet.of("tld")) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setTokenType(DEFAULT_PROMO) + .build()); + AllocationToken moreExpensiveToken = + persistResource( + new AllocationToken.Builder() + .setToken("moreExpensiveToken") + .setDiscountFraction(0.1) + .setAllowedTlds(ImmutableSet.of("tld")) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setTokenType(DEFAULT_PROMO) + .build()); + // List the more expensive token first to ensure that we don't just pick the first valid one + tld = + persistResource( + tld.asBuilder() + .setDefaultPromoTokens( + ImmutableList.of(moreExpensiveToken.createVKey(), cheaperToken.createVKey())) + .build()); + + assertThat( + AllocationTokenFlowUtils.loadTokenFromExtensionOrGetDefault( + "TheRegistrar", + clock.nowUtc(), + Optional.empty(), + tld, + "example.tld", + CommandName.CREATE, + Optional.of(1), + domainPricingLogic)) + .hasValue(cheaperToken); + } + + @Test + void testSuccess_default_twoYearsIsCheaper() throws Exception { + AllocationToken longerToken = + persistResource( + new AllocationToken.Builder() + .setToken("longerToken") + .setDiscountFraction(0.4) + .setDiscountYears(2) + .setAllowedTlds(ImmutableSet.of("tld")) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setTokenType(DEFAULT_PROMO) + .build()); + AllocationToken shorterToken = + persistResource( + new AllocationToken.Builder() + .setToken("shorterToken") + .setDiscountFraction(0.5) + .setDiscountYears(1) + .setAllowedTlds(ImmutableSet.of("tld")) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setTokenType(DEFAULT_PROMO) + .build()); + tld = + persistResource( + tld.asBuilder() + .setDefaultPromoTokens( + ImmutableList.of(shorterToken.createVKey(), longerToken.createVKey())) + .build()); + + // The token with the smaller discount fraction should be chosen because it runs for 2 years + assertThat( + AllocationTokenFlowUtils.loadTokenFromExtensionOrGetDefault( + "TheRegistrar", + clock.nowUtc(), + Optional.empty(), + tld, + "example.tld", + CommandName.CREATE, + Optional.of(2), + domainPricingLogic)) + .hasValue(longerToken); } private AllocationToken persistDefaultToken() { diff --git a/db/src/test/java/google/registry/sql/flyway/FlywayDeadlockTest.java b/db/src/test/java/google/registry/sql/flyway/FlywayDeadlockTest.java index e5759e4be78..d8232aed953 100644 --- a/db/src/test/java/google/registry/sql/flyway/FlywayDeadlockTest.java +++ b/db/src/test/java/google/registry/sql/flyway/FlywayDeadlockTest.java @@ -178,8 +178,7 @@ static ImmutableSet parseDdlScript(Path path) { .filter(line -> !line.isBlank()) .collect(joining(" "))) .map(FlywayDeadlockTest::getDdlLockedElementName) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(toImmutableSet()); } catch (IOException e) { throw new RuntimeException(e);