From 9d9c52791792576dd17cfb280fb19c97ad8f2f0b Mon Sep 17 00:00:00 2001 From: mcilwain Date: Wed, 21 Dec 2016 16:44:08 -0800 Subject: [PATCH] Reconcile FeesAndCredits handling in price customization Also adds a mechanism to ensure that fee extensions are included when custom pricing logic adds a custom fee, and fixes up the domain restore flow to properly use the restore price. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=142715136 --- docs/flows.md | 4 +- .../custom/DomainPricingCustomLogic.java | 2 +- .../flows/domain/DomainAllocateFlow.java | 1 - .../domain/DomainApplicationCreateFlow.java | 3 +- .../domain/DomainApplicationUpdateFlow.java | 11 +- .../flows/domain/DomainCreateFlow.java | 4 +- .../flows/domain/DomainFlowUtils.java | 40 ++-- .../flows/domain/DomainPricingLogic.java | 128 +++---------- .../flows/domain/DomainRenewFlow.java | 4 +- .../domain/DomainRestoreRequestFlow.java | 19 +- .../domain/DomainTransferRequestFlow.java | 3 +- .../flows/domain/DomainUpdateFlow.java | 13 +- .../registry/flows/domain/FeesAndCredits.java | 176 ++++++++++++++++++ .../custom/TestDomainPricingCustomLogic.java | 35 ++-- .../DomainApplicationUpdateFlowTest.java | 2 +- .../domain/DomainTransferRequestFlowTest.java | 32 +++- .../flows/domain/DomainUpdateFlowTest.java | 4 +- .../testdata/domain_transfer_request_fee.xml | 6 +- ...domain_transfer_request_response_fees.xml} | 7 + 19 files changed, 312 insertions(+), 182 deletions(-) create mode 100644 java/google/registry/flows/domain/FeesAndCredits.java rename javatests/google/registry/flows/domain/testdata/{domain_transfer_request_response_wildcard.xml => domain_transfer_request_response_fees.xml} (73%) diff --git a/docs/flows.md b/docs/flows.md index 9ef81bda9..2756b0f89 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -246,8 +246,8 @@ the domain to convert to a normal create and be billed for accordingly. * 2003 * At least one of 'add' or 'rem' is required on a secDNS update. - * Fees must be explicitly acknowledged when performing an update which is - not free. + * Fees must be explicitly acknowledged when performing an operation which + is not free. * Admin contact is required. * Technical contact is required. * 2004 diff --git a/java/google/registry/flows/custom/DomainPricingCustomLogic.java b/java/google/registry/flows/custom/DomainPricingCustomLogic.java index 67b7724e8..7955fd707 100644 --- a/java/google/registry/flows/custom/DomainPricingCustomLogic.java +++ b/java/google/registry/flows/custom/DomainPricingCustomLogic.java @@ -19,7 +19,7 @@ import com.google.common.net.InternetDomainName; import google.registry.flows.EppException; import google.registry.flows.SessionMetadata; import google.registry.flows.domain.DomainPricingLogic; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; +import google.registry.flows.domain.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainApplication; import google.registry.model.eppinput.EppInput; diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 69a662875..0457d8140 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -51,7 +51,6 @@ import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.Superuser; import google.registry.flows.FlowModule.TargetId; import google.registry.flows.TransactionalFlow; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; diff --git a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java index 8a3125089..c5c75197a 100644 --- a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java @@ -63,7 +63,6 @@ import google.registry.flows.custom.DomainApplicationCreateFlowCustomLogic.After import google.registry.flows.custom.DomainApplicationCreateFlowCustomLogic.BeforeResponseParameters; import google.registry.flows.custom.DomainApplicationCreateFlowCustomLogic.BeforeResponseReturnData; import google.registry.flows.custom.EntityChanges; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.DomainCommand.Create; @@ -223,7 +222,7 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { } FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); - validateFeeChallenge(targetId, tld, now, feeCreate, feesAndCredits.getTotalCost()); + validateFeeChallenge(targetId, tld, now, feeCreate, feesAndCredits); SecDnsCreateExtension secDnsCreate = validateSecDnsExtension(eppInput.getSingleExtension(SecDnsCreateExtension.class)); customLogic.afterValidation( diff --git a/java/google/registry/flows/domain/DomainApplicationUpdateFlow.java b/java/google/registry/flows/domain/DomainApplicationUpdateFlow.java index c0b53ed8f..f49670a21 100644 --- a/java/google/registry/flows/domain/DomainApplicationUpdateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationUpdateFlow.java @@ -54,8 +54,7 @@ import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.Superuser; import google.registry.flows.FlowModule.TargetId; import google.registry.flows.TransactionalFlow; -import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeUpdateException; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; +import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeOperationException; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.DomainCommand.Update; @@ -75,7 +74,6 @@ import google.registry.model.eppoutput.EppResponse; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; import javax.inject.Inject; -import org.joda.money.Money; import org.joda.time.DateTime; /** @@ -188,12 +186,11 @@ public class DomainApplicationUpdateFlow implements TransactionalFlow { // If the fee extension is present, validate it (even if the cost is zero, to check for price // mismatches). Don't rely on the the validateFeeChallenge check for feeUpdate nullness, because // it throws an error if the name is premium, and we don't want to do that here. - Money totalCost = feesAndCredits.getTotalCost(); if (feeUpdate != null) { - validateFeeChallenge(targetId, tld, now, feeUpdate, totalCost); - } else if (!totalCost.isZero()) { + validateFeeChallenge(targetId, tld, now, feeUpdate, feesAndCredits); + } else if (!feesAndCredits.getTotalCost().isZero()) { // If it's not present but the cost is not zero, throw an exception. - throw new FeesRequiredForNonFreeUpdateException(); + throw new FeesRequiredForNonFreeOperationException(feesAndCredits.getTotalCost()); } verifyNotInPendingDelete( add.getContacts(), diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index baa17140b..79db242b7 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -61,7 +61,6 @@ import google.registry.flows.custom.DomainCreateFlowCustomLogic; import google.registry.flows.custom.DomainCreateFlowCustomLogic.BeforeResponseParameters; import google.registry.flows.custom.DomainCreateFlowCustomLogic.BeforeResponseReturnData; import google.registry.flows.custom.EntityChanges; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; @@ -212,8 +211,7 @@ public class DomainCreateFlow implements TransactionalFlow { FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice(registry, targetId, now, years); - validateFeeChallenge( - targetId, registry.getTldStr(), now, feeCreate, feesAndCredits.getTotalCost()); + validateFeeChallenge(targetId, registry.getTldStr(), now, feeCreate, feesAndCredits); // Superusers can create reserved domains, force creations on domains that require a claims // notice without specifying a claims key, ignore the registry phase, and override blocks on // registering premium domains. diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index a305845d8..cc3bd17ee 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -56,7 +56,6 @@ import google.registry.flows.EppException.ParameterValueSyntaxErrorException; import google.registry.flows.EppException.RequiredParameterMissingException; import google.registry.flows.EppException.StatusProhibitsOperationException; import google.registry.flows.EppException.UnimplementedOptionException; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.flows.exceptions.ResourceAlreadyExistsException; import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException; import google.registry.model.EppResource; @@ -674,8 +673,7 @@ public class DomainFlowUtils { String tld, DateTime priceTime, final FeeTransformCommandExtension feeCommand, - Money cost, - Money... otherCosts) + FeesAndCredits feesAndCredits) throws EppException { Registry registry = Registry.get(tld); if (registry.getPremiumPriceAckRequired() @@ -683,9 +681,17 @@ public class DomainFlowUtils { && feeCommand == null) { throw new FeesRequiredForPremiumNameException(); } + + // Check for the case where a fee command extension was required but not provided. + // This only happens when the total fees are non-zero and include custom fees requiring the + // extension. if (feeCommand == null) { - return; + if (feesAndCredits.getTotalCost().isZero() || !feesAndCredits.isFeeExtensionRequired()) { + return; + } + throw new FeesRequiredForNonFreeOperationException(feesAndCredits.getTotalCost()); } + List fees = feeCommand.getFees(); // The schema guarantees that at least one fee will be present. checkState(!fees.isEmpty()); @@ -710,16 +716,11 @@ public class DomainFlowUtils { throw new CurrencyValueScaleException(); } - Money costTotal = cost; - for (Money otherCost : otherCosts) { - costTotal = costTotal.plus(otherCost); - } - - if (!feeTotal.getCurrencyUnit().equals(costTotal.getCurrencyUnit())) { + if (!feeTotal.getCurrencyUnit().equals(feesAndCredits.getCurrency())) { throw new CurrencyUnitMismatchException(); } - if (!feeTotal.equals(costTotal)) { - throw new FeesMismatchException(costTotal); + if (!feeTotal.equals(feesAndCredits.getTotalCost())) { + throw new FeesMismatchException(feesAndCredits.getTotalCost()); } } @@ -1282,10 +1283,17 @@ public class DomainFlowUtils { } } - /** Fees must be explicitly acknowledged when performing an update which is not free. */ - static class FeesRequiredForNonFreeUpdateException extends RequiredParameterMissingException { - FeesRequiredForNonFreeUpdateException() { - super("Fees must be explicitly acknowledged when performing an update which is not free."); + /** Fees must be explicitly acknowledged when performing an operation which is not free. */ + static class FeesRequiredForNonFreeOperationException extends RequiredParameterMissingException { + FeesRequiredForNonFreeOperationException() { + super("Fees must be explicitly acknowledged when performing an operation which is not free."); + } + + public FeesRequiredForNonFreeOperationException(Money expectedFee) { + super( + "Fees must be explicitly acknowledged when performing an operation which is not free." + + " The total fee is: " + + expectedFee); } } diff --git a/java/google/registry/flows/domain/DomainPricingLogic.java b/java/google/registry/flows/domain/DomainPricingLogic.java index 50be79e34..8923f762f 100644 --- a/java/google/registry/flows/domain/DomainPricingLogic.java +++ b/java/google/registry/flows/domain/DomainPricingLogic.java @@ -14,16 +14,12 @@ package google.registry.flows.domain; -import static com.google.common.collect.Iterables.concat; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.pricing.PricingEngineProxy.getDomainCreateCost; import static google.registry.pricing.PricingEngineProxy.getDomainFeeClass; import static google.registry.pricing.PricingEngineProxy.getDomainRenewCost; -import static google.registry.util.CollectionUtils.nullToEmpty; -import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.base.Optional; -import com.google.common.collect.ImmutableList; import com.google.common.net.InternetDomainName; import com.googlecode.objectify.Key; import google.registry.flows.EppException; @@ -35,12 +31,10 @@ import google.registry.flows.custom.DomainPricingCustomLogic.RenewPriceParameter import google.registry.flows.custom.DomainPricingCustomLogic.RestorePriceParameters; import google.registry.flows.custom.DomainPricingCustomLogic.TransferPriceParameters; import google.registry.flows.custom.DomainPricingCustomLogic.UpdatePriceParameters; -import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.LrpTokenEntity; import google.registry.model.domain.fee.BaseFee; import google.registry.model.domain.fee.BaseFee.FeeType; -import google.registry.model.domain.fee.Credit; import google.registry.model.domain.fee.Fee; import google.registry.model.registry.Registry; import javax.inject.Inject; @@ -61,83 +55,6 @@ public final class DomainPricingLogic { @Inject DomainPricingLogic() {} - /** A collection of fees and credits for a specific EPP transform. */ - public static final class FeesAndCredits extends ImmutableObject { - - private final CurrencyUnit currency; - private final ImmutableList fees; - private final ImmutableList credits; - - /** Constructs a new instance. The currency must be the same across all fees and credits. */ - public FeesAndCredits(CurrencyUnit currency, BaseFee... baseFees) { - this.currency = checkArgumentNotNull(currency, "Currency may not be null in FeesAndCredits."); - ImmutableList.Builder feeBuilder = new ImmutableList.Builder<>(); - ImmutableList.Builder creditBuilder = new ImmutableList.Builder<>(); - for (BaseFee feeOrCredit : baseFees) { - if (feeOrCredit instanceof Credit) { - creditBuilder.add((Credit) feeOrCredit); - } else { - feeBuilder.add((Fee) feeOrCredit); - } - } - this.fees = feeBuilder.build(); - this.credits = creditBuilder.build(); - } - - private Money getTotalCostForType(FeeType type) { - Money result = Money.zero(currency); - checkArgumentNotNull(type); - for (Fee fee : fees) { - if (fee.getType() == type) { - result = result.plus(fee.getCost()); - } - } - return result; - } - - /** Returns the total cost of all fees and credits for the event. */ - public Money getTotalCost() { - Money result = Money.zero(currency); - for (Fee fee : fees) { - result = result.plus(fee.getCost()); - } - for (Credit credit : credits) { - result = result.plus(credit.getCost()); - } - return result; - } - - /** Returns the create cost for the event. */ - public Money getCreateCost() { - return getTotalCostForType(FeeType.CREATE); - } - - /** Returns the EAP cost for the event. */ - public Money getEapCost() { - return getTotalCostForType(FeeType.EAP); - } - - /** Returns the list of fees for the event. */ - public ImmutableList getFees() { - return fees; - } - - /** Returns the list of credits for the event. */ - public ImmutableList getCredits() { - return ImmutableList.copyOf(nullToEmpty(credits)); - } - - /** Returns the currency for all fees in the event. */ - public final CurrencyUnit getCurrency() { - return currency; - } - - /** Returns all fees and credits for the event. */ - public ImmutableList getFeesAndCredits() { - return ImmutableList.copyOf(concat(getFees(), getCredits())); - } - } - /** Returns a new create price for the Pricer. */ public FeesAndCredits getCreatePrice( Registry registry, String domainName, DateTime date, int years) throws EppException { @@ -147,20 +64,18 @@ public final class DomainPricingLogic { BaseFee createFeeOrCredit = Fee.create(getDomainCreateCost(domainName, date, years).getAmount(), FeeType.CREATE); - FeesAndCredits feesAndCredits; - // Create fees for the cost and the EAP fee, if any. Fee eapFee = registry.getEapFeeFor(date); + FeesAndCredits.Builder feesBuilder = + new FeesAndCredits.Builder().setCurrency(currency).addFeeOrCredit(createFeeOrCredit); if (!eapFee.hasZeroCost()) { - feesAndCredits = new FeesAndCredits(currency, createFeeOrCredit, eapFee); - } else { - feesAndCredits = new FeesAndCredits(currency, createFeeOrCredit); + feesBuilder.addFeeOrCredit(eapFee); } // Apply custom logic to the create fee, if any. return customLogic.customizeCreatePrice( CreatePriceParameters.newBuilder() - .setFeesAndCredits(feesAndCredits) + .setFeesAndCredits(feesBuilder.build()) .setRegistry(registry) .setDomainName(InternetDomainName.from(domainName)) .setAsOfDate(date) @@ -183,8 +98,10 @@ public final class DomainPricingLogic { return customLogic.customizeRenewPrice( RenewPriceParameters.newBuilder() .setFeesAndCredits( - new FeesAndCredits( - registry.getCurrency(), Fee.create(renewCost.getAmount(), FeeType.RENEW))) + new FeesAndCredits.Builder() + .setCurrency(registry.getCurrency()) + .addFeeOrCredit(Fee.create(renewCost.getAmount(), FeeType.RENEW)) + .build()) .setRegistry(registry) .setDomainName(InternetDomainName.from(domainName)) .setAsOfDate(date) @@ -197,10 +114,13 @@ public final class DomainPricingLogic { public FeesAndCredits getRestorePrice(Registry registry, String domainName, DateTime date) throws EppException { FeesAndCredits feesAndCredits = - new FeesAndCredits( - registry.getCurrency(), - Fee.create(getDomainRenewCost(domainName, date, 1).getAmount(), FeeType.RENEW), - Fee.create(registry.getStandardRestoreCost().getAmount(), FeeType.RESTORE)); + new FeesAndCredits.Builder() + .setCurrency(registry.getCurrency()) + .addFeeOrCredit( + Fee.create(getDomainRenewCost(domainName, date, 1).getAmount(), FeeType.RENEW)) + .addFeeOrCredit( + Fee.create(registry.getStandardRestoreCost().getAmount(), FeeType.RESTORE)) + .build(); return customLogic.customizeRestorePrice( RestorePriceParameters.newBuilder() .setFeesAndCredits(feesAndCredits) @@ -221,8 +141,10 @@ public final class DomainPricingLogic { return customLogic.customizeTransferPrice( TransferPriceParameters.newBuilder() .setFeesAndCredits( - new FeesAndCredits( - registry.getCurrency(), Fee.create(renewCost.getAmount(), FeeType.RENEW))) + new FeesAndCredits.Builder() + .setCurrency(registry.getCurrency()) + .addFeeOrCredit(Fee.create(renewCost.getAmount(), FeeType.RENEW)) + .build()) .setRegistry(registry) .setDomainName(InternetDomainName.from(domainName)) .setAsOfDate(transferDate) @@ -238,7 +160,11 @@ public final class DomainPricingLogic { Fee.create(Money.zero(registry.getCurrency()).getAmount(), FeeType.UPDATE); return customLogic.customizeUpdatePrice( UpdatePriceParameters.newBuilder() - .setFeesAndCredits(new FeesAndCredits(currency, feeOrCredit)) + .setFeesAndCredits( + new FeesAndCredits.Builder() + .setCurrency(currency) + .addFeeOrCredit(feeOrCredit) + .build()) .setRegistry(registry) .setDomainName(InternetDomainName.from(domainName)) .setAsOfDate(date) @@ -253,7 +179,11 @@ public final class DomainPricingLogic { Fee.create(Money.zero(registry.getCurrency()).getAmount(), FeeType.UPDATE); return customLogic.customizeApplicationUpdatePrice( ApplicationUpdatePriceParameters.newBuilder() - .setFeesAndCredits(new FeesAndCredits(registry.getCurrency(), feeOrCredit)) + .setFeesAndCredits( + new FeesAndCredits.Builder() + .setCurrency(registry.getCurrency()) + .setFeesAndCredits(feeOrCredit) + .build()) .setRegistry(registry) .setDomainApplication(application) .setAsOfDate(date) diff --git a/java/google/registry/flows/domain/DomainRenewFlow.java b/java/google/registry/flows/domain/DomainRenewFlow.java index 96e392b47..aa1efbfc4 100644 --- a/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/java/google/registry/flows/domain/DomainRenewFlow.java @@ -49,7 +49,6 @@ import google.registry.flows.custom.DomainRenewFlowCustomLogic.BeforeResponsePar import google.registry.flows.custom.DomainRenewFlowCustomLogic.BeforeResponseReturnData; import google.registry.flows.custom.DomainRenewFlowCustomLogic.BeforeSaveParameters; import google.registry.flows.custom.EntityChanges; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.OneTime; @@ -140,8 +139,7 @@ public final class DomainRenewFlow implements TransactionalFlow { eppInput.getSingleExtension(FeeRenewCommandExtension.class); FeesAndCredits feesAndCredits = pricingLogic.getRenewPrice(Registry.get(existingDomain.getTld()), targetId, now, years); - validateFeeChallenge( - targetId, existingDomain.getTld(), now, feeRenew, feesAndCredits.getTotalCost()); + validateFeeChallenge(targetId, existingDomain.getTld(), now, feeRenew, feesAndCredits); customLogic.afterValidation( AfterValidationParameters.newBuilder() .setExistingDomain(existingDomain) diff --git a/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index 6100b8dc2..025d3d0da 100644 --- a/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -42,7 +42,6 @@ import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.Superuser; import google.registry.flows.FlowModule.TargetId; import google.registry.flows.TransactionalFlow; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.OneTime; @@ -128,17 +127,16 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { Update command = (Update) resourceCommand; DateTime now = ofy().getTransactionTime(); DomainResource existingDomain = loadAndVerifyExistence(DomainResource.class, targetId, now); - Money restoreCost = Registry.get(existingDomain.getTld()).getStandardRestoreCost(); FeesAndCredits feesAndCredits = - pricingLogic.getRenewPrice(Registry.get(existingDomain.getTld()), targetId, now, 1); + pricingLogic.getRestorePrice(Registry.get(existingDomain.getTld()), targetId, now); FeeUpdateCommandExtension feeUpdate = eppInput.getSingleExtension(FeeUpdateCommandExtension.class); - Money totalCost = feesAndCredits.getTotalCost(); - verifyRestoreAllowed(command, existingDomain, restoreCost, totalCost, feeUpdate, now); + verifyRestoreAllowed(command, existingDomain, feeUpdate, feesAndCredits, now); HistoryEntry historyEntry = buildHistory(existingDomain, now); ImmutableSet.Builder entitiesToSave = new ImmutableSet.Builder<>(); entitiesToSave.addAll( - createRestoreAndRenewBillingEvents(historyEntry, restoreCost, totalCost, now)); + createRestoreAndRenewBillingEvents( + historyEntry, feesAndCredits.getRestoreCost(), feesAndCredits.getRenewCost(), now)); // We don't preserve the original expiration time of the domain when we restore, since doing so // would require us to know if they received a grace period refund when they deleted the domain, // and to charge them for that again. Instead, we just say that all restores get a fresh year of @@ -162,7 +160,9 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { ofy().delete().key(existingDomain.getDeletePollMessage()); dnsQueue.addDomainRefreshTask(existingDomain.getFullyQualifiedDomainName()); return responseBuilder - .setExtensions(createResponseExtensions(restoreCost, totalCost, feeUpdate)) + .setExtensions( + createResponseExtensions( + feesAndCredits.getRestoreCost(), feesAndCredits.getRenewCost(), feeUpdate)) .build(); } @@ -177,9 +177,8 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { private void verifyRestoreAllowed( Update command, DomainResource existingDomain, - Money restoreCost, - Money renewCost, FeeUpdateCommandExtension feeUpdate, + FeesAndCredits feesAndCredits, DateTime now) throws EppException { verifyOptionalAuthInfo(authInfo, existingDomain); if (!isSuperuser) { @@ -196,7 +195,7 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { throw new DomainNotEligibleForRestoreException(); } checkAllowedAccessToTld(clientId, existingDomain.getTld()); - validateFeeChallenge(targetId, existingDomain.getTld(), now, feeUpdate, restoreCost, renewCost); + validateFeeChallenge(targetId, existingDomain.getTld(), now, feeUpdate, feesAndCredits); } private ImmutableSet createRestoreAndRenewBillingEvents( diff --git a/java/google/registry/flows/domain/DomainTransferRequestFlow.java b/java/google/registry/flows/domain/DomainTransferRequestFlow.java index 1e3c408d7..de9b2afff 100644 --- a/java/google/registry/flows/domain/DomainTransferRequestFlow.java +++ b/java/google/registry/flows/domain/DomainTransferRequestFlow.java @@ -44,7 +44,6 @@ import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.Superuser; import google.registry.flows.FlowModule.TargetId; import google.registry.flows.TransactionalFlow; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.flows.exceptions.AlreadyPendingTransferException; import google.registry.flows.exceptions.ObjectAlreadySponsoredException; import google.registry.model.billing.BillingEvent; @@ -145,7 +144,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { FeeTransferCommandExtension feeTransfer = eppInput.getSingleExtension(FeeTransferCommandExtension.class); FeesAndCredits feesAndCredits = pricingLogic.getTransferPrice(registry, targetId, now, years); - validateFeeChallenge(targetId, tld, now, feeTransfer, feesAndCredits.getTotalCost()); + validateFeeChallenge(targetId, tld, now, feeTransfer, feesAndCredits); HistoryEntry historyEntry = buildHistory(period, existingDomain, now); DateTime automaticTransferTime = now.plus(registry.getAutomaticTransferLength()); // The new expiration time if there is a server approval. diff --git a/java/google/registry/flows/domain/DomainUpdateFlow.java b/java/google/registry/flows/domain/DomainUpdateFlow.java index 80602cda0..5ad361b8c 100644 --- a/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -55,8 +55,7 @@ import google.registry.flows.custom.DomainUpdateFlowCustomLogic; import google.registry.flows.custom.DomainUpdateFlowCustomLogic.AfterValidationParameters; import google.registry.flows.custom.DomainUpdateFlowCustomLogic.BeforeSaveParameters; import google.registry.flows.custom.EntityChanges; -import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeUpdateException; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; +import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeOperationException; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; @@ -79,7 +78,6 @@ import google.registry.model.eppoutput.EppResponse; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; import javax.inject.Inject; -import org.joda.money.Money; import org.joda.time.DateTime; /** @@ -109,7 +107,7 @@ import org.joda.time.DateTime; * @error {@link DomainFlowUtils.DuplicateContactForRoleException} * @error {@link DomainFlowUtils.EmptySecDnsUpdateException} * @error {@link DomainFlowUtils.FeesMismatchException} - * @error {@link DomainFlowUtils.FeesRequiredForNonFreeUpdateException} + * @error {@link DomainFlowUtils.FeesRequiredForNonFreeOperationException} * @error {@link DomainFlowUtils.LinkedResourcesDoNotExistException} * @error {@link DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException} * @error {@link DomainFlowUtils.MaxSigLifeChangeNotSupportedException} @@ -219,12 +217,11 @@ public final class DomainUpdateFlow implements TransactionalFlow { // mismatches). Don't rely on the the validateFeeChallenge check for feeUpdate nullness, because // it throws an error if the name is premium, and we don't want to do that here. FeesAndCredits feesAndCredits = pricingLogic.getUpdatePrice(Registry.get(tld), targetId, now); - Money totalCost = feesAndCredits.getTotalCost(); if (feeUpdate != null) { - validateFeeChallenge(targetId, existingDomain.getTld(), now, feeUpdate, totalCost); - } else if (!totalCost.isZero()) { + validateFeeChallenge(targetId, existingDomain.getTld(), now, feeUpdate, feesAndCredits); + } else if (!feesAndCredits.getTotalCost().isZero()) { // If it's not present but the cost is not zero, throw an exception. - throw new FeesRequiredForNonFreeUpdateException(); + throw new FeesRequiredForNonFreeOperationException(feesAndCredits.getTotalCost()); } verifyNotInPendingDelete( add.getContacts(), diff --git a/java/google/registry/flows/domain/FeesAndCredits.java b/java/google/registry/flows/domain/FeesAndCredits.java new file mode 100644 index 000000000..71f42882b --- /dev/null +++ b/java/google/registry/flows/domain/FeesAndCredits.java @@ -0,0 +1,176 @@ +// Copyright 2016 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.flows.domain; + +import static com.google.common.collect.Iterables.concat; +import static google.registry.util.CollectionUtils.nullToEmpty; +import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; + +import com.google.common.collect.ImmutableList; +import google.registry.model.Buildable; +import google.registry.model.ImmutableObject; +import google.registry.model.domain.fee.BaseFee; +import google.registry.model.domain.fee.BaseFee.FeeType; +import google.registry.model.domain.fee.Credit; +import google.registry.model.domain.fee.Fee; +import org.joda.money.CurrencyUnit; +import org.joda.money.Money; + +/** A collection of fees and credits for a specific EPP transform. */ +public class FeesAndCredits extends ImmutableObject implements Buildable { + + private CurrencyUnit currency; + private boolean feeExtensionRequired; + private ImmutableList fees; + private ImmutableList credits; + + private Money getTotalCostForType(FeeType type) { + Money result = Money.zero(currency); + checkArgumentNotNull(type); + for (Fee fee : fees) { + if (fee.getType() == type) { + result = result.plus(fee.getCost()); + } + } + return result; + } + + /** Returns the total cost of all fees and credits for the event. */ + public Money getTotalCost() { + Money result = Money.zero(currency); + for (Fee fee : fees) { + result = result.plus(fee.getCost()); + } + for (Credit credit : credits) { + result = result.plus(credit.getCost()); + } + return result; + } + + /** Returns the create cost for the event. */ + public Money getCreateCost() { + return getTotalCostForType(FeeType.CREATE); + } + + /** Returns the EAP cost for the event. */ + public Money getEapCost() { + return getTotalCostForType(FeeType.EAP); + } + + /** Returns the renew cost for the event. */ + public Money getRenewCost() { + return getTotalCostForType(FeeType.RENEW); + } + + /** Returns the restore cost for the event. */ + public Money getRestoreCost() { + return getTotalCostForType(FeeType.RESTORE); + } + + /** Returns the list of fees for the event. */ + public ImmutableList getFees() { + return fees; + } + + /** Returns whether a custom fee is present that requires fee extension acknowledgement. */ + public boolean isFeeExtensionRequired() { + return feeExtensionRequired; + } + + /** Returns the list of credits for the event. */ + public ImmutableList getCredits() { + return ImmutableList.copyOf(nullToEmpty(credits)); + } + + /** Returns the currency for all fees in the event. */ + public final CurrencyUnit getCurrency() { + return currency; + } + + /** Returns all fees and credits for the event. */ + public ImmutableList getFeesAndCredits() { + return ImmutableList.copyOf(concat(getFees(), getCredits())); + } + + @Override + public Builder asBuilder() { + return new Builder(clone(this)); + } + + /** A builder for constructing {@link FeesAndCredits} objects, since they are immutable. */ + public static class Builder extends Buildable.Builder { + + public Builder() {} + + private Builder(FeesAndCredits instance) { + super(instance); + } + + public Builder addFeeOrCredit(BaseFee feeOrCredit) { + if (feeOrCredit instanceof Credit) { + getInstance().credits = + new ImmutableList.Builder() + .addAll(nullToEmptyImmutableCopy(getInstance().credits)) + .add((Credit) feeOrCredit) + .build(); + } else { + getInstance().fees = + new ImmutableList.Builder() + .addAll(nullToEmptyImmutableCopy(getInstance().fees)) + .add((Fee) feeOrCredit) + .build(); + } + return this; + } + + public Builder setFeesAndCredits(ImmutableList feesAndCredits) { + ImmutableList.Builder feeBuilder = new ImmutableList.Builder<>(); + ImmutableList.Builder creditBuilder = new ImmutableList.Builder<>(); + for (BaseFee feeOrCredit : feesAndCredits) { + if (feeOrCredit instanceof Credit) { + creditBuilder.add((Credit) feeOrCredit); + } else { + feeBuilder.add((Fee) feeOrCredit); + } + } + getInstance().fees = feeBuilder.build(); + getInstance().credits = creditBuilder.build(); + return this; + } + + public Builder setFeesAndCredits(BaseFee ... feesAndCredits) { + return setFeesAndCredits(ImmutableList.copyOf(feesAndCredits)); + } + + public Builder setCurrency(CurrencyUnit currency) { + getInstance().currency = currency; + return this; + } + + public Builder setFeeExtensionRequired(boolean feeExtensionRequired) { + getInstance().feeExtensionRequired = feeExtensionRequired; + return this; + } + + @Override + public FeesAndCredits build() { + checkArgumentNotNull(getInstance().currency, "Currency must be specified in FeesAndCredits"); + getInstance().fees = nullToEmptyImmutableCopy(getInstance().fees); + getInstance().credits = nullToEmptyImmutableCopy(getInstance().credits); + return getInstance(); + } + } +} diff --git a/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java b/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java index 708f562f9..83a1f3834 100644 --- a/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java +++ b/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java @@ -15,8 +15,6 @@ package google.registry.flows.custom; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.collect.Iterables.toArray; -import static java.math.BigDecimal.TEN; import com.google.common.base.Ascii; import com.google.common.base.Splitter; @@ -26,7 +24,7 @@ import com.google.common.net.InternetDomainName; import google.registry.flows.EppException; import google.registry.flows.SessionMetadata; import google.registry.flows.domain.DomainPricingLogic; -import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; +import google.registry.flows.domain.FeesAndCredits; import google.registry.model.domain.fee.BaseFee; import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Credit; @@ -34,10 +32,14 @@ import google.registry.model.domain.fee.Fee; import google.registry.model.eppinput.EppInput; import java.math.BigDecimal; import java.util.List; +import org.joda.money.CurrencyUnit; +import org.joda.money.Money; /** A class to customize {@link DomainPricingLogic} for testing. */ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { + private static final BigDecimal ONE_HUNDRED_BUCKS = Money.of(CurrencyUnit.USD, 100).getAmount(); + protected TestDomainPricingCustomLogic(EppInput eppInput, SessionMetadata sessionMetadata) { super(eppInput, sessionMetadata); } @@ -48,14 +50,16 @@ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { InternetDomainName domainName = priceParameters.domainName(); if (domainName.parent().toString().equals("flags")) { FeesAndCredits feesAndCredits = priceParameters.feesAndCredits(); + FeesAndCredits.Builder feesBuilder = + new FeesAndCredits.Builder().setCurrency(feesAndCredits.getCurrency()); ImmutableList.Builder baseFeeBuilder = new ImmutableList.Builder<>(); baseFeeBuilder.addAll(feesAndCredits.getCredits()); for (BaseFee fee : feesAndCredits.getFees()) { baseFeeBuilder.add( fee.getType() == FeeType.CREATE ? domainNameToFeeOrCredit(domainName) : fee); + feesBuilder.setFeeExtensionRequired(true); } - return new FeesAndCredits( - feesAndCredits.getCurrency(), Iterables.toArray(baseFeeBuilder.build(), BaseFee.class)); + return feesBuilder.setFeesAndCredits(baseFeeBuilder.build()).build(); } else { return priceParameters.feesAndCredits(); } @@ -69,7 +73,7 @@ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { .getFullyQualifiedDomainName() .startsWith("non-free-update")) ? addCustomFee( - priceParameters.feesAndCredits(), Fee.create(BigDecimal.valueOf(100), FeeType.UPDATE)) + priceParameters.feesAndCredits(), Fee.create(ONE_HUNDRED_BUCKS, FeeType.UPDATE)) : priceParameters.feesAndCredits(); } @@ -78,7 +82,7 @@ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { throws EppException { return (priceParameters.domainName().toString().startsWith("costly-renew")) ? addCustomFee( - priceParameters.feesAndCredits(), Fee.create(BigDecimal.valueOf(100), FeeType.RENEW)) + priceParameters.feesAndCredits(), Fee.create(ONE_HUNDRED_BUCKS, FeeType.RENEW)) : priceParameters.feesAndCredits(); } @@ -87,25 +91,24 @@ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { throws EppException { return (priceParameters.domainName().toString().startsWith("expensive")) ? addCustomFee( - priceParameters.feesAndCredits(), Fee.create(BigDecimal.valueOf(100), FeeType.TRANSFER)) + priceParameters.feesAndCredits(), Fee.create(ONE_HUNDRED_BUCKS, FeeType.TRANSFER)) : priceParameters.feesAndCredits(); } @Override public FeesAndCredits customizeUpdatePrice(UpdatePriceParameters priceParameters) { return (priceParameters.domainName().toString().startsWith("non-free-update")) - ? addCustomFee(priceParameters.feesAndCredits(), Fee.create(TEN, FeeType.UPDATE)) + ? addCustomFee( + priceParameters.feesAndCredits(), Fee.create(ONE_HUNDRED_BUCKS, FeeType.UPDATE)) : priceParameters.feesAndCredits(); } private static FeesAndCredits addCustomFee(FeesAndCredits feesAndCredits, BaseFee customFee) { - List newFeesAndCredits = - new ImmutableList.Builder() - .addAll(feesAndCredits.getFeesAndCredits()) - .add(customFee) - .build(); - return new FeesAndCredits( - feesAndCredits.getCurrency(), toArray(newFeesAndCredits, BaseFee.class)); + return feesAndCredits + .asBuilder() + .setFeeExtensionRequired(true) + .addFeeOrCredit(customFee) + .build(); } private static BaseFee domainNameToFeeOrCredit(InternetDomainName domainName) { diff --git a/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java index e6af57cc6..b239fbfd3 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java @@ -679,7 +679,7 @@ public class DomainApplicationUpdateFlowTest newDomainApplication("non-free-update.tld").asBuilder().setRepoId("1-ROID").build()); setEppInput( "domain_update_sunrise_fee.xml", - ImmutableMap.of("DOMAIN", "non-free-update.tld", "AMOUNT", "12")); + ImmutableMap.of("DOMAIN", "non-free-update.tld", "AMOUNT", "12.00")); clock.advanceOneMilli(); thrown.expect(FeesMismatchException.class); runFlow(); diff --git a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java index 4a442cc3e..accb712ca 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -84,13 +84,30 @@ import org.junit.Test; public class DomainTransferRequestFlowTest extends DomainTransferFlowTestCase { + private static final ImmutableMap BASE_FEE_MAP = + new ImmutableMap.Builder() + .put("DOMAIN", "example.tld") + .put("YEARS", "1") + .put("AMOUNT", "11.00") + .build(); private static final ImmutableMap FEE_06_MAP = - ImmutableMap.of("FEE_VERSION", "0.6", "FEE_NS", "fee"); + new ImmutableMap.Builder() + .putAll(BASE_FEE_MAP) + .put("FEE_VERSION", "0.6") + .put("FEE_NS", "fee") + .build(); private static final ImmutableMap FEE_11_MAP = - ImmutableMap.of("FEE_VERSION", "0.11", "FEE_NS", "fee11"); + new ImmutableMap.Builder() + .putAll(BASE_FEE_MAP) + .put("FEE_VERSION", "0.11") + .put("FEE_NS", "fee11") + .build(); private static final ImmutableMap FEE_12_MAP = - ImmutableMap.of("FEE_VERSION", "0.12", "FEE_NS", "fee12"); - + new ImmutableMap.Builder() + .putAll(BASE_FEE_MAP) + .put("FEE_VERSION", "0.12") + .put("FEE_NS", "fee12") + .build(); @Before public void setUp() throws Exception { @@ -532,13 +549,16 @@ public class DomainTransferRequestFlowTest setupDomain("expensive-domain", "foo"); clock.advanceOneMilli(); doSuccessfulTest( - "domain_transfer_request_wildcard.xml", - "domain_transfer_request_response_wildcard.xml", + "domain_transfer_request_fee.xml", + "domain_transfer_request_response_fees.xml", domain.getRegistrationExpirationTime().plusYears(3), new ImmutableMap.Builder() .put("DOMAIN", "expensive-domain.foo") .put("YEARS", "3") + .put("AMOUNT", "133.00") .put("EXDATE", "2004-09-08T22:00:00.0Z") + .put("FEE_VERSION", "0.6") + .put("FEE_NS", "fee") .build(), Optional.of(Money.of(USD, 133))); } diff --git a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java index 65e0b6669..96794aea5 100644 --- a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java @@ -49,7 +49,7 @@ import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException; import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleException; import google.registry.flows.domain.DomainFlowUtils.EmptySecDnsUpdateException; import google.registry.flows.domain.DomainFlowUtils.FeesMismatchException; -import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeUpdateException; +import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeOperationException; import google.registry.flows.domain.DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException; import google.registry.flows.domain.DomainFlowUtils.LinkedResourcesDoNotExistException; import google.registry.flows.domain.DomainFlowUtils.MaxSigLifeChangeNotSupportedException; @@ -1196,7 +1196,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase - example.tld - 1 + %DOMAIN% + %YEARS% 2fooBAR @@ -13,7 +13,7 @@ USD - 11.00 + %AMOUNT% ABC-12345 diff --git a/javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_wildcard.xml b/javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_fees.xml similarity index 73% rename from javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_wildcard.xml rename to javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_fees.xml index bc67c5ee6..dcce1ed03 100644 --- a/javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_wildcard.xml +++ b/javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_fees.xml @@ -15,6 +15,13 @@ %EXDATE% + + + USD + 33.00 + 100.00 + + ABC-12345 server-trid