From 74d64f502ed2bc3c073494125cf806d185697227 Mon Sep 17 00:00:00 2001 From: jianglai Date: Fri, 9 Dec 2016 14:30:03 -0800 Subject: [PATCH] Modify create price custom logic to return FeesAndCredits Previously DomainPricingCustomLogic#customizeCreatePrice takes in the create price itself and modifies it. Change it to take in the entire FeesAndCredits (previously named EppCommandOperations) which may contain other fees related to domain creation, such as EAP fees, and returns FeesAndCredits that may either change the fees or add new type of fees. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=141600614 --- .../custom/DomainPricingCustomLogic.java | 10 +-- .../flows/domain/DomainAllocateFlow.java | 7 +- .../domain/DomainApplicationCreateFlow.java | 14 ++-- .../flows/domain/DomainCreateFlow.java | 49 +++++++------ .../flows/domain/DomainFlowUtils.java | 14 ++-- .../flows/domain/DomainPricingLogic.java | 71 ++++++++----------- .../custom/TestDomainPricingCustomLogic.java | 16 ++++- 7 files changed, 90 insertions(+), 91 deletions(-) diff --git a/java/google/registry/flows/custom/DomainPricingCustomLogic.java b/java/google/registry/flows/custom/DomainPricingCustomLogic.java index 865e3f1e7..35599c7d4 100644 --- a/java/google/registry/flows/custom/DomainPricingCustomLogic.java +++ b/java/google/registry/flows/custom/DomainPricingCustomLogic.java @@ -19,8 +19,8 @@ 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.model.ImmutableObject; -import google.registry.model.domain.fee.BaseFee; import google.registry.model.eppinput.EppInput; import google.registry.model.registry.Registry; import org.joda.time.DateTime; @@ -38,16 +38,16 @@ public class DomainPricingCustomLogic extends BaseFlowCustomLogic { /** A hook that customizes create price. */ @SuppressWarnings("unused") - public BaseFee customizeCreatePrice(CreatePriceParameters createPriceParameters) + public FeesAndCredits customizeCreatePrice(CreatePriceParameters createPriceParameters) throws EppException { - return createPriceParameters.createFee(); + return createPriceParameters.feesAndCredits(); } /** A class to encapsulate parameters for a call to {@link #customizeCreatePrice} . */ @AutoValue public abstract static class CreatePriceParameters extends ImmutableObject { - public abstract BaseFee createFee(); + public abstract FeesAndCredits feesAndCredits(); public abstract Registry registry(); @@ -65,7 +65,7 @@ public class DomainPricingCustomLogic extends BaseFlowCustomLogic { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setCreateFee(BaseFee createFee); + public abstract Builder setFeesAndCredits(FeesAndCredits feesAndCredits); public abstract Builder setRegistry(Registry registry); diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 7b78c677e..831d61d15 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -52,7 +52,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.DomainPricingLogic.EppCommandOperations; +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; @@ -390,13 +390,12 @@ public class DomainAllocateFlow implements TransactionalFlow { private ImmutableList createResponseExtensions( DateTime now, Registry registry, int years) throws EppException { - EppCommandOperations commandOperations = - pricingLogic.getCreatePrice(registry, targetId, now, years); + FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice(registry, targetId, now, years); FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); return (feeCreate == null) ? ImmutableList.of() - : ImmutableList.of(createFeeCreateResponse(feeCreate, commandOperations)); + : ImmutableList.of(createFeeCreateResponse(feeCreate, feesAndCredits)); } /** Domain application with specific ROID does not exist. */ diff --git a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java index ee315b2ff..94fb1c4d0 100644 --- a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java @@ -64,7 +64,7 @@ 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.EppCommandOperations; +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; @@ -201,7 +201,7 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { String tld = domainName.parent().toString(); checkAllowedAccessToTld(clientId, tld); Registry registry = Registry.get(tld); - EppCommandOperations commandOperations = + FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice(registry, targetId, now, command.getPeriod().getValue()); // Superusers can create reserved domains, force creations on domains that require a claims // notice without specifying a claims key, and override blocks on registering premium domains. @@ -224,7 +224,7 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { } FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); - validateFeeChallenge(targetId, tld, now, feeCreate, commandOperations.getTotalCost()); + validateFeeChallenge(targetId, tld, now, feeCreate, feesAndCredits.getTotalCost()); SecDnsCreateExtension secDnsCreate = validateSecDnsExtension(eppInput.getSingleExtension(SecDnsCreateExtension.class)); customLogic.afterValidation( @@ -292,7 +292,7 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { newApplication.getForeignKey(), launchCreate.getPhase(), feeCreate, - commandOperations)) + feesAndCredits)) .build()); return responseBuilder .setResData(responseData.resData()) @@ -366,11 +366,11 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { .build(); } - private ImmutableList createResponseExtensions( + private static ImmutableList createResponseExtensions( String applicationId, LaunchPhase launchPhase, FeeTransformCommandExtension feeCreate, - EppCommandOperations commandOperations) { + FeesAndCredits feesAndCredits) { ImmutableList.Builder responseExtensionsBuilder = new ImmutableList.Builder<>(); responseExtensionsBuilder.add(new LaunchCreateResponseExtension.Builder() @@ -378,7 +378,7 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { .setApplicationId(applicationId) .build()); if (feeCreate != null) { - responseExtensionsBuilder.add(createFeeCreateResponse(feeCreate, commandOperations)); + responseExtensionsBuilder.add(createFeeCreateResponse(feeCreate, feesAndCredits)); } return responseExtensionsBuilder.build(); } diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index e244acf8a..baa17140b 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -61,7 +61,7 @@ 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.EppCommandOperations; +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; @@ -211,10 +211,9 @@ public class DomainCreateFlow implements TransactionalFlow { FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); - EppCommandOperations commandOperations = - pricingLogic.getCreatePrice(registry, targetId, now, years); + FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice(registry, targetId, now, years); validateFeeChallenge( - targetId, registry.getTldStr(), now, feeCreate, commandOperations.getTotalCost()); + targetId, registry.getTldStr(), now, feeCreate, feesAndCredits.getTotalCost()); // 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. @@ -242,8 +241,9 @@ public class DomainCreateFlow implements TransactionalFlow { DateTime registrationExpirationTime = leapSafeAddYears(now, years); HistoryEntry historyEntry = buildHistory(repoId, period, now); // Bill for the create. - BillingEvent.OneTime createBillingEvent = createOneTimeBillingEvent( - registry, isAnchorTenant, years, commandOperations, historyEntry, now); + BillingEvent.OneTime createBillingEvent = + createOneTimeBillingEvent( + registry, isAnchorTenant, years, feesAndCredits, historyEntry, now); // Create a new autorenew billing event and poll message starting at the expiration time. BillingEvent.Recurring autorenewBillingEvent = createAutorenewBillingEvent(historyEntry, registrationExpirationTime); @@ -256,8 +256,8 @@ public class DomainCreateFlow implements TransactionalFlow { autorenewBillingEvent, autorenewPollMessage); // Bill for EAP cost, if any. - if (!commandOperations.getEapCost().isZero()) { - entitiesToSave.add(createEapBillingEvent(commandOperations, createBillingEvent)); + if (!feesAndCredits.getEapCost().isZero()) { + entitiesToSave.add(createEapBillingEvent(feesAndCredits, createBillingEvent)); } DomainResource newDomain = new DomainResource.Builder() .setCreationClientId(clientId) @@ -308,7 +308,7 @@ public class DomainCreateFlow implements TransactionalFlow { customLogic.beforeResponse( BeforeResponseParameters.newBuilder() .setResData(DomainCreateData.create(targetId, now, registrationExpirationTime)) - .setResponseExtensions(createResponseExtensions(feeCreate, commandOperations)) + .setResponseExtensions(createResponseExtensions(feeCreate, feesAndCredits)) .build()); return responseBuilder .setResData(responseData.resData()) @@ -352,7 +352,7 @@ public class DomainCreateFlow implements TransactionalFlow { Registry registry, boolean isAnchorTenant, int years, - EppCommandOperations commandOperations, + FeesAndCredits feesAndCredits, HistoryEntry historyEntry, DateTime now) { return new BillingEvent.OneTime.Builder() @@ -360,14 +360,17 @@ public class DomainCreateFlow implements TransactionalFlow { .setTargetId(targetId) .setClientId(clientId) .setPeriodYears(years) - .setCost(commandOperations.getCreateCost()) + .setCost(feesAndCredits.getCreateCost()) .setEventTime(now) - .setBillingTime(now.plus(isAnchorTenant - ? registry.getAnchorTenantAddGracePeriodLength() - : registry.getAddGracePeriodLength())) - .setFlags(isAnchorTenant - ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) - : ImmutableSet.of()) + .setBillingTime( + now.plus( + isAnchorTenant + ? registry.getAnchorTenantAddGracePeriodLength() + : registry.getAddGracePeriodLength())) + .setFlags( + isAnchorTenant + ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) + : ImmutableSet.of()) .setParent(historyEntry) .build(); } @@ -396,13 +399,13 @@ public class DomainCreateFlow implements TransactionalFlow { .build(); } - private OneTime createEapBillingEvent(EppCommandOperations commandOperations, - BillingEvent.OneTime createBillingEvent) { + private static OneTime createEapBillingEvent( + FeesAndCredits feesAndCredits, BillingEvent.OneTime createBillingEvent) { return new BillingEvent.OneTime.Builder() .setReason(Reason.FEE_EARLY_ACCESS) .setTargetId(createBillingEvent.getTargetId()) .setClientId(createBillingEvent.getClientId()) - .setCost(commandOperations.getEapCost()) + .setCost(feesAndCredits.getEapCost()) .setEventTime(createBillingEvent.getEventTime()) .setBillingTime(createBillingEvent.getBillingTime()) .setFlags(createBillingEvent.getFlags()) @@ -424,11 +427,11 @@ public class DomainCreateFlow implements TransactionalFlow { } } - private ImmutableList createResponseExtensions( - FeeCreateCommandExtension feeCreate, EppCommandOperations commandOperations) { + private static ImmutableList createResponseExtensions( + FeeCreateCommandExtension feeCreate, FeesAndCredits feesAndCredits) { return (feeCreate == null) ? ImmutableList.of() - : ImmutableList.of(createFeeCreateResponse(feeCreate, commandOperations)); + : ImmutableList.of(createFeeCreateResponse(feeCreate, feesAndCredits)); } /** There is an open application for this domain. */ diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index a422a87e1..75064f8fe 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -56,7 +56,7 @@ 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.EppCommandOperations; +import google.registry.flows.domain.DomainPricingLogic.FeesAndCredits; import google.registry.flows.exceptions.ResourceAlreadyExistsException; import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException; import google.registry.model.EppResource; @@ -994,12 +994,12 @@ public class DomainFlowUtils { /** Create a response extension listign the fees on a domain or application create. */ static FeeTransformResponseExtension createFeeCreateResponse( - FeeTransformCommandExtension feeCreate, - EppCommandOperations commandOperations) { - return feeCreate.createResponseBuilder() - .setCurrency(commandOperations.getCurrency()) - .setFees(commandOperations.getFees()) - .setCredits(commandOperations.getCredits()) + FeeTransformCommandExtension feeCreate, FeesAndCredits feesAndCredits) { + return feeCreate + .createResponseBuilder() + .setCurrency(feesAndCredits.getCurrency()) + .setFees(feesAndCredits.getFees()) + .setCredits(feesAndCredits.getCredits()) .build(); } diff --git a/java/google/registry/flows/domain/DomainPricingLogic.java b/java/google/registry/flows/domain/DomainPricingLogic.java index eb7555060..b087767a5 100644 --- a/java/google/registry/flows/domain/DomainPricingLogic.java +++ b/java/google/registry/flows/domain/DomainPricingLogic.java @@ -14,7 +14,6 @@ package google.registry.flows.domain; -import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.pricing.PricingEngineProxy.getDomainCreateCost; @@ -62,31 +61,17 @@ public final class DomainPricingLogic { DomainPricingLogic() {} /** A collection of fees and credits for a specific EPP transform. */ - public static final class EppCommandOperations extends ImmutableObject { + public static final class FeesAndCredits extends ImmutableObject { private final CurrencyUnit currency; private final ImmutableList fees; private final ImmutableList credits; - /** Constructs an EppCommandOperations object using separate lists of fees and credits. */ - EppCommandOperations( - CurrencyUnit currency, ImmutableList fees, ImmutableList credits) { - this.currency = - checkArgumentNotNull(currency, "Currency may not be null in EppCommandOperations."); - checkArgument(!fees.isEmpty(), "You must specify one or more fees."); - this.fees = checkArgumentNotNull(fees, "Fees may not be null in EppCommandOperations."); - this.credits = - checkArgumentNotNull(credits, "Credits may not be null in EppCommandOperations."); - } - - /** - * Constructs an EppCommandOperations object. The arguments are sorted into fees and credits. - */ - EppCommandOperations(CurrencyUnit currency, BaseFee... feesAndCredits) { - this.currency = - checkArgumentNotNull(currency, "Currency may not be null in EppCommandOperations."); + /** Constructs an FeesAndCredits object. The arguments are sorted into 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 : feesAndCredits) { + for (BaseFee feeOrCredit : baseFees) { if (feeOrCredit instanceof Credit) { creditBuilder.add((Credit) feeOrCredit); } else { @@ -147,7 +132,7 @@ public final class DomainPricingLogic { } /** Returns a new create price for the Pricer. */ - public EppCommandOperations getCreatePrice( + public FeesAndCredits getCreatePrice( Registry registry, String domainName, DateTime date, int years) throws EppException { CurrencyUnit currency = registry.getCurrency(); @@ -155,24 +140,26 @@ public final class DomainPricingLogic { BaseFee createFeeOrCredit = Fee.create(getDomainCreateCost(domainName, date, years).getAmount(), FeeType.CREATE); - // Apply custom logic to the create fee, if any. - createFeeOrCredit = - customLogic.customizeCreatePrice( - CreatePriceParameters.newBuilder() - .setCreateFee(createFeeOrCredit) - .setRegistry(registry) - .setDomainName(InternetDomainName.from(domainName)) - .setAsOfDate(date) - .setYears(years) - .build()); + FeesAndCredits feesAndCredits; // Create fees for the cost and the EAP fee, if any. Fee eapFee = registry.getEapFeeFor(date); if (!eapFee.hasZeroCost()) { - return new EppCommandOperations(currency, createFeeOrCredit, eapFee); + feesAndCredits = new FeesAndCredits(currency, createFeeOrCredit, eapFee); } else { - return new EppCommandOperations(currency, createFeeOrCredit); + feesAndCredits = new FeesAndCredits(currency, createFeeOrCredit); } + + // Apply custom logic to the create fee, if any. + return customLogic.customizeCreatePrice( + CreatePriceParameters.newBuilder() + .setFeesAndCredits(feesAndCredits) + .setRegistry(registry) + .setDomainName(InternetDomainName.from(domainName)) + .setAsOfDate(date) + .setYears(years) + .build()); + } // TODO: (b/33000134) clean up the rest of the pricing calls. @@ -204,7 +191,7 @@ public final class DomainPricingLogic { } /** Returns a new renew price for the pricer. */ - public static EppCommandOperations getRenewPrice( + public static FeesAndCredits getRenewPrice( Registry registry, String domainName, String clientId, @@ -212,23 +199,23 @@ public final class DomainPricingLogic { int years, EppInput eppInput) throws EppException { - return new EppCommandOperations( + return new FeesAndCredits( registry.getCurrency(), getRenewFeeOrCredit(registry, domainName, clientId, date, years, eppInput)); } /** Returns a new restore price for the pricer. */ - public static EppCommandOperations getRestorePrice( + public static FeesAndCredits getRestorePrice( Registry registry, String domainName, String clientId, DateTime date, EppInput eppInput) throws EppException { - return new EppCommandOperations( + return new FeesAndCredits( registry.getCurrency(), getRenewFeeOrCredit(registry, domainName, clientId, date, 1, eppInput), Fee.create(registry.getStandardRestoreCost().getAmount(), FeeType.RESTORE)); } /** Returns a new transfer price for the pricer. */ - public static EppCommandOperations getTransferPrice( + public static FeesAndCredits getTransferPrice( Registry registry, String domainName, String clientId, @@ -241,7 +228,7 @@ public final class DomainPricingLogic { } /** Returns a new update price for the pricer. */ - public static EppCommandOperations getUpdatePrice( + public static FeesAndCredits getUpdatePrice( Registry registry, String domainName, String clientId, DateTime date, EppInput eppInput) throws EppException { CurrencyUnit currency = registry.getCurrency(); @@ -261,11 +248,11 @@ public final class DomainPricingLogic { feeOrCredit = Fee.create(Money.zero(registry.getCurrency()).getAmount(), FeeType.UPDATE); } - return new EppCommandOperations(currency, feeOrCredit); + return new FeesAndCredits(currency, feeOrCredit); } /** Returns a new domain application update price for the pricer. */ - public static EppCommandOperations getApplicationUpdatePrice( + public static FeesAndCredits getApplicationUpdatePrice( Registry registry, DomainApplication application, String clientId, @@ -287,7 +274,7 @@ public final class DomainPricingLogic { feeOrCredit = Fee.create(Money.zero(registry.getCurrency()).getAmount(), FeeType.UPDATE); } - return new EppCommandOperations(currency, feeOrCredit); + return new FeesAndCredits(currency, feeOrCredit); } /** Returns the fee class for a given domain and date. */ diff --git a/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java b/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java index e6bd1357b..4d6994702 100644 --- a/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java +++ b/javatests/google/registry/flows/custom/TestDomainPricingCustomLogic.java @@ -18,11 +18,13 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.Ascii; import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; 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.model.domain.fee.BaseFee; import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Credit; @@ -61,13 +63,21 @@ public class TestDomainPricingCustomLogic extends DomainPricingCustomLogic { /** A hook that customizes create price. */ @Override - public BaseFee customizeCreatePrice(CreatePriceParameters createPriceParameters) + public FeesAndCredits customizeCreatePrice(CreatePriceParameters createPriceParameters) throws EppException { InternetDomainName domainName = createPriceParameters.domainName(); if (domainName.parent().toString().equals("flags")) { - return domainNameToFeeOrCredit(domainName); + FeesAndCredits feesAndCredits = createPriceParameters.feesAndCredits(); + ImmutableList.Builder baseFeeBuilder = new ImmutableList.Builder<>(); + baseFeeBuilder.addAll(feesAndCredits.getCredits()); + for (BaseFee fee : feesAndCredits.getFees()) { + baseFeeBuilder.add( + fee.getType() == FeeType.CREATE ? domainNameToFeeOrCredit(domainName) : fee); + } + return new FeesAndCredits( + feesAndCredits.getCurrency(), Iterables.toArray(baseFeeBuilder.build(), BaseFee.class)); } else { - return createPriceParameters.createFee(); + return createPriceParameters.feesAndCredits(); } } }