diff --git a/java/com/google/domain/registry/flows/domain/DomainTransferRequestFlow.java b/java/com/google/domain/registry/flows/domain/DomainTransferRequestFlow.java index 81869e5c3..632f2cc46 100644 --- a/java/com/google/domain/registry/flows/domain/DomainTransferRequestFlow.java +++ b/java/com/google/domain/registry/flows/domain/DomainTransferRequestFlow.java @@ -201,7 +201,7 @@ public class DomainTransferRequestFlow .setClientId(existingResource.getCurrentSponsorClientId()) .setEventTime(automaticTransferTime) .setBillingTime(expirationTime.plus(registry.getAutoRenewGracePeriodLength())) - .setEventRef(existingResource.getAutorenewBillingEvent()) + .setRecurringEventRef(existingResource.getAutorenewBillingEvent()) .setParent(historyEntry) .build(); ofy().save().entity(autorenewCancellation); diff --git a/java/com/google/domain/registry/model/billing/BillingEvent.java b/java/com/google/domain/registry/model/billing/BillingEvent.java index b1aef68ca..1a18e56cf 100644 --- a/java/com/google/domain/registry/model/billing/BillingEvent.java +++ b/java/com/google/domain/registry/model/billing/BillingEvent.java @@ -376,24 +376,30 @@ public abstract class BillingEvent extends ImmutableObject GracePeriodStatus.TRANSFER, Reason.TRANSFER); /** - * Creates a cancellation billing event for the provided grace period parented on the provided - * history entry (and with the same event time) that will cancel out the - * grace-period-originating billing event on the supplied targetId. + * Creates a cancellation billing event (parented on the provided history entry, and with the + * history entry's event time) that will cancel out the provided grace period's billing event, + * using the supplied targetId and deriving other metadata (clientId, billing time, and the + * cancellation reason) from the grace period. */ public static BillingEvent.Cancellation forGracePeriod( GracePeriod gracePeriod, HistoryEntry historyEntry, String targetId) { - return new BillingEvent.Cancellation.Builder() + checkArgument(gracePeriod.hasBillingEvent(), + "Cannot create cancellation for grace period without billing event"); + BillingEvent.Cancellation.Builder builder = new BillingEvent.Cancellation.Builder() .setReason(checkNotNull(GRACE_PERIOD_TO_REASON.get(gracePeriod.getType()))) .setTargetId(targetId) .setClientId(gracePeriod.getClientId()) .setEventTime(historyEntry.getModificationTime()) // The charge being cancelled will take place at the grace period's expiration time. .setBillingTime(gracePeriod.getExpirationTime()) - .setEventRef(firstNonNull( - gracePeriod.getOneTimeBillingEvent(), - gracePeriod.getRecurringBillingEvent())) - .setParent(historyEntry) - .build(); + .setParent(historyEntry); + // Set the grace period's billing event using the appropriate Cancellation builder method. + if (gracePeriod.getOneTimeBillingEvent() != null) { + builder.setOneTimeEventRef(gracePeriod.getOneTimeBillingEvent()); + } else if (gracePeriod.getRecurringBillingEvent() != null) { + builder.setRecurringEventRef(gracePeriod.getRecurringBillingEvent()); + } + return builder.build(); } @Override @@ -404,8 +410,6 @@ public abstract class BillingEvent extends ImmutableObject /** A builder for {@link Cancellation} since it is immutable. */ public static class Builder extends BillingEvent.Builder { - private Ref refTemp; - public Builder() {} private Builder(Cancellation instance) { @@ -417,28 +421,23 @@ public abstract class BillingEvent extends ImmutableObject return this; } - public Builder setEventRef(Ref eventRef) { - refTemp = eventRef; + public Builder setOneTimeEventRef(Ref eventRef) { + getInstance().refOneTime = eventRef; + return this; + } + + public Builder setRecurringEventRef(Ref eventRef) { + getInstance().refRecurring = eventRef; return this; } @Override - @SuppressWarnings("unchecked") public Cancellation build() { Cancellation instance = getInstance(); checkNotNull(instance.billingTime); checkNotNull(instance.reason); - // If refTemp is set, use it to populate the correct ref. - if (refTemp != null) { - if (Reason.AUTO_RENEW.equals(instance.reason)) { - instance.refRecurring = (Ref) refTemp; - } else { - instance.refOneTime = (Ref) refTemp; - } - } - // Ensure that even if refTemp was not set, the builder has exactly one of the two refs - // set to a non-null value (using != as an XOR for booleans). - checkState((instance.refOneTime == null) != (instance.refRecurring == null)); + checkState((instance.refOneTime == null) != (instance.refRecurring == null), + "Cancellations must have exactly one billing event ref set"); return super.build(); } } diff --git a/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java index e15ee2292..3e09e2241 100644 --- a/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java +++ b/javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java @@ -169,7 +169,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase builder) { + private > E commonInit(B builder) { return builder .setClientId("a registrar") - .setFlags(ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT)) - .setEventTime(START_OF_TIME) - .setTargetId("foo") - .setParent(historyEntry) + .setTargetId("foo.tld") .build(); } @@ -87,7 +123,8 @@ public class BillingEventTest extends EntityTestCase { public void testPersistence() throws Exception { assertThat(ofy().load().entity(oneTime).now()).isEqualTo(oneTime); assertThat(ofy().load().entity(recurring).now()).isEqualTo(recurring); - assertThat(ofy().load().entity(cancellation).now()).isEqualTo(cancellation); + assertThat(ofy().load().entity(cancellationOneTime).now()).isEqualTo(cancellationOneTime); + assertThat(ofy().load().entity(cancellationRecurring).now()).isEqualTo(cancellationRecurring); assertThat(ofy().load().entity(modification).now()).isEqualTo(modification); } @@ -100,16 +137,16 @@ public class BillingEventTest extends EntityTestCase { assertThat(ofy().load().type(BillingEvent.Recurring.class).ancestor(domain).list()) .containsExactly(recurring); assertThat(ofy().load().type(BillingEvent.Cancellation.class).ancestor(domain).list()) - .containsExactly(cancellation); + .containsExactly(cancellationOneTime, cancellationRecurring); assertThat(ofy().load().type(BillingEvent.Modification.class).ancestor(domain).list()) .containsExactly(modification); assertThat(ofy().load().type(BillingEvent.OneTime.class).ancestor(historyEntry).list()) .containsExactly(oneTime); assertThat(ofy().load().type(BillingEvent.Recurring.class).ancestor(historyEntry).list()) .containsExactly(recurring); - assertThat(ofy().load().type(BillingEvent.Cancellation.class).ancestor(historyEntry).list()) - .containsExactly(cancellation); - assertThat(ofy().load().type(BillingEvent.Modification.class).ancestor(historyEntry).list()) + assertThat(ofy().load().type(BillingEvent.Cancellation.class).ancestor(historyEntry2).list()) + .containsExactly(cancellationOneTime, cancellationRecurring); + assertThat(ofy().load().type(BillingEvent.Modification.class).ancestor(historyEntry2).list()) .containsExactly(modification); } @@ -118,10 +155,63 @@ public class BillingEventTest extends EntityTestCase { verifyIndexing(oneTime, "clientId", "eventTime", "billingTime"); verifyIndexing( recurring, "clientId", "eventTime", "recurrenceEndTime", "recurrenceTimeOfYear.timeString"); - verifyIndexing(cancellation, "clientId", "eventTime", "billingTime"); + verifyIndexing(cancellationOneTime, "clientId", "eventTime", "billingTime"); verifyIndexing(modification, "clientId", "eventTime"); } + @Test + public void testSuccess_cancellation_forGracePeriod_withOneTime() { + BillingEvent.Cancellation newCancellation = BillingEvent.Cancellation.forGracePeriod( + GracePeriod.forBillingEvent(GracePeriodStatus.ADD, oneTime), + historyEntry2, + "foo.tld"); + // Set ID to be the same to ignore for the purposes of comparison. + newCancellation = newCancellation.asBuilder().setId(cancellationOneTime.getId()).build(); + assertThat(newCancellation).isEqualTo(cancellationOneTime); + } + + @Test + public void testSuccess_cancellation_forGracePeriod_withRecurring() { + BillingEvent.Cancellation newCancellation = BillingEvent.Cancellation.forGracePeriod( + GracePeriod.createForRecurring( + GracePeriodStatus.AUTO_RENEW, + now.plusYears(1).plusDays(45), + "a registrar", + Ref.create(recurring)), + historyEntry2, + "foo.tld"); + // Set ID to be the same to ignore for the purposes of comparison. + newCancellation = newCancellation.asBuilder().setId(cancellationRecurring.getId()).build(); + assertThat(newCancellation).isEqualTo(cancellationRecurring); + } + + @Test + public void testFailure_cancellation_forGracePeriodWithoutBillingEvent() { + thrown.expect(IllegalArgumentException.class, "grace period without billing event"); + BillingEvent.Cancellation.forGracePeriod( + GracePeriod.createWithoutBillingEvent( + GracePeriodStatus.REDEMPTION, + now.plusDays(1), + "a registrar"), + historyEntry, + "foo.tld"); + } + + @Test + public void testFailure_cancellationWithNoBillingEvent() { + thrown.expect(IllegalStateException.class, "exactly one billing event"); + cancellationOneTime.asBuilder().setOneTimeEventRef(null).setRecurringEventRef(null).build(); + } + + @Test + public void testFailure_cancellationWithBothBillingEvents() { + thrown.expect(IllegalStateException.class, "exactly one billing event"); + cancellationOneTime.asBuilder() + .setOneTimeEventRef(Ref.create(oneTime)) + .setRecurringEventRef(Ref.create(recurring)) + .build(); + } + @Test public void testDeadCodeThatDeletedScrapCommandsReference() throws Exception { assertThat(recurring.getParentKey()).isEqualTo(Key.create(historyEntry));