Also load domains for domain checks of type renew/transfer (#2207)

The domains (and their associated billing recurrences) were already being loaded
to check restores, but they also now need to be loaded for renews and transfers
as well, as the billing renewal behavior on the recurrence could be modifying
the relevant renew price that should be shown. (The renew price is used for
transfers as well.)

See https://buganizer.corp.google.com/issues/306212810
This commit is contained in:
Ben McIlwain 2023-11-03 14:33:34 -04:00 committed by GitHub
parent b5e131ecba
commit ba54208dad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 219 additions and 19 deletions

View file

@ -66,7 +66,6 @@ import google.registry.model.domain.DomainCommand.Check;
import google.registry.model.domain.fee.FeeCheckCommandExtension;
import google.registry.model.domain.fee.FeeCheckCommandExtensionItem;
import google.registry.model.domain.fee.FeeCheckResponseExtensionItem;
import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName;
import google.registry.model.domain.fee06.FeeCheckCommandExtensionV06;
import google.registry.model.domain.launch.LaunchCheckExtension;
import google.registry.model.domain.token.AllocationToken;
@ -272,7 +271,7 @@ public final class DomainCheckFlow implements TransactionalFlow {
ImmutableList.Builder<FeeCheckResponseExtensionItem> responseItems =
new ImmutableList.Builder<>();
ImmutableMap<String, Domain> domainObjs =
loadDomainsForRestoreChecks(feeCheck, domainNames, existingDomains);
loadDomainsForChecks(feeCheck, domainNames, existingDomains);
ImmutableMap<String, BillingRecurrence> recurrences = loadRecurrencesForDomains(domainObjs);
for (FeeCheckCommandExtensionItem feeCheckItem : feeCheck.getItems()) {
@ -335,17 +334,20 @@ public final class DomainCheckFlow implements TransactionalFlow {
}
/**
* Loads and returns all existing domains that are having restore fees checked.
* Loads and returns all existing domains that are having restore/renew/transfer fees checked.
*
* <p>This is necessary so that we can check their expiration dates to determine if a one-year
* renewal is part of the cost of a restore.
* <p>These need to be loaded for renews and transfers because there could be a relevant {@link
* google.registry.model.billing.BillingBase.RenewalPriceBehavior} on the {@link
* BillingRecurrence} affecting the price. They also need to be loaded for restores so that we can
* check their expiration dates to determine if a one-year renewal is part of the cost of a
* restore.
*
* <p>This may be resource-intensive for large checks of many restore fees, but those are
* comparatively rare, and we are at least using an in-memory cache. Also, this will get a lot
* nicer in Cloud SQL when we can SELECT just the fields we want rather than having to load the
* entire entity.
*/
private ImmutableMap<String, Domain> loadDomainsForRestoreChecks(
private ImmutableMap<String, Domain> loadDomainsForChecks(
FeeCheckCommandExtension<?, ?> feeCheck,
ImmutableMap<String, InternetDomainName> domainNames,
ImmutableMap<String, VKey<Domain>> existingDomains) {
@ -354,18 +356,18 @@ public final class DomainCheckFlow implements TransactionalFlow {
// The V06 fee extension supports specifying the command fees to check on a per-domain basis.
restoreCheckDomains =
feeCheck.getItems().stream()
.filter(fc -> fc.getCommandName() == CommandName.RESTORE)
.filter(fc -> fc.getCommandName().shouldLoadDomainForCheck())
.map(FeeCheckCommandExtensionItem::getDomainName)
.distinct()
.collect(toImmutableList());
} else if (feeCheck.getItems().stream()
.anyMatch(fc -> fc.getCommandName() == CommandName.RESTORE)) {
.anyMatch(fc -> fc.getCommandName().shouldLoadDomainForCheck())) {
// The more recent fee extension versions support specifying the command fees to check only on
// the overall domain check, not per-domain.
restoreCheckDomains = ImmutableList.copyOf(domainNames.keySet());
} else {
// Fall-through case for more recent fee extension versions when the restore fee isn't being
// checked.
// Fall-through case for more recent fee extension versions when the restore/renew/transfer
// fees aren't being checked.
restoreCheckDomains = ImmutableList.of();
}

View file

@ -34,12 +34,14 @@ public abstract class FeeQueryCommandExtensionItem extends ImmutableObject {
/** The name of a command that might have an associated fee. */
public enum CommandName {
UNKNOWN,
CREATE,
RENEW,
TRANSFER,
RESTORE,
UPDATE;
UNKNOWN(false),
CREATE(false),
RENEW(true),
TRANSFER(true),
RESTORE(true),
UPDATE(false);
private final boolean loadDomainForCheck;
public static CommandName parseKnownCommand(String string) {
try {
@ -52,6 +54,14 @@ public abstract class FeeQueryCommandExtensionItem extends ImmutableObject {
+ " UPDATE");
}
}
CommandName(boolean loadDomainForCheck) {
this.loadDomainForCheck = loadDomainForCheck;
}
public boolean shouldLoadDomainForCheck() {
return this.loadDomainForCheck;
}
}
/** The default validity period (if not specified) is 1 year for all operations. */

View file

@ -1072,6 +1072,30 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase<DomainCheckFlow, Dom
ImmutableMap.of("RENEWPRICE", "11.00")));
}
@Test
void testFeeExtension_existingPremiumDomain_withNonPremiumRenewalBehavior_renewPriceOnly()
throws Exception {
createTld("example");
persistBillingRecurrenceForDomain(persistActiveDomain("rich.example"), NONPREMIUM, null);
setEppInput("domain_check_fee_premium_v06_renew_only.xml");
runFlowAssertResponse(
loadFile(
"domain_check_fee_response_domain_exists_v06_renew_only.xml",
ImmutableMap.of("RENEWPRICE", "11.00")));
}
@Test
void testFeeExtension_existingPremiumDomain_withNonPremiumRenewalBehavior_transferPriceOnly()
throws Exception {
createTld("example");
persistBillingRecurrenceForDomain(persistActiveDomain("rich.example"), NONPREMIUM, null);
setEppInput("domain_check_fee_premium_v06_transfer_only.xml");
runFlowAssertResponse(
loadFile(
"domain_check_fee_response_domain_exists_v06_transfer_only.xml",
ImmutableMap.of("RENEWPRICE", "11.00")));
}
@Test
void testFeeExtension_existingPremiumDomain_withSpecifiedRenewalBehavior() throws Exception {
createTld("example");
@ -1205,6 +1229,18 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase<DomainCheckFlow, Dom
runFlowAssertResponse(loadFile("domain_check_fee_premium_response_v12.xml"));
}
@Test
void testFeeExtension_premiumLabels_v12_specifiedPriceRenewal_renewPriceOnly() throws Exception {
createTld("example");
persistBillingRecurrenceForDomain(
persistActiveDomain("rich.example"), SPECIFIED, Money.of(USD, new BigDecimal("27.74")));
setEppInput("domain_check_fee_premium_v12_renew_only.xml");
runFlowAssertResponse(
loadFile(
"domain_check_fee_premium_response_v12_renew_only.xml",
ImmutableMap.of("RENEWPRICE", "27.74")));
}
@Test
void testFeeExtension_premiumLabels_doesNotApplyDefaultToken_v12() throws Exception {
createTld("example");

View file

@ -0,0 +1,33 @@
domain_check_fee_premium_response_v12.xml<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<response>
<result code="1000">
<msg>Command completed successfully</msg>
</result>
<resData>
<domain:chkData xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:cd>
<domain:name avail="false">rich.example</domain:name>
<domain:reason>In use</domain:reason>
</domain:cd>
</domain:chkData>
</resData>
<extension>
<fee:chkData xmlns:fee="urn:ietf:params:xml:ns:fee-0.12"
xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<fee:cd>
<fee:object>
<domain:name>rich.example</domain:name>
</fee:object>
<fee:command name="renew">
<fee:period unit="y">1</fee:period>
<fee:fee description="renew">%RENEWPRICE%</fee:fee>
</fee:command>
</fee:cd>
</fee:chkData>
</extension>
<trID>
<clTRID>ABC-12345</clTRID>
<svTRID>server-trid</svTRID>
</trID>
</response>
</epp>

View file

@ -0,0 +1,18 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<command>
<check>
<domain:check xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>rich.example</domain:name>
</domain:check>
</check>
<extension>
<fee:check xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:domain>
<fee:name>rich.example</fee:name>
<fee:command>renew</fee:command>
</fee:domain>
</fee:check>
</extension>
<clTRID>ABC-12345</clTRID>
</command>
</epp>

View file

@ -0,0 +1,18 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<command>
<check>
<domain:check xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>rich.example</domain:name>
</domain:check>
</check>
<extension>
<fee:check xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:domain>
<fee:name>rich.example</fee:name>
<fee:command>transfer</fee:command>
</fee:domain>
</fee:check>
</extension>
<clTRID>ABC-12345</clTRID>
</command>
</epp>

View file

@ -0,0 +1,15 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<command>
<check>
<domain:check xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>rich.example</domain:name>
</domain:check>
</check>
<extension>
<fee:check xmlns:fee="urn:ietf:params:xml:ns:fee-0.12">
<fee:command name="renew" />
</fee:check>
</extension>
<clTRID>ABC-12345</clTRID>
</command>
</epp>

View file

@ -0,0 +1,30 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<response>
<result code="1000">
<msg>Command completed successfully</msg>
</result>
<resData>
<domain:chkData xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:cd>
<domain:name avail="0">rich.example</domain:name>
<domain:reason>In use</domain:reason>
</domain:cd>
</domain:chkData>
</resData>
<extension>
<fee:chkData xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:cd xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:name>rich.example</fee:name>
<fee:currency>USD</fee:currency>
<fee:command>renew</fee:command>
<fee:period unit="y">1</fee:period>
<fee:fee description="renew">%RENEWPRICE%</fee:fee>
</fee:cd>
</fee:chkData>
</extension>
<trID>
<clTRID>ABC-12345</clTRID>
<svTRID>server-trid</svTRID>
</trID>
</response>
</epp>

View file

@ -0,0 +1,30 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<response>
<result code="1000">
<msg>Command completed successfully</msg>
</result>
<resData>
<domain:chkData xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:cd>
<domain:name avail="0">rich.example</domain:name>
<domain:reason>In use</domain:reason>
</domain:cd>
</domain:chkData>
</resData>
<extension>
<fee:chkData xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:cd xmlns:fee="urn:ietf:params:xml:ns:fee-0.6">
<fee:name>rich.example</fee:name>
<fee:currency>USD</fee:currency>
<fee:command>transfer</fee:command>
<fee:period unit="y">1</fee:period>
<fee:fee description="renew">%RENEWPRICE%</fee:fee>
</fee:cd>
</fee:chkData>
</extension>
<trID>
<clTRID>ABC-12345</clTRID>
<svTRID>server-trid</svTRID>
</trID>
</response>
</epp>