Grandfather in old data for one-time billing event requirement (#1423)

* Grandfather in old data for one-time billing event requirement

We have data from 2018 and earlier where we didn't consistently set periodYears
for OneTime BillingEvents with certain reasons. This grandfathers in that old
data so that we can successfully move it over to Cloud SQL for now, then we can
later run a query that will backfill it, after which we can then tighten up the
requirement again. Note that the requirement is still being enforced for all
billing events from 2019 onwards.

This also improves the handling of validation, by adding a private field to the
Reason enum rather than creating a throwaway inline ImmmutableSet in the
Builder.

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,
+    ERROR(false),
-    FEE_EARLY_ACCESS,
+    FEE_EARLY_ACCESS(true),
-    RENEW,
+    RENEW(true),
-    RESTORE,
+    RESTORE(true),
-    SERVER_STATUS,
+    SERVER_STATUS(false),
-    TRANSFER
+    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.
+     *
+     * <p>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<Reason> reasonsWithPeriods =
+        // TODO(mcilwain): Enforce this check on all billing events (not just more recent ones)
-            Sets.immutableEnumSet(
+        //                 post-migration after we add the missing period years values in SQL.
-                Reason.CREATE,
+        if (instance.eventTime.isAfter(DateTime.parse("2019-01-01T00:00:00Z"))) {
-                Reason.FEE_EARLY_ACCESS,
+          checkState(
-                Reason.RENEW,
+              instance.reason.hasPeriodYears() == (instance.periodYears != null),
-                Reason.RESTORE,
+              "Period years must be set if and only if reason is "
-                Reason.TRANSFER);
+                  + "CREATE, FEE_EARLY_ACCESS, RENEW, RESTORE or 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.");
         checkState(
             instance.getFlags().contains(Flag.SYNTHETIC)
                 == (instance.syntheticCreationTime != null),

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();
+  }
 }