From 6641f105b77e73fb5d1e40f3eefe7d1da6e94f56 Mon Sep 17 00:00:00 2001 From: jianglai Date: Tue, 6 Sep 2016 10:28:56 -0700 Subject: [PATCH] Create a separate billing event when EAP is applied When EAP is involed we current have one billing event for domain create that has the create fee and EAP fee lumped together. Change it to record two separate billing events for each. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=132335349 --- .../flows/domain/DomainCreateFlow.java | 53 +++++++++++----- .../flows/domain/DomainRenewFlow.java | 18 +++--- .../domain/DomainRestoreRequestFlow.java | 19 +++--- .../domain/DomainTransferRequestFlow.java | 11 ++-- .../registry/model/billing/BillingEvent.java | 1 + .../registry/model/domain/fee/BaseFee.java | 31 +++++++++ .../google/registry/model/domain/fee/Fee.java | 10 ++- .../pricing/TldSpecificLogicProxy.java | 43 +++++++++---- .../flows/domain/DomainCreateFlowTest.java | 63 +++++++++++++------ javatests/google/registry/model/schema.txt | 1 + .../pricing/TldSpecificLogicProxyTest.java | 11 ++-- .../registry/testing/DatastoreHelper.java | 7 +++ 12 files changed, 196 insertions(+), 72 deletions(-) diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index a8b0e0cab..8ec7506c7 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -14,11 +14,13 @@ package google.registry.flows.domain; +import static com.google.common.collect.Sets.union; import static google.registry.flows.domain.DomainFlowUtils.validateFeeChallenge; import static google.registry.model.domain.fee.Fee.FEE_CREATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER; import static google.registry.model.index.DomainApplicationIndex.loadActiveApplicationsByDomainName; import static google.registry.model.ofy.ObjectifyService.ofy; + import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import google.registry.flows.EppException; @@ -146,23 +148,42 @@ public class DomainCreateFlow extends DomainCreateOrAllocateFlow { Registry registry = Registry.get(getTld()); // Bill for the create. - BillingEvent.OneTime createEvent = new BillingEvent.OneTime.Builder() - .setReason(Reason.CREATE) - .setTargetId(targetId) - .setClientId(getClientId()) - .setPeriodYears(command.getPeriod().getValue()) - // TODO(b/29089413): the EAP fee needs to be a separate billing event. - .setCost(commandOperations.getTotalCost()) - .setEventTime(now) - .setBillingTime(now.plus(isAnchorTenant() - ? registry.getAnchorTenantAddGracePeriodLength() - : registry.getAddGracePeriodLength())) - .setFlags(isAnchorTenant() - ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) - : ImmutableSet.of()) - .setParent(historyEntry) - .build(); + BillingEvent.OneTime createEvent = + new BillingEvent.OneTime.Builder() + .setReason(Reason.CREATE) + .setTargetId(targetId) + .setClientId(getClientId()) + .setPeriodYears(command.getPeriod().getValue()) + .setCost(commandOperations.getCreateCost()) + .setEventTime(now) + .setBillingTime(now.plus(isAnchorTenant() + ? registry.getAnchorTenantAddGracePeriodLength() + : registry.getAddGracePeriodLength())) + .setFlags(isAnchorTenant() + ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) + : ImmutableSet.of()) + .setParent(historyEntry) + .build(); ofy().save().entity(createEvent); + + // Bill for EAP cost, if any. + if (!commandOperations.getEapCost().isZero()) { + BillingEvent.OneTime eapEvent = + new BillingEvent.OneTime.Builder() + .setReason(createEvent.getReason()) + .setTargetId(createEvent.getTargetId()) + .setClientId(createEvent.getClientId()) + .setPeriodYears(createEvent.getPeriodYears()) + .setCost(commandOperations.getEapCost()) + .setEventTime(createEvent.getEventTime()) + .setBillingTime(createEvent.getBillingTime()) + .setFlags(union(createEvent.getFlags(), + ImmutableSet.of(BillingEvent.Flag.EAP)).immutableCopy()) + .setParent(createEvent.getParentKey()) + .build(); + ofy().save().entity(eapEvent); + } + builder.addGracePeriod(GracePeriod.forBillingEvent(GracePeriodStatus.ADD, createEvent)); if (launchCreate != null && (launchCreate.getNotice() != null || hasSignedMarks)) { builder diff --git a/java/google/registry/flows/domain/DomainRenewFlow.java b/java/google/registry/flows/domain/DomainRenewFlow.java index f748e71e7..ff9775194 100644 --- a/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/java/google/registry/flows/domain/DomainRenewFlow.java @@ -42,6 +42,7 @@ import google.registry.model.domain.DomainRenewData; import google.registry.model.domain.DomainResource; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.Period; +import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Fee; import google.registry.model.domain.fee.FeeTransformCommandExtension; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -174,13 +175,16 @@ public class DomainRenewFlow extends OwnedResourceMutateFlow getTransferResponseExtensions() { if (feeTransfer != null) { - return ImmutableList.of(feeTransfer.createResponseBuilder() - .setCurrency(renewCost.getCurrencyUnit()) - .setFees(ImmutableList.of(Fee.create(renewCost.getAmount(), "renew"))) - .build()); + return ImmutableList.of( + feeTransfer + .createResponseBuilder() + .setCurrency(renewCost.getCurrencyUnit()) + .setFees(ImmutableList.of(Fee.create(renewCost.getAmount(), FeeType.RENEW))) + .build()); } else { return null; } diff --git a/java/google/registry/model/billing/BillingEvent.java b/java/google/registry/model/billing/BillingEvent.java index bc6d430eb..245ff3943 100644 --- a/java/google/registry/model/billing/BillingEvent.java +++ b/java/google/registry/model/billing/BillingEvent.java @@ -71,6 +71,7 @@ public abstract class BillingEvent extends ImmutableObject ALLOCATION, ANCHOR_TENANT, AUTO_RENEW, + EAP, LANDRUSH, SUNRISE, /** diff --git a/java/google/registry/model/domain/fee/BaseFee.java b/java/google/registry/model/domain/fee/BaseFee.java index 4e74c3948..5b10e888e 100644 --- a/java/google/registry/model/domain/fee/BaseFee.java +++ b/java/google/registry/model/domain/fee/BaseFee.java @@ -15,6 +15,7 @@ package google.registry.model.domain.fee; import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.base.Preconditions.checkState; import google.registry.model.ImmutableObject; import google.registry.xml.PeriodAdapter; @@ -38,6 +39,24 @@ public abstract class BaseFee extends ImmutableObject { @XmlEnumValue("delayed") DELAYED } + + /** Enum for the type of the fee. */ + public enum FeeType { + CREATE("create"), + EAP("Early Access Period, fee expires: %s"), + RENEW("renew"), + RESTORE("restore"); + + private final String formatString; + + FeeType(String formatString) { + this.formatString = formatString; + } + + String renderDescription(Object... args) { + return String.format(formatString, args); + } + } @XmlAttribute String description; @@ -54,6 +73,9 @@ public abstract class BaseFee extends ImmutableObject { @XmlValue BigDecimal cost; + + @XmlTransient + FeeType type; public String getDescription() { return description; @@ -81,6 +103,15 @@ public abstract class BaseFee extends ImmutableObject { public BigDecimal getCost() { return cost; } + + public FeeType getType() { + return type; + } + + protected void generateDescription(Object... args) { + checkState(type != null); + description = type.renderDescription(args); + } public boolean hasDefaultAttributes() { return getGracePeriod().equals(Period.ZERO) diff --git a/java/google/registry/model/domain/fee/Fee.java b/java/google/registry/model/domain/fee/Fee.java index a64fcad3f..de17cea53 100644 --- a/java/google/registry/model/domain/fee/Fee.java +++ b/java/google/registry/model/domain/fee/Fee.java @@ -37,13 +37,17 @@ import google.registry.model.domain.fee12.FeeUpdateCommandExtensionV12; import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension; import java.math.BigDecimal; -/** A fee, in currency units specified elsewhere in the xml, and with an optional description. */ +/** + * A fee, in currency units specified elsewhere in the xml, with type of the fee an optional fee + * description. + */ public class Fee extends BaseFee { - public static Fee create(BigDecimal cost, String description) { + public static Fee create(BigDecimal cost, FeeType type, Object... descriptionArgs) { Fee instance = new Fee(); instance.cost = checkNotNull(cost); checkArgument(instance.cost.signum() >= 0); - instance.description = description; + instance.type = checkNotNull(type); + instance.generateDescription(descriptionArgs); return instance; } diff --git a/java/google/registry/pricing/TldSpecificLogicProxy.java b/java/google/registry/pricing/TldSpecificLogicProxy.java index f45b08fe9..21d344822 100644 --- a/java/google/registry/pricing/TldSpecificLogicProxy.java +++ b/java/google/registry/pricing/TldSpecificLogicProxy.java @@ -26,6 +26,7 @@ import com.googlecode.objectify.Key; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainCommand.Create; import google.registry.model.domain.LrpToken; +import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.EapFee; import google.registry.model.domain.fee.Fee; import google.registry.model.pricing.PremiumPricingEngine.DomainPrices; @@ -39,9 +40,6 @@ import org.joda.time.DateTime; * implementations on a per-TLD basis. */ public final class TldSpecificLogicProxy { - - private static final String EAP_DESCRIPTION_FORMAT = "Early Access Period, fee expires: %s"; - /** * A collection of fees for a specific event. */ @@ -56,9 +54,18 @@ public final class TldSpecificLogicProxy { this.fees = checkArgumentNotNull(fees, "Fees may not be null in EppCommandOperations."); } - /** - * Returns the total cost of all fees for the event. - */ + 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 for the event. */ public Money getTotalCost() { Money result = Money.zero(currency); for (Fee fee : fees) { @@ -67,6 +74,16 @@ public final class TldSpecificLogicProxy { 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 all costs for the event as a list of fees. */ @@ -94,7 +111,8 @@ public final class TldSpecificLogicProxy { ImmutableList.Builder feeBuilder = new ImmutableList.Builder<>(); // Add Create cost. - feeBuilder.add(Fee.create(prices.getCreateCost().multipliedBy(years).getAmount(), "create")); + feeBuilder.add( + Fee.create(prices.getCreateCost().multipliedBy(years).getAmount(), FeeType.CREATE)); // Add EAP Fee. EapFee eapFee = registry.getEapFeeFor(date); @@ -103,8 +121,7 @@ public final class TldSpecificLogicProxy { if (!eapFeeCost.getAmount().equals(Money.zero(currency).getAmount())) { feeBuilder.add( Fee.create( - eapFeeCost.getAmount(), - String.format(EAP_DESCRIPTION_FORMAT, eapFee.getPeriod().upperEndpoint()))); + eapFeeCost.getAmount(), FeeType.EAP, eapFee.getPeriod().upperEndpoint())); } return new EppCommandOperations(currency, feeBuilder.build()); @@ -119,7 +136,8 @@ public final class TldSpecificLogicProxy { return new EppCommandOperations( registry.getCurrency(), ImmutableList.of( - Fee.create(prices.getRenewCost().multipliedBy(years).getAmount(), "renew"))); + Fee.create( + prices.getRenewCost().multipliedBy(years).getAmount(), FeeType.RENEW))); } /** @@ -131,8 +149,9 @@ public final class TldSpecificLogicProxy { return new EppCommandOperations( registry.getCurrency(), ImmutableList.of( - Fee.create(prices.getRenewCost().multipliedBy(years).getAmount(), "renew"), - Fee.create(registry.getStandardRestoreCost().getAmount(), "restore"))); + Fee.create( + prices.getRenewCost().multipliedBy(years).getAmount(), FeeType.RENEW), + Fee.create(registry.getStandardRestoreCost().getAmount(), FeeType.RESTORE))); } /** diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 202ce5b80..00205428b 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -169,9 +169,6 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase billingEvents = ImmutableSet.of( + createBillingEvent, renewBillingEvent); + + // If EAP is applied, a billing event for EAP should be present. + if (!eapFee.isZero()) { + ImmutableSet eapFlags = + isAnchorTenant + ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT, BillingEvent.Flag.EAP) + : ImmutableSet.of(BillingEvent.Flag.EAP); + BillingEvent.OneTime eapBillingEvent = + new BillingEvent.OneTime.Builder() + .setReason(Reason.CREATE) + .setTargetId(getUniqueIdFromCommand()) + .setClientId("TheRegistrar") + .setCost(eapFee) + .setPeriodYears(2) + .setEventTime(clock.nowUtc()) + .setBillingTime(billingTime) + .setFlags(eapFlags) + .setParent(historyEntry) + .build(); + billingEvents = ImmutableSet.builder() + .addAll(billingEvents) + .add(eapBillingEvent) + .build(); + } + assertBillingEvents(billingEvents); + assertGracePeriods( domain.getGracePeriods(), ImmutableMap.of( diff --git a/javatests/google/registry/model/schema.txt b/javatests/google/registry/model/schema.txt index afd4f07bb..aaf323318 100644 --- a/javatests/google/registry/model/schema.txt +++ b/javatests/google/registry/model/schema.txt @@ -24,6 +24,7 @@ enum google.registry.model.billing.BillingEvent$Flag { ALLOCATION; ANCHOR_TENANT; AUTO_RENEW; + EAP; LANDRUSH; SUNRISE; SYNTHETIC; diff --git a/javatests/google/registry/pricing/TldSpecificLogicProxyTest.java b/javatests/google/registry/pricing/TldSpecificLogicProxyTest.java index f1a1fb388..aaa0cf8e6 100644 --- a/javatests/google/registry/pricing/TldSpecificLogicProxyTest.java +++ b/javatests/google/registry/pricing/TldSpecificLogicProxyTest.java @@ -22,6 +22,7 @@ import static org.joda.money.CurrencyUnit.USD; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSortedMap; +import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Fee; import google.registry.model.ofy.Ofy; import google.registry.model.registry.Registry; @@ -88,18 +89,18 @@ public class TldSpecificLogicProxyTest { assertThat(createPrice.getTotalCost()).isEqualTo(basicCreateCost.plus(Money.of(USD, 100))); assertThat(createPrice.getCurrency()).isEqualTo(USD); assertThat(createPrice.getFees().get(0)) - .isEqualTo(Fee.create(basicCreateCost.getAmount(), "create")); + .isEqualTo(Fee.create(basicCreateCost.getAmount(), FeeType.CREATE)); assertThat(createPrice.getFees().get(1)) .isEqualTo( Fee.create( - Money.of(USD, 100).getAmount(), - "Early Access Period, fee expires: " + clock.nowUtc().plusDays(1))); + Money.of(USD, 100).getAmount(), FeeType.EAP, clock.nowUtc().plusDays(1))); assertThat(createPrice.getFees()) .isEqualTo( ImmutableList.of( - Fee.create(basicCreateCost.getAmount(), "create"), + Fee.create(basicCreateCost.getAmount(), FeeType.CREATE), Fee.create( Money.of(USD, 100).getAmount(), - "Early Access Period, fee expires: " + clock.nowUtc().plusDays(1)))); + FeeType.EAP, + clock.nowUtc().plusDays(1)))); } } diff --git a/javatests/google/registry/testing/DatastoreHelper.java b/javatests/google/registry/testing/DatastoreHelper.java index f966d100c..382adc655 100644 --- a/javatests/google/registry/testing/DatastoreHelper.java +++ b/javatests/google/registry/testing/DatastoreHelper.java @@ -608,6 +608,13 @@ public class DatastoreHelper { FluentIterable.from(asList(expected)).transform(BILLING_EVENT_ID_STRIPPER)); } + /** Assert that the expected billing events set is exactly the one found in the fake datastore. */ + public static void assertBillingEvents(ImmutableSet expected) throws Exception { + assertThat(FluentIterable.from(getBillingEvents()).transform(BILLING_EVENT_ID_STRIPPER)) + .containsExactlyElementsIn( + FluentIterable.from(expected.asList()).transform(BILLING_EVENT_ID_STRIPPER)); + } + /** * Assert that the expected billing events are exactly the ones found for the given EppResource. */