zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-04-30 03:57:51 +02:00
Code Issues Projects Releases Packages Wiki Activity

Make GracePeriod OneTime vs Recurring logic explicit

Browse source 
This CL cleans up some old crufy logic in GracePeriod that overloaded
methods to accept either OneTime or Recurring billing events refs,
despite storing them in separate fields in the entity (since
BillingEvent is not a polymorphic superclass, just a Java-only one,
you can't store them as refs to BillingEvent).

That overloading was ultimately only there as a convenience/hack from
when we added Recurring events and didn't want to go back and change
everything.  It obfuscates what's really going on, requires extra
casting/loss of type-safety, and relies on indirect signals (e.g. the
grace period type being AUTO_RENEW) to guess what the right billing
event type is.  That latter aspect will likely no longer work in a
monthly billing world, and was brittle in any case.

A coming CL will rip the same logic out of Cancellation.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=117164286
This commit is contained in:
nickfelt 2016-03-14 13:31:48 -07:00 • committed by Justine Tunney
parent d006506531
commit 9786e6732f
10 changed files with 186 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
java/com/google/domain/registry/flows/domain/DomainDeleteFlow.java
View file

@ -110,11 +110,10 @@ public class DomainDeleteFlow extends ResourceSyncDeleteFlow<DomainResource, Bui
.setDeletionTime(deletionTime)
// Clear out all old grace periods and add REDEMPTION, which does not include a ref
// to a billing event because there isn't one for a domain delete.
.setGracePeriods(ImmutableSet.of(GracePeriod.create(
.setGracePeriods(ImmutableSet.of(GracePeriod.createWithoutBillingEvent(
GracePeriodStatus.REDEMPTION,
now.plus(registry.getRedemptionGracePeriodLength()),
getClientId(),
null)))
getClientId())))
.setDeletePollMessage(Key.create(deletePollMessage));
}
}
@ -130,7 +129,7 @@ public class DomainDeleteFlow extends ResourceSyncDeleteFlow<DomainResource, Bui
ImmutableList.Builder<Credit> 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));

2
java/com/google/domain/registry/flows/domain/DomainUpdateFlow.java
View file

@ -104,7 +104,7 @@ public class DomainUpdateFlow extends BaseDomainUpdateFlow<DomainResource, Build
// occur at the same time as the sunrush add grace period, as the event time will differ
// between them.
BillingEvent.OneTime originalAddEvent =
((BillingEvent.OneTime) sunrushAddGracePeriod.get().getBillingEvent().get());
sunrushAddGracePeriod.get().getOneTimeBillingEvent().get();
BillingEvent.OneTime billingEvent = new BillingEvent.OneTime.Builder()
.setReason(Reason.CREATE)
.setTargetId(targetId)

4
java/com/google/domain/registry/model/billing/BillingEvent.java
View file

@ -389,7 +389,9 @@ public abstract class BillingEvent extends ImmutableObject
.setEventTime(historyEntry.getModificationTime())
// The charge being cancelled will take place at the grace period's expiration time.
.setBillingTime(gracePeriod.getExpirationTime())
.setEventRef(gracePeriod.getBillingEvent())
.setEventRef(firstNonNull(
gracePeriod.getOneTimeBillingEvent(),
gracePeriod.getRecurringBillingEvent()))
.setParent(historyEntry)
.build();
}

2
java/com/google/domain/registry/model/domain/DomainResource.java
View file

@ -287,7 +287,7 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc
DateTime newExpirationTime = lastAutorenewTime.plusYears(1);
builder
.setRegistrationExpirationTime(newExpirationTime)
.addGracePeriod(GracePeriod.create(
.addGracePeriod(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
lastAutorenewTime.plus(Registry.get(getTld()).getAutoRenewGracePeriodLength()),
getCurrentSponsorClientId(),

71
java/com/google/domain/registry/model/domain/GracePeriod.java
View file

@ -14,9 +14,9 @@
package com.google.domain.registry.model.domain;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.domain.registry.util.PreconditionsUtils.checkArgumentNotNull;
import com.google.common.base.Optional;
import com.google.domain.registry.model.ImmutableObject;
import com.google.domain.registry.model.billing.BillingEvent;
import com.google.domain.registry.model.domain.rgp.GracePeriodStatus;
@ -78,15 +78,9 @@ public class GracePeriod extends ImmutableObject {
return clientId;
}
/**
* Returns the ref to the billing event associated with this grace period, or null if there is
* no applicable billing event (i.e. this is a redemption grace period).
*/
@Nullable
public Ref<? extends BillingEvent> getBillingEvent() {
return Optional.<Ref<? extends BillingEvent>>fromNullable(billingEventOneTime)
.or(Optional.<Ref<? extends BillingEvent>>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<BillingEvent.OneTime> billingEventOneTime,
@Nullable Ref<BillingEvent.Recurring> 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.
*
* <p>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<? extends BillingEvent> 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.Recurring>) billingEvent;
} else {
instance.billingEventOneTime = (Ref<BillingEvent.OneTime>) billingEvent;
@Nullable Ref<BillingEvent.OneTime> billingEventOneTime) {
return createInternal(type, expirationTime, clientId, billingEventOneTime, null);
}
return instance;
/** Create a GracePeriod for a Recurring billing event. */
public static GracePeriod createForRecurring(
GracePeriodStatus type,
DateTime expirationTime,
String clientId,
Ref<BillingEvent.Recurring> 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. */

8
javatests/com/google/domain/registry/flows/FlowTestCase.java
View file

@ -227,7 +227,13 @@ public abstract class FlowTestCase<F extends Flow> {
new Function<GracePeriod, BillingEvent>() {
@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));

2
javatests/com/google/domain/registry/flows/domain/DomainDeleteFlowTest.java
View file

@ -143,7 +143,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
.build());
domain = persistResource(
domain.asBuilder()
.setGracePeriods(ImmutableSet.of(GracePeriod.create(
.setGracePeriods(ImmutableSet.of(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
A_MONTH_AGO.plusDays(45),
"TheRegistrar",

9
javatests/com/google/domain/registry/flows/domain/DomainInfoFlowTest.java
View file

@ -32,6 +32,7 @@ import com.google.domain.registry.flows.domain.DomainFlowUtils.BadPeriodUnitExce
import com.google.domain.registry.flows.domain.DomainFlowUtils.CurrencyUnitMismatchException;
import com.google.domain.registry.flows.domain.DomainFlowUtils.FeeChecksDontSupportPhasesException;
import com.google.domain.registry.flows.domain.DomainFlowUtils.RestoresAreAlwaysForOneYearException;
import com.google.domain.registry.model.billing.BillingEvent.Recurring;
import com.google.domain.registry.model.contact.ContactAuthInfo;
import com.google.domain.registry.model.contact.ContactResource;
import com.google.domain.registry.model.domain.DesignatedContact;
@ -47,6 +48,7 @@ import com.google.domain.registry.model.eppcommon.StatusValue;
import com.google.domain.registry.model.host.HostResource;
import com.google.domain.registry.testing.AppEngineRule;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.Ref;
import org.joda.time.DateTime;
@ -278,8 +280,11 @@ public class DomainInfoFlowTest extends ResourceFlowTestCase<DomainInfoFlow, Dom
persistTestEntities(false);
// Add an AUTO_RENEW grace period to the saved resource.
persistResource(domain.asBuilder()
.addGracePeriod(GracePeriod.create(
GracePeriodStatus.AUTO_RENEW, clock.nowUtc().plusDays(1), "foo", null))
.addGracePeriod(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
clock.nowUtc().plusDays(1),
"foo",
Ref.create(Key.create(Recurring.class, 12345))))
.build());
doSuccessfulTest("domain_info_response_autorenewperiod.xml", false);
}

2
javatests/com/google/domain/registry/model/domain/DomainResourceTest.java
View file

@ -416,7 +416,7 @@ public class DomainResourceTest extends EntityTestCase {
.isEqualTo(oldExpirationTime.plusYears(3));
assertThat(renewedThreeTimes.getLastEppUpdateTime()).isEqualTo(clock.nowUtc());
assertThat(renewedThreeTimes.getGracePeriods())
.containsExactly(GracePeriod.create(
.containsExactly(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
oldExpirationTime.plusYears(2).plus(
Registry.get("com").getAutoRenewGracePeriodLength()),

115
javatests/com/google/domain/registry/model/domain/GracePeriodTest.java Normal file
View file

@ -0,0 +1,115 @@
// Copyright 2016 Google Inc. 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 com.google.domain.registry.model.domain;
import static com.google.common.truth.Truth.assertThat;
import static org.joda.time.DateTimeZone.UTC;
import com.google.domain.registry.model.billing.BillingEvent;
import com.google.domain.registry.model.billing.BillingEvent.Reason;
import com.google.domain.registry.model.billing.BillingEvent.Recurring;
import com.google.domain.registry.model.domain.rgp.GracePeriodStatus;
import com.google.domain.registry.model.reporting.HistoryEntry;
import com.google.domain.registry.testing.AppEngineRule;
import com.google.domain.registry.testing.ExceptionRule;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.Ref;
import org.joda.money.CurrencyUnit;
import org.joda.money.Money;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/** Unit tests for {@link GracePeriod}. */
@RunWith(JUnit4.class)
public class GracePeriodTest {
@Rule
public final AppEngineRule appEngine = AppEngineRule.builder()
.withDatastore() // Needed to be able to construct Keys.
.build();
@Rule
public final ExceptionRule thrown = new ExceptionRule();
private final DateTime now = DateTime.now(UTC);
private BillingEvent.OneTime onetime;
@Before
public void before() {
onetime = new BillingEvent.OneTime.Builder()
.setEventTime(now)
.setBillingTime(now.plusDays(1))
.setClientId("TheRegistrar")
.setCost(Money.of(CurrencyUnit.USD, 42))
.setParent(Key.create(HistoryEntry.class, 12345))
.setReason(Reason.CREATE)
.setPeriodYears(1)
.setTargetId("foo.google")
.build();
}
@Test
public void testSuccess_forBillingEvent() {
GracePeriod gracePeriod = GracePeriod.forBillingEvent(GracePeriodStatus.ADD, onetime);
assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.ADD);
assertThat(gracePeriod.getOneTimeBillingEvent()).isEqualTo(Ref.create(onetime));
assertThat(gracePeriod.getRecurringBillingEvent()).isNull();
assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar");
assertThat(gracePeriod.getExpirationTime()).isEqualTo(now.plusDays(1));
assertThat(gracePeriod.isSunrushAddGracePeriod()).isFalse();
assertThat(gracePeriod.hasBillingEvent()).isTrue();
}
@Test
public void testSuccess_forBillingEvent_sunrushAdd() {
GracePeriod gracePeriod = GracePeriod.forBillingEvent(GracePeriodStatus.SUNRUSH_ADD, onetime);
assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.ADD);
assertThat(gracePeriod.isSunrushAddGracePeriod()).isTrue();
}
@Test
public void testSuccess_createWithoutBillingEvent() {
GracePeriod gracePeriod = GracePeriod.createWithoutBillingEvent(
GracePeriodStatus.REDEMPTION, now, "TheRegistrar");
assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.REDEMPTION);
assertThat(gracePeriod.getOneTimeBillingEvent()).isNull();
assertThat(gracePeriod.getRecurringBillingEvent()).isNull();
assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar");
assertThat(gracePeriod.getExpirationTime()).isEqualTo(now);
assertThat(gracePeriod.hasBillingEvent()).isFalse();
}
@Test
public void testFailure_forBillingEvent_autoRenew() {
thrown.expect(IllegalArgumentException.class, "autorenew");
GracePeriod.forBillingEvent(GracePeriodStatus.AUTO_RENEW, onetime);
}
@Test
public void testFailure_createForRecurring_notAutoRenew() {
thrown.expect(IllegalArgumentException.class, "autorenew");
GracePeriod.createForRecurring(
GracePeriodStatus.RENEW,
now.plusDays(1),
"TheRegistrar",
Ref.create(Key.create(Recurring.class, 12345)));
}
}