diff --git a/java/com/google/domain/registry/flows/domain/DomainDeleteFlow.java b/java/com/google/domain/registry/flows/domain/DomainDeleteFlow.java index 2723d579c..2d1679aa5 100644 --- a/java/com/google/domain/registry/flows/domain/DomainDeleteFlow.java +++ b/java/com/google/domain/registry/flows/domain/DomainDeleteFlow.java @@ -110,11 +110,10 @@ public class DomainDeleteFlow extends ResourceSyncDeleteFlow creditsBuilder = new ImmutableList.Builder<>(); for (GracePeriod gracePeriod : existingResource.getGracePeriods()) { // No cancellation is written if the grace period was not for a billable event. - if (gracePeriod.getBillingEvent() != null) { + if (gracePeriod.hasBillingEvent()) { ofy().save().entity( BillingEvent.Cancellation.forGracePeriod(gracePeriod, historyEntry, targetId)); diff --git a/java/com/google/domain/registry/flows/domain/DomainUpdateFlow.java b/java/com/google/domain/registry/flows/domain/DomainUpdateFlow.java index 187b35e0d..27d963f17 100644 --- a/java/com/google/domain/registry/flows/domain/DomainUpdateFlow.java +++ b/java/com/google/domain/registry/flows/domain/DomainUpdateFlow.java @@ -104,7 +104,7 @@ public class DomainUpdateFlow extends BaseDomainUpdateFlow getBillingEvent() { - return Optional.>fromNullable(billingEventOneTime) - .or(Optional.>fromNullable(billingEventRecurring)) - .orNull(); + /** Returns true if this GracePeriod has an associated BillingEvent; i.e. if it's refundable. */ + public boolean hasBillingEvent() { + return billingEventOneTime != null || billingEventRecurring != null; } /** @@ -106,26 +100,53 @@ public class GracePeriod extends ImmutableObject { return billingEventRecurring; } - /** - * Constructs a GracePeriod with some interpretation of the parameters. In particular, selects - * the field to store the billing event ref in based on the specified grace period status type. + private static GracePeriod createInternal( + GracePeriodStatus type, + DateTime expirationTime, + String clientId, + @Nullable Ref billingEventOneTime, + @Nullable Ref billingEventRecurring) { + checkArgument((billingEventOneTime == null) || (billingEventRecurring == null), + "A grace period can have at most one billing event"); + checkArgument((billingEventRecurring != null) == (GracePeriodStatus.AUTO_RENEW.equals(type)), + "Recurring billing events must be present on (and only on) autorenew grace periods"); + GracePeriod instance = new GracePeriod(); + instance.type = checkArgumentNotNull(type); + instance.expirationTime = checkArgumentNotNull(expirationTime); + instance.clientId = checkArgumentNotNull(clientId); + instance.billingEventOneTime = billingEventOneTime; + instance.billingEventRecurring = billingEventRecurring; + return instance; + } + + /** Create a GracePeriod for an (optional) OneTime billing event. + * + *

Normal callers should always use {@link #forBillingEvent} instead, assuming they do not + * need to avoid loading the BillingEvent from datastore. This method should typically be + * called only from test code to explicitly construct GracePeriods. */ - @SuppressWarnings("unchecked") public static GracePeriod create( GracePeriodStatus type, DateTime expirationTime, String clientId, - @Nullable Ref billingEvent) { - GracePeriod instance = new GracePeriod(); - instance.type = checkNotNull(type); - instance.expirationTime = checkNotNull(expirationTime); - instance.clientId = checkNotNull(clientId); - if (GracePeriodStatus.AUTO_RENEW.equals(instance.type)) { - instance.billingEventRecurring = (Ref) billingEvent; - } else { - instance.billingEventOneTime = (Ref) billingEvent; - } - return instance; + @Nullable Ref billingEventOneTime) { + return createInternal(type, expirationTime, clientId, billingEventOneTime, null); + } + + /** Create a GracePeriod for a Recurring billing event. */ + public static GracePeriod createForRecurring( + GracePeriodStatus type, + DateTime expirationTime, + String clientId, + Ref billingEventRecurring) { + checkArgumentNotNull(billingEventRecurring); + return createInternal(type, expirationTime, clientId, null, billingEventRecurring); + } + + /** Create a GracePeriod with no billing event. */ + public static GracePeriod createWithoutBillingEvent( + GracePeriodStatus type, DateTime expirationTime, String clientId) { + return createInternal(type, expirationTime, clientId, null, null); } /** Constructs a GracePeriod of the given type from the provided one-time BillingEvent. */ diff --git a/javatests/com/google/domain/registry/flows/FlowTestCase.java b/javatests/com/google/domain/registry/flows/FlowTestCase.java index 0c5c686fb..dc3ecc3ce 100644 --- a/javatests/com/google/domain/registry/flows/FlowTestCase.java +++ b/javatests/com/google/domain/registry/flows/FlowTestCase.java @@ -227,7 +227,13 @@ public abstract class FlowTestCase { new Function() { @Override public BillingEvent apply(GracePeriod gracePeriod) { - return gracePeriod.getBillingEvent().get(); + assertThat(gracePeriod.hasBillingEvent()) + .named("Billing event is present for grace period: " + gracePeriod) + .isTrue(); + return firstNonNull( + gracePeriod.getOneTimeBillingEvent(), + gracePeriod.getRecurringBillingEvent()) + .get(); }}; assertThat(canonicalizeGracePeriods(Maps.toMap(actual, gracePeriodExpander))) .isEqualTo(canonicalizeGracePeriods(expected)); diff --git a/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java index 24de6ecdc..e15ee2292 100644 --- a/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java +++ b/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java @@ -143,7 +143,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase