From ba54208dadf2e8d3c927e377525be9c32ac21011 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Fri, 3 Nov 2023 14:33:34 -0400 Subject: [PATCH] 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 --- .../flows/domain/DomainCheckFlow.java | 22 ++++++------ .../fee/FeeQueryCommandExtensionItem.java | 22 ++++++++---- .../flows/domain/DomainCheckFlowTest.java | 36 +++++++++++++++++++ ...ck_fee_premium_response_v12_renew_only.xml | 33 +++++++++++++++++ ...omain_check_fee_premium_v06_renew_only.xml | 18 ++++++++++ ...in_check_fee_premium_v06_transfer_only.xml | 18 ++++++++++ ...omain_check_fee_premium_v12_renew_only.xml | 15 ++++++++ ..._response_domain_exists_v06_renew_only.xml | 30 ++++++++++++++++ ...sponse_domain_exists_v06_transfer_only.xml | 30 ++++++++++++++++ package-lock.json | 14 ++++++-- 10 files changed, 219 insertions(+), 19 deletions(-) create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_response_v12_renew_only.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_renew_only.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_transfer_only.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v12_renew_only.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_renew_only.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_transfer_only.xml 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 89fd18faf..44ac3f616 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java @@ -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 responseItems = new ImmutableList.Builder<>(); ImmutableMap domainObjs = - loadDomainsForRestoreChecks(feeCheck, domainNames, existingDomains); + loadDomainsForChecks(feeCheck, domainNames, existingDomains); ImmutableMap 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. * - *

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. + *

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. * *

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 loadDomainsForRestoreChecks( + private ImmutableMap loadDomainsForChecks( FeeCheckCommandExtension feeCheck, ImmutableMap domainNames, ImmutableMap> 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(); } diff --git a/core/src/main/java/google/registry/model/domain/fee/FeeQueryCommandExtensionItem.java b/core/src/main/java/google/registry/model/domain/fee/FeeQueryCommandExtensionItem.java index 33d7dee8d..2d98090bd 100644 --- a/core/src/main/java/google/registry/model/domain/fee/FeeQueryCommandExtensionItem.java +++ b/core/src/main/java/google/registry/model/domain/fee/FeeQueryCommandExtensionItem.java @@ -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. */ diff --git a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java index cfc677408..73534ae34 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java @@ -1072,6 +1072,30 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase + + + Command completed successfully + + + + + rich.example + In use + + + + + + + + rich.example + + + 1 + %RENEWPRICE% + + + + + + ABC-12345 + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_renew_only.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_renew_only.xml new file mode 100644 index 000000000..f1dc6f1ef --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_renew_only.xml @@ -0,0 +1,18 @@ + + + + + rich.example + + + + + + rich.example + renew + + + + ABC-12345 + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_transfer_only.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_transfer_only.xml new file mode 100644 index 000000000..9d560d42d --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v06_transfer_only.xml @@ -0,0 +1,18 @@ + + + + + rich.example + + + + + + rich.example + transfer + + + + ABC-12345 + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v12_renew_only.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v12_renew_only.xml new file mode 100644 index 000000000..776f8febd --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_premium_v12_renew_only.xml @@ -0,0 +1,15 @@ + + + + + rich.example + + + + + + + + ABC-12345 + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_renew_only.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_renew_only.xml new file mode 100644 index 000000000..827249604 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_renew_only.xml @@ -0,0 +1,30 @@ + + + + Command completed successfully + + + + + rich.example + In use + + + + + + + rich.example + USD + renew + 1 + %RENEWPRICE% + + + + + ABC-12345 + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_transfer_only.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_transfer_only.xml new file mode 100644 index 000000000..c1255db77 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_response_domain_exists_v06_transfer_only.xml @@ -0,0 +1,30 @@ + + + + Command completed successfully + + + + + rich.example + In use + + + + + + + rich.example + USD + transfer + 1 + %RENEWPRICE% + + + + + ABC-12345 + server-trid + + + diff --git a/package-lock.json b/package-lock.json index a0fac1ee1..9b77d9705 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,10 +1,18 @@ { "name": "nomulus", "version": "1.0.0", - "lockfileVersion": 1, + "lockfileVersion": 3, "requires": true, - "dependencies": { - "google-closure-library": { + "packages": { + "": { + "name": "nomulus", + "version": "1.0.0", + "license": "Apache-2.0", + "dependencies": { + "google-closure-library": "20210406.0.0" + } + }, + "node_modules/google-closure-library": { "version": "20210406.0.0", "resolved": "https://registry.npmjs.org/google-closure-library/-/google-closure-library-20210406.0.0.tgz", "integrity": "sha512-1lAC/KC9R2QM6nygniM0pRcGrv5bkCUrIZb2hXFxLtAkA+zRiVeWtRYpFWDHXXJzkavKjsn9upiffL4x/nmmVg=="