diff --git a/core/src/main/java/google/registry/model/billing/BillingEvent.java b/core/src/main/java/google/registry/model/billing/BillingEvent.java index f702fd9ab..af436282d 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -27,7 +27,6 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -77,14 +76,29 @@ public abstract class BillingEvent extends ImmutableObject /** The reason for the bill, which maps 1:1 to skus in go/registry-billing-skus. */ public enum Reason { - CREATE, + CREATE(true), @Deprecated // TODO(b/31676071): remove this legacy value once old data is cleaned up. - ERROR, - FEE_EARLY_ACCESS, - RENEW, - RESTORE, - SERVER_STATUS, - TRANSFER + ERROR(false), + FEE_EARLY_ACCESS(true), + RENEW(true), + RESTORE(true), + SERVER_STATUS(false), + TRANSFER(true); + + private final boolean requiresPeriod; + + Reason(boolean requiresPeriod) { + this.requiresPeriod = requiresPeriod; + } + + /** + * Returns whether billing events with this reason have a period years associated with them. + * + *

Note that this is an "if an only if" condition. + */ + public boolean hasPeriodYears() { + return requiresPeriod; + } } /** Set of flags that can be applied to billing events. */ @@ -268,7 +282,7 @@ public abstract class BillingEvent extends ImmutableObject public T build() { T instance = getInstance(); checkNotNull(instance.reason, "Reason must be set"); - checkNotNull(instance.clientId, "Client ID must be set"); + checkNotNull(instance.clientId, "Registrar ID must be set"); checkNotNull(instance.eventTime, "Event time must be set"); checkNotNull(instance.targetId, "Target ID must be set"); checkNotNull(instance.parent, "Parent must be set"); @@ -462,17 +476,14 @@ public abstract class BillingEvent extends ImmutableObject checkNotNull(instance.billingTime); checkNotNull(instance.cost); checkState(!instance.cost.isNegative(), "Costs should be non-negative."); - ImmutableSet reasonsWithPeriods = - Sets.immutableEnumSet( - Reason.CREATE, - Reason.FEE_EARLY_ACCESS, - Reason.RENEW, - Reason.RESTORE, - Reason.TRANSFER); - checkState( - reasonsWithPeriods.contains(instance.reason) == (instance.periodYears != null), - "Period years must be set if and only if reason is " - + "CREATE, FEE_EARLY_ACCESS, RENEW, RESTORE or TRANSFER."); + // TODO(mcilwain): Enforce this check on all billing events (not just more recent ones) + // post-migration after we add the missing period years values in SQL. + if (instance.eventTime.isAfter(DateTime.parse("2019-01-01T00:00:00Z"))) { + checkState( + instance.reason.hasPeriodYears() == (instance.periodYears != null), + "Period years must be set if and only if reason is " + + "CREATE, FEE_EARLY_ACCESS, RENEW, RESTORE or TRANSFER."); + } checkState( instance.getFlags().contains(Flag.SYNTHETIC) == (instance.syntheticCreationTime != null), diff --git a/core/src/test/java/google/registry/model/billing/BillingEventTest.java b/core/src/test/java/google/registry/model/billing/BillingEventTest.java index d7b284e35..5acdd1c6a 100644 --- a/core/src/test/java/google/registry/model/billing/BillingEventTest.java +++ b/core/src/test/java/google/registry/model/billing/BillingEventTest.java @@ -416,4 +416,62 @@ public class BillingEventTest extends EntityTestCase { assertThat(recurring.getParentKey()).isEqualTo(Key.create(domainHistory)); new BillingEvent.OneTime.Builder().setParent(Key.create(domainHistory)); } + + @TestOfyAndSql + void testReasonRequiringPeriodYears_missingPeriodYears_throwsException() { + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, + () -> + new BillingEvent.OneTime.Builder() + .setBillingTime(DateTime.parse("2020-02-05T15:33:11Z")) + .setEventTime(DateTime.parse("2020-01-05T15:33:11Z")) + .setCost(Money.of(USD, 10)) + .setReason(Reason.RENEW) + .setCost(Money.of(USD, 10)) + .setRegistrarId("TheRegistrar") + .setTargetId("example.tld") + .setParent(domainHistory) + .build()); + assertThat(thrown) + .hasMessageThat() + .contains("Period years must be set if and only if reason is"); + } + + @TestOfyAndSql + void testReasonNotRequiringPeriodYears_havingPeriodYears_throwsException() { + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, + () -> + new BillingEvent.OneTime.Builder() + .setBillingTime(DateTime.parse("2020-02-05T15:33:11Z")) + .setEventTime(DateTime.parse("2020-01-05T15:33:11Z")) + .setCost(Money.of(USD, 10)) + .setPeriodYears(2) + .setReason(Reason.SERVER_STATUS) + .setCost(Money.of(USD, 10)) + .setRegistrarId("TheRegistrar") + .setTargetId("example.tld") + .setParent(domainHistory) + .build()); + assertThat(thrown) + .hasMessageThat() + .contains("Period years must be set if and only if reason is"); + } + + @TestOfyAndSql + void testReasonRequiringPeriodYears_missingPeriodYears_isAllowedOnOldData() { + // This won't throw even though periodYears is missing on a RESTORE because the event time + // is before 2019. + new BillingEvent.OneTime.Builder() + .setBillingTime(DateTime.parse("2018-02-05T15:33:11Z")) + .setEventTime(DateTime.parse("2018-01-05T15:33:11Z")) + .setReason(Reason.RESTORE) + .setCost(Money.of(USD, 10)) + .setRegistrarId("TheRegistrar") + .setTargetId("example.tld") + .setParent(domainHistory) + .build(); + } }