diff --git a/core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java b/core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java index 34c1f2845..e1f963e4a 100644 --- a/core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java +++ b/core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java @@ -131,12 +131,14 @@ public class ExpandRecurringBillingEventsAction implements Runnable { + "WHERE eventTime <= :executeTime " + "AND eventTime < recurrenceEndTime " + "AND id > :maxProcessedRecurrenceId " - + "AND recurrenceEndTime > :cursorTime " + + "AND recurrenceEndTime > :adjustedCursorTime " + "ORDER BY id ASC", Recurring.class) .setParameter("executeTime", executeTime) .setParameter("maxProcessedRecurrenceId", prevMaxProcessedRecurrenceId) - .setParameter("cursorTime", cursorTime) + .setParameter( + "adjustedCursorTime", + cursorTime.minus(Registry.DEFAULT_AUTO_RENEW_GRACE_PERIOD)) .setMaxResults(batchSize) .getResultList(); for (Recurring recurring : recurrings) { @@ -279,6 +281,19 @@ public class ExpandRecurringBillingEventsAction implements Runnable { .setReason("Domain autorenewal by ExpandRecurringBillingEventsAction") .setRequestedByRegistrar(false) .setType(DOMAIN_AUTORENEW) + // Note: the following statement seems to not be entirely correct as manual renewal + // during the autorenew grace period also closes out the existing recurrence, but in + // that instance the autorenew history entry should still have the transaction records + // for obvious reasons. It can be argued the history entry should always have the + // transaction record, regardless of what happens afterward. If the domain is deleted + // later during the autorenew grace period, another history entry for the delete would + // record that mutation separately, but the previous autorenew should not have its + // history entry retroactively altered, or in this case have the transaction records + // omitted when its created belatedly (when billing time is in scope). However, since + // we will be rewriting this action and only want to do the absolute minimum change to + // fix it for now, we will leave the current logic in place to avoid any unnecessary + // complications. + // // 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). diff --git a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java index 10a12f919..170ae191f 100644 --- a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java +++ b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java @@ -455,6 +455,45 @@ public class ExpandRecurringBillingEventsActionTest { assertCursorAt(currentTestTime); } + @Test + void testSuccess_noEventExpanded_recurrenceEndAfterEvent_cursorTimeTooLate() throws Exception { + // This can occur when a domain is transferred/renewed/deleted during autorenew grace peroid. + recurring = + persistResource( + recurring + .asBuilder() + .setRecurrenceEndTime(recurring.getEventTime().plusDays(5)) + .build()); + action.cursorTimeParam = Optional.of(recurring.getRecurrenceEndTime().plusDays(46)); + runAction(); + // No new history entries should be generated because cursor time is more than 45 days after + // recurrence end time. + assertThat(getHistoryEntriesOfType(domain, DOMAIN_AUTORENEW)).isEmpty(); + assertBillingEventsForResource(domain, recurring); + assertCursorAt(currentTestTime); + } + + @Test + void testSuccess_expandSingleEvent_recurrenceEndAfterEvent() throws Exception { + // This can occur when a domain is transferred/renewed/deleted during autorenew grace peroid. + recurring = + persistResource( + recurring + .asBuilder() + .setRecurrenceEndTime(recurring.getEventTime().plusDays(5)) + .build()); + action.cursorTimeParam = Optional.of(recurring.getRecurrenceEndTime().plusDays(35)); + runAction(); + DomainHistory persistedEntry = + getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW, DomainHistory.class); + assertHistoryEntryMatches( + domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), false); + BillingEvent.OneTime expected = + defaultOneTimeBuilder().setDomainHistory(persistedEntry).build(); + assertBillingEventsForResource(domain, expected, recurring); + assertCursorAt(currentTestTime); + } + @Test void testSuccess_expandSingleEvent_dryRun() throws Exception { persistResource(recurring);