Don't write TX records for domains deleted in autorenew grace period (#244)

* Don't write TX records for domains deleted in autorenew grace period

When the project was originally being designed, we envisioned have a purely
point-in-time architecture that would allow the system to run indefinitely
without requiring any background batch jobs. That is, you could create a domain,
and 10 years later you could infer every autorenewal billing event that should
have happened during those 10 years, without ever having to run any code that
would go through and retroactively create those events as they happened.

This ended up being very complicated, especially when it came to generating
invoices, so we gave up on it and instead wrote the
ExpandRecurringBillingEventsAction mapreduce, which would run as a cronjob and
periodically expand the recurring billing information into actual one-time
billing events. This made the invoicing scripts MUCH less complicated since they
only had to tabulate one-time billing events that had actually occurred over the
past month, rather than perform complicated logic to infer every one-time event
over an arbitrarily long period.

I bring this up because this architectural legacy explains why billing events
are more complicated than could otherwise be explained from current
requirements. This is why, for instance, when a domain is deleted during the 45
day autorenewal period, the ExpandRecurringBillingEventsAction will still write
out a history entry (and corresponding billing events) on the 45th day, because
it needs to be offset by the cancellation billing event for the autorenew grace
period that was already written out synchronously as part of the delete flow.

This no longer really makes sense, and it would be simpler to just not write out
these phantom history entries and billing events at all, but it would be a
larger modification to fix this, so I'm not touching it here.

Instead, what I have done is to simply not write out the DomainTransactionRecord
in the mapreduce if the recurring billing event has already been canceled
(i.e. because the domain was deleted or transferred). This seems inconsistent
but actually does make sense, because domain transaction records are never
written out speculatively (unlike history entries and billing events); they
correspond only to actions that have actually happen.  This is because they were
architected much more recently than billing events, and don't use the
point-in-time hierarchy.

So, here's a full accounting of how DomainTransactionRecords work as of this commit:
1. When a domain is created, one is written out.
2. When a domain is explicitly renewed, one is written out.
3. When a domain is autorenewed, one is written out at the end of the grace period.
4. When a domain is deleted (in all cases), a record is written out recording the
   deletion.
5. When a domain is deleted in the add grace period, an offsetting record is
   written out with a negative number of years, in addition to the deletion record.
6. When a domain is deleted in the renewal grace period, an offsetting record is
   likely written out in addition.
7. When a domain is deleted in the autorenew grace period, there is no record that
   needs to be offset because no code ran at the exact time of the autorenew, so
   NO additional record should be written out by the expand mapreduce.
   *THIS IS CHANGED AS OF THIS COMMIT*.
8. When a domain is transferred, all existing grace periods are cancelled and
   corresponding cancelling records are written out. Note that transfers include a
   mandatory, irrevocable 1 year renewal.
9. In the rare event that a domain is restored, all recurring events are
   re-created, and there is a 1 year mandatory renewal as part of the restore with
   corresponding record written out.

So, in summary, billing events and history entries are often written out
speculatively, and can subsequently be canceled, but the same is not true of
domain transaction records.  Domain transaction records are only written out as
part of a corresponding action (which for autorenewals is the expand recurring
cronjob).

* rm unused import

core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java

@@ -190,14 +190,18 @@ public class ExpandRecurringBillingEventsAction implements Runnable {
                 .setReason("Domain autorenewal by ExpandRecurringBillingEventsAction")
                 .setRequestedByRegistrar(false)
                 .setType(DOMAIN_AUTORENEW)
+                // Don't write a domain transaction record if the recurrence was ended prior to the
+                // billing time (i.e. a domain was deleted during the autorenew grace period).
                 .setDomainTransactionRecords(
-                    ImmutableSet.of(
+                    recurring.getRecurrenceEndTime().isBefore(billingTime)
-                        DomainTransactionRecord.create(
+                        ? ImmutableSet.of()
-                            tld.getTldStr(),
+                        : ImmutableSet.of(
-                            // We report this when the autorenew grace period ends
+                            DomainTransactionRecord.create(
-                            billingTime,
+                                tld.getTldStr(),
-                            TransactionReportField.netRenewsFieldFromYears(1),
+                                // We report this when the autorenew grace period ends
-                            1)))
+                                billingTime,
+                                TransactionReportField.netRenewsFieldFromYears(1),
+                                1)))
                 .build();
             historyEntriesBuilder.add(historyEntry);
 

core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java

@@ -25,7 +25,7 @@ import static google.registry.testing.DatastoreHelper.assertBillingEventsForReso
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getHistoryEntriesOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
-import static google.registry.testing.DatastoreHelper.persistActiveDomain;
+import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
 import static google.registry.testing.DatastoreHelper.persistPremiumList;
 import static google.registry.testing.DatastoreHelper.persistResource;
@@ -87,7 +87,8 @@ public class ExpandRecurringBillingEventsActionTest
     action.clock = clock;
     action.cursorTimeParam = Optional.empty();
     createTld("tld");
-    domain = persistActiveDomain("example.tld");
+    domain = persistResource(newDomainBase("example.tld").asBuilder()
+        .setCreationTimeForTest(DateTime.parse("1999-01-05T00:00:00Z")).build());
     historyEntry = persistResource(new HistoryEntry.Builder().setParent(domain).build());
     recurring = new BillingEvent.Recurring.Builder()
         .setParent(historyEntry)
@@ -101,25 +102,26 @@ public class ExpandRecurringBillingEventsActionTest
         .build();
   }
 
-  void saveCursor(final DateTime cursorTime) {
+  private void saveCursor(final DateTime cursorTime) {
     tm().transact(() -> ofy().save().entity(Cursor.createGlobal(RECURRING_BILLING, cursorTime)));
   }
 
-  void runMapreduce() throws Exception {
+  private void runMapreduce() throws Exception {
     action.response = new FakeResponse();
     action.run();
     executeTasksUntilEmpty("mapreduce", clock);
     ofy().clearSessionCache();
   }
 
-  void assertCursorAt(DateTime expectedCursorTime) {
+  private void assertCursorAt(DateTime expectedCursorTime) {
     Cursor cursor = ofy().load().key(Cursor.createGlobalKey(RECURRING_BILLING)).now();
     assertThat(cursor).isNotNull();
     assertThat(cursor.getCursorTime()).isEqualTo(expectedCursorTime);
   }
 
-  void assertHistoryEntryMatches(
+  private void assertHistoryEntryMatches(
-      DomainBase domain, HistoryEntry actual, String clientId, DateTime billingTime) {
+      DomainBase domain, HistoryEntry actual, String clientId, DateTime billingTime,
+      boolean shouldHaveTxRecord) {
     assertThat(actual.getBySuperuser()).isFalse();
     assertThat(actual.getClientId()).isEqualTo(clientId);
     assertThat(actual.getParent()).isEqualTo(Key.create(domain));
@@ -128,13 +130,17 @@ public class ExpandRecurringBillingEventsActionTest
         .isEqualTo("Domain autorenewal by ExpandRecurringBillingEventsAction");
     assertThat(actual.getRequestedByRegistrar()).isFalse();
     assertThat(actual.getType()).isEqualTo(DOMAIN_AUTORENEW);
-    assertThat(actual.getDomainTransactionRecords())
+    if (shouldHaveTxRecord) {
-        .containsExactly(
+      assertThat(actual.getDomainTransactionRecords())
-            DomainTransactionRecord.create(
+          .containsExactly(
-                "tld",
+              DomainTransactionRecord.create(
-                billingTime,
+                  "tld",
-                TransactionReportField.NET_RENEWS_1_YR,
+                  billingTime,
-                1));
+                  TransactionReportField.NET_RENEWS_1_YR,
+                  1));
+    } else {
+      assertThat(actual.getDomainTransactionRecords()).isEmpty();
+    }
   }
 
   private OneTime.Builder defaultOneTimeBuilder() {
@@ -158,7 +164,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setParent(persistedEntry)
         .build();
@@ -185,7 +191,8 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(deletedDomain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        deletedDomain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        deletedDomain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"),
+        true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setParent(persistedEntry)
         .setTargetId(deletedDomain.getFullyQualifiedDomainName())
@@ -201,7 +208,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
     assertCursorAt(beginningOfTest);
     DateTime beginningOfSecondRun = clock.nowUtc();
@@ -234,7 +241,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
     // Persist an otherwise identical billing event that differs only in billing time.
     BillingEvent.OneTime persisted = persistResource(expected.asBuilder()
@@ -258,7 +265,7 @@ public class ExpandRecurringBillingEventsActionTest
         getHistoryEntriesOfType(domain, DOMAIN_AUTORENEW);
     for (HistoryEntry persistedEntry : persistedEntries) {
       assertHistoryEntryMatches(
-          domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+          domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     }
     assertThat(persistedEntries).hasSize(2);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
@@ -306,7 +313,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
     assertBillingEventsForResource(domain, expected, recurring);
     assertCursorAt(beginningOfTest);
@@ -320,7 +327,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
     assertBillingEventsForResource(domain, expected, recurring);
     assertCursorAt(beginningOfTest);
@@ -354,7 +361,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2002-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2002-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setBillingTime(DateTime.parse("2002-02-19T00:00:00Z"))
         .setEventTime(DateTime.parse("2002-01-05T00:00:00Z"))
@@ -372,7 +379,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
     assertBillingEventsForResource(domain, expected, recurring);
     assertCursorAt(beginningOfTest);
@@ -435,7 +442,7 @@ public class ExpandRecurringBillingEventsActionTest
           domain,
           persistedEntries.get(year),
           "TheRegistrar",
-          billingDate.plusYears(year));
+          billingDate.plusYears(year), true);
       expectedEvents.add(defaultOneTimeBuilder()
           .setBillingTime(billingDate.plusYears(year))
           .setEventTime(eventDate.plusYears(year))
@@ -463,7 +470,7 @@ public class ExpandRecurringBillingEventsActionTest
     // Only expect the last two years' worth of billing events.
     for (int year = 0; year < 2; year++) {
       assertHistoryEntryMatches(
-          domain, persistedEntries.get(year), "TheRegistrar", billingDate.plusYears(year));
+          domain, persistedEntries.get(year), "TheRegistrar", billingDate.plusYears(year), true);
       expectedEvents.add(defaultOneTimeBuilder()
           .setBillingTime(billingDate.plusYears(year))
           .setParent(persistedEntries.get(year))
@@ -490,19 +497,47 @@ public class ExpandRecurringBillingEventsActionTest
   }
 
   @Test
-  public void testSuccess_singleEvent_afterRecurrenceEnd() throws Exception {
+  public void testSuccess_singleEvent_afterRecurrenceEnd_inAutorenewGracePeriod() throws Exception {
-    DateTime testTime = beginningOfTest.plusYears(2);
+    // The domain creation date is 1999-01-05, and the first renewal date is thus 2000-01-05.
+    DateTime testTime = DateTime.parse("2001-02-06T00:00:00Z");
     clock.setTo(testTime);
     recurring = persistResource(recurring.asBuilder()
-        // Set between event time and billing time (i.e. before the grace period expires) for 2000.
+        // The domain deletion date is 2000-01-29, which is within the 45 day autorenew grace period
-        // We should still expect a billing event.
+        // from the renewal date.
         .setRecurrenceEndTime(DateTime.parse("2000-01-29T00:00:00Z"))
+        .setEventTime(domain.getCreationTime().plusYears(1))
         .build());
     action.cursorTimeParam = Optional.of(START_OF_TIME);
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), false);
+    BillingEvent.OneTime expected = defaultOneTimeBuilder()
+        .setBillingTime(DateTime.parse("2000-02-19T00:00:00Z"))
+        .setParent(persistedEntry)
+        .setSyntheticCreationTime(testTime)
+        .build();
+    assertBillingEventsForResource(domain, recurring, expected);
+    assertCursorAt(testTime);
+  }
+
+  @Test
+  public void testSuccess_singleEvent_afterRecurrenceEnd_outsideAutorenewGracePeriod()
+      throws Exception {
+    // The domain creation date is 1999-01-05, and the first renewal date is thus 2000-01-05.
+    DateTime testTime = DateTime.parse("2001-02-06T00:00:00Z");
+    clock.setTo(testTime);
+    recurring = persistResource(recurring.asBuilder()
+        // The domain deletion date is 2000-04-05, which is not within the 45 day autorenew grace
+        // period from the renewal date.
+        .setRecurrenceEndTime(DateTime.parse("2000-04-05T00:00:00Z"))
+        .setEventTime(domain.getCreationTime().plusYears(1))
+        .build());
+    action.cursorTimeParam = Optional.of(START_OF_TIME);
+    runMapreduce();
+    HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
+    assertHistoryEntryMatches(
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setBillingTime(DateTime.parse("2000-02-19T00:00:00Z"))
         .setParent(persistedEntry)
@@ -521,7 +556,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-29T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-29T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setBillingTime(DateTime.parse("2000-02-29T00:00:00Z"))
         .setEventTime(DateTime.parse("2000-01-15T00:00:00Z"))
@@ -542,7 +577,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2001-03-01T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2001-03-01T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setBillingTime(DateTime.parse("2001-03-01T00:00:00Z"))
         .setEventTime(DateTime.parse("2001-01-15T00:00:00Z"))
@@ -565,13 +600,15 @@ public class ExpandRecurringBillingEventsActionTest
     List<HistoryEntry> persistedEntries = getHistoryEntriesOfType(domain, DOMAIN_AUTORENEW);
     assertThat(persistedEntries).hasSize(2);
     assertHistoryEntryMatches(
-        domain, persistedEntries.get(0), "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntries.get(0), "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"),
+        true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setParent(persistedEntries.get(0))
         .setCancellationMatchingBillingEvent(Key.create(recurring))
         .build();
     assertHistoryEntryMatches(
-        domain, persistedEntries.get(1), "TheRegistrar", DateTime.parse("2000-05-20T00:00:00Z"));
+        domain, persistedEntries.get(1), "TheRegistrar", DateTime.parse("2000-05-20T00:00:00Z"),
+        true);
     BillingEvent.OneTime expected2 = defaultOneTimeBuilder()
         .setBillingTime(DateTime.parse("2000-05-20T00:00:00Z"))
         .setEventTime(DateTime.parse("2000-04-05T00:00:00Z"))
@@ -594,7 +631,7 @@ public class ExpandRecurringBillingEventsActionTest
     runMapreduce();
     HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW);
     assertHistoryEntryMatches(
-        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"));
+        domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder()
         .setParent(persistedEntry)
         .setCost(Money.of(USD, 100))
@@ -623,7 +660,7 @@ public class ExpandRecurringBillingEventsActionTest
     assertThat(persistedEntries).hasSize(2);
     DateTime eventDate = DateTime.parse("2000-01-05T00:00:00Z");
     DateTime billingDate = DateTime.parse("2000-02-19T00:00:00Z");
-    assertHistoryEntryMatches(domain, persistedEntries.get(0), "TheRegistrar", billingDate);
+    assertHistoryEntryMatches(domain, persistedEntries.get(0), "TheRegistrar", billingDate, true);
     BillingEvent.OneTime cheaper = defaultOneTimeBuilder()
         .setBillingTime(billingDate)
         .setEventTime(eventDate)
@@ -632,7 +669,7 @@ public class ExpandRecurringBillingEventsActionTest
         .setSyntheticCreationTime(testTime)
         .build();
     assertHistoryEntryMatches(
-        domain, persistedEntries.get(1), "TheRegistrar", billingDate.plusYears(1));
+        domain, persistedEntries.get(1), "TheRegistrar", billingDate.plusYears(1), true);
     BillingEvent.OneTime expensive = cheaper.asBuilder()
         .setCost(Money.of(USD, 10))
         .setBillingTime(billingDate.plusYears(1))