From 127ae08790f6d97f2b23c0d0d277d5ee08fb21cc Mon Sep 17 00:00:00 2001
From: Ben McIlwain <mcilwain@google.com>
Date: Wed, 17 Mar 2021 15:39:13 -0400
Subject: [PATCH] Clear autorenew end time when a domain is restored (#1015)

* Clear autorenew end time when a domain is restored

This allows us to still see in the database which now-deleted domains had
reached expiration, while correctly not re-deleting the domain immediately if
the registrar pays to explicitly restore the domain.

This also resolves some TODOs around data migration for this field on domain so
that it's not null, as said migration has already been completed.
---
 .../flows/domain/DomainRestoreRequestFlow.java   |  3 +++
 .../registry/model/domain/DomainContent.java     | 13 +------------
 .../domain/DomainRestoreRequestFlowTest.java     | 16 ++++++++++++++++
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java
index f1ed7934d..38c4232f5 100644
--- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java
@@ -233,6 +233,9 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow  {
         .setDeletePollMessage(null)
         .setAutorenewBillingEvent(autorenewEvent.createVKey())
         .setAutorenewPollMessage(autorenewPollMessage.createVKey())
+        // Clear the autorenew end time so if it had expired but is now explicitly being restored,
+        // it won't immediately be deleted again.
+        .setAutorenewEndTime(Optional.empty())
         .setLastEppUpdateTime(now)
         .setLastEppUpdateClientId(clientId)
         .build();
diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java
index b037368c8..4ad21f982 100644
--- a/core/src/main/java/google/registry/model/domain/DomainContent.java
+++ b/core/src/main/java/google/registry/model/domain/DomainContent.java
@@ -302,12 +302,6 @@ public class DomainContent extends EppResource
 
   @OnLoad
   void load() {
-    // Back fill with correct END_OF_TIME sentinel value.
-    // TODO(mcilwain): Remove this once back-filling is complete.
-    if (autorenewEndTime == null) {
-      autorenewEndTime = END_OF_TIME;
-    }
-
     // Reconstitute all of the contacts so that they have VKeys.
     allContacts =
         allContacts.stream().map(DesignatedContact::reconstitute).collect(toImmutableSet());
@@ -440,12 +434,7 @@ public class DomainContent extends EppResource
    * purposes of more legible business logic.
    */
   public Optional<DateTime> getAutorenewEndTime() {
-    // TODO(mcilwain): Remove null handling for autorenewEndTime once data migration away from null
-    //                 is complete.
-    return Optional.ofNullable(
-        (autorenewEndTime == null || autorenewEndTime.equals(END_OF_TIME))
-            ? null
-            : autorenewEndTime);
+    return Optional.ofNullable(autorenewEndTime.equals(END_OF_TIME) ? null : autorenewEndTime);
   }
 
   @Override
diff --git a/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
index d5337a6a2..35c3fe9e2 100644
--- a/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
@@ -15,6 +15,7 @@
 package google.registry.flows.domain;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.assertBillingEvents;
 import static google.registry.testing.DatabaseHelper.assertPollMessages;
@@ -72,6 +73,7 @@ import google.registry.model.reporting.DomainTransactionRecord.TransactionReport
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.testing.ReplayExtension;
 import java.util.Map;
+import java.util.Optional;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
@@ -291,6 +293,20 @@ class DomainRestoreRequestFlowTest
             .build());
   }
 
+  @Test
+  void testSuccess_autorenewEndTimeIsCleared() throws Exception {
+    setEppInput("domain_update_restore_request_fee.xml", FEE_06_MAP);
+    persistPendingDeleteDomain();
+    persistResource(
+        reloadResourceByForeignKey()
+            .asBuilder()
+            .setAutorenewEndTime(Optional.of(clock.nowUtc().plusYears(2)))
+            .build());
+    assertThat(reloadResourceByForeignKey().getAutorenewEndTime()).isPresent();
+    runFlowAssertResponse(loadFile("domain_update_restore_request_response_fee.xml", FEE_06_MAP));
+    assertThat(reloadResourceByForeignKey().getAutorenewEndTime()).isEmpty();
+  }
+
   @Test
   void testSuccess_fee_v06() throws Exception {
     setEppInput("domain_update_restore_request_fee.xml", FEE_06_MAP);