From cde1c78f5e7c6ecf8369ff4ee9f2de8e7e85c624 Mon Sep 17 00:00:00 2001
From: Michael Muller <mmuller@google.com>
Date: Wed, 25 Nov 2020 11:15:10 -0500
Subject: [PATCH] Add replay-testing to DomainDeleteFlowTest (#886)

* Minor fixes:

- Initialize "requestedByRegistrar" to false (it's non-nullable).
- Store test entities (registrar, hosts and contacts) in JPA.

* Flyway changes

* Add ReplayExtension to DomainDeleteFlowTest

* Check in latest ER diagrams
---
 .../flows/domain/DomainDeleteFlowTest.java    | 49 ++++++++++++++-----
 .../registry/testing/DatabaseHelper.java      | 13 +++--
 .../sql/er_diagram/brief_er_diagram.html      |  4 +-
 .../sql/er_diagram/full_er_diagram.html       | 14 +++---
 4 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java
index c86c3f4dd..0b942a29a 100644
--- a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java
@@ -96,17 +96,24 @@ import google.registry.model.transfer.DomainTransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import google.registry.persistence.VKey;
+import google.registry.testing.ReplayExtension;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import java.util.Map;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /** Unit tests for {@link DomainDeleteFlow}. */
 class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, DomainBase> {
 
+  @Order(value = Order.DEFAULT - 2)
+  @RegisterExtension
+  final ReplayExtension replayExtension = new ReplayExtension(clock);
+
   private DomainBase domain;
   private HistoryEntry earlierHistoryEntry;
 
@@ -137,6 +144,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
         persistResource(createAutorenewBillingEvent("TheRegistrar").build());
     PollMessage.Autorenew autorenewPollMessage =
         persistResource(createAutorenewPollMessage("TheRegistrar").build());
+
     domain =
         persistResource(
             domain
@@ -144,6 +152,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
                 .setAutorenewBillingEvent(autorenewBillingEvent.createVKey())
                 .setAutorenewPollMessage(autorenewPollMessage.createVKey())
                 .build());
+
     assertTransactionalFlow(true);
   }
 
@@ -151,15 +160,20 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
     // Persist a linked contact.
     ContactResource contact = persistActiveContact("sh8013");
     domain =
-        newDomainBase(getUniqueIdFromCommand())
-            .asBuilder()
-            .setCreationTimeForTest(TIME_BEFORE_FLOW)
-            .setRegistrant(contact.createVKey())
-            .setRegistrationExpirationTime(expirationTime)
-            .build();
+        persistResource(
+            newDomainBase(getUniqueIdFromCommand())
+                .asBuilder()
+                .setCreationTimeForTest(TIME_BEFORE_FLOW)
+                .setRegistrant(contact.createVKey())
+                .setRegistrationExpirationTime(expirationTime)
+                .build());
     earlierHistoryEntry =
         persistResource(
-            new HistoryEntry.Builder().setType(DOMAIN_CREATE).setParent(domain).build());
+            new HistoryEntry.Builder()
+                .setType(DOMAIN_CREATE)
+                .setParent(domain)
+                .setModificationTime(clock.nowUtc())
+                .build());
   }
 
   private void setUpGracePeriods(GracePeriod... gracePeriods) {
@@ -208,7 +222,14 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
 
   private void assertAutorenewClosedAndCancellationCreatedFor(
       BillingEvent.OneTime graceBillingEvent, HistoryEntry historyEntryDomainDelete) {
-    DateTime eventTime = clock.nowUtc();
+    assertAutorenewClosedAndCancellationCreatedFor(
+        graceBillingEvent, historyEntryDomainDelete, clock.nowUtc());
+  }
+
+  private void assertAutorenewClosedAndCancellationCreatedFor(
+      BillingEvent.OneTime graceBillingEvent,
+      HistoryEntry historyEntryDomainDelete,
+      DateTime eventTime) {
     assertBillingEvents(
         createAutorenewBillingEvent("TheRegistrar").setRecurrenceEndTime(eventTime).build(),
         graceBillingEvent,
@@ -293,7 +314,11 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
     setUpSuccessfulTest();
     setUpGracePeriods(
         GracePeriod.create(
-            GracePeriodStatus.ADD, domain.getRepoId(), TIME_BEFORE_FLOW.plusDays(1), "foo", null));
+            GracePeriodStatus.ADD,
+            domain.getRepoId(),
+            TIME_BEFORE_FLOW.plusDays(1),
+            "TheRegistrar",
+            null));
     dryRunFlowAssertResponse(loadFile("generic_success_response.xml"));
   }
 
@@ -398,7 +423,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
             GracePeriodStatus.TRANSFER,
             domain.getRepoId(),
             TIME_BEFORE_FLOW.plusDays(1),
-            "foo",
+            "NewRegistrar",
             null));
     // We should see exactly one poll message, which is for the autorenew 1 month in the future.
     assertPollMessages(createAutorenewPollMessage("TheRegistrar").build());
@@ -730,11 +755,11 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
             .setNameservers(ImmutableSet.of(host.createVKey()))
             .setDeletionTime(START_OF_TIME)
             .build());
-    clock.advanceOneMilli();
+    DateTime eventTime = clock.nowUtc();
     runFlowAssertResponse(loadFile("generic_success_response.xml"));
     assertDnsTasksEnqueued("example.tld");
     assertAutorenewClosedAndCancellationCreatedFor(
-        graceBillingEvent, getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE));
+        graceBillingEvent, getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE), eventTime);
   }
 
   @Test
diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java
index a2ccf28a5..8b1bf8454 100644
--- a/core/src/test/java/google/registry/testing/DatabaseHelper.java
+++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java
@@ -638,11 +638,13 @@ public class DatabaseHelper {
       DateTime requestTime,
       DateTime expirationTime,
       DateTime extendedRegistrationExpirationTime) {
-    HistoryEntry historyEntryDomainTransfer = persistResource(
-        new HistoryEntry.Builder()
-            .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST)
-            .setParent(domain)
-            .build());
+    HistoryEntry historyEntryDomainTransfer =
+        persistResource(
+            new HistoryEntry.Builder()
+                .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST)
+                .setModificationTime(tm().transact(() -> tm().getTransactionTime()))
+                .setParent(domain)
+                .build());
     BillingEvent.OneTime transferBillingEvent = persistResource(createBillingEventForTransfer(
             domain,
             historyEntryDomainTransfer,
@@ -1184,6 +1186,7 @@ public class DatabaseHelper {
   public static void deleteResource(final Object resource) {
     if (alwaysSaveWithBackup) {
       tm().transact(() -> tm().delete(resource));
+      maybeAdvanceClock();
     } else {
       transactIfJpaTm(() -> tm().deleteWithoutBackup(resource));
     }
diff --git a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html
index 28dcf5ae1..cb9d64cd0 100644
--- a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html
+++ b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html
@@ -261,7 +261,7 @@ td.section {
     </tr> 
     <tr> 
      <td class="property_name">generated on</td> 
-     <td class="property_value">2020-11-18 21:42:47.876348</td> 
+     <td class="property_value">2020-11-24 20:57:00.530791</td> 
     </tr> 
     <tr>
      <td class="property_name">last flyway file</td>
@@ -284,7 +284,7 @@ td.section {
      generated on
     </text> 
     <text text-anchor="start" x="4027.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
-     2020-11-18 21:42:47.876348
+     2020-11-24 20:57:00.530791
     </text> 
     <polygon fill="none" stroke="#888888" points="3940.44,-4 3940.44,-44 4205.44,-44 4205.44,-4 3940.44,-4" /> <!-- allocationtoken_a08ccbef --> 
     <g id="node1" class="node"> 
diff --git a/db/src/main/resources/sql/er_diagram/full_er_diagram.html b/db/src/main/resources/sql/er_diagram/full_er_diagram.html
index a4d9da2b7..3f12e4c00 100644
--- a/db/src/main/resources/sql/er_diagram/full_er_diagram.html
+++ b/db/src/main/resources/sql/er_diagram/full_er_diagram.html
@@ -261,7 +261,7 @@ td.section {
     </tr> 
     <tr> 
      <td class="property_name">generated on</td> 
-     <td class="property_value">2020-11-18 21:42:46.037281</td> 
+     <td class="property_value">2020-11-24 20:56:58.3401</td> 
     </tr> 
     <tr>
      <td class="property_name">last flyway file</td>
@@ -274,19 +274,19 @@ td.section {
   <svg viewbox="0.00 0.00 4825.18 4553.15" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" id="erDiagram" style="overflow: hidden; width: 100%; height: 800px"> <g id="graph0" class="graph" transform="scale(1 1) rotate(0) translate(4 4549.15)"> 
     <title>SchemaCrawler_Diagram</title> 
     <polygon fill="white" stroke="transparent" points="-4,4 -4,-4549.15 4821.18,-4549.15 4821.18,4 -4,4" /> 
-    <text text-anchor="start" x="4548.68" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="4563.18" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      generated by
     </text> 
-    <text text-anchor="start" x="4631.68" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="4646.18" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      SchemaCrawler 16.10.1
     </text> 
-    <text text-anchor="start" x="4547.68" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="4562.18" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      generated on
     </text> 
-    <text text-anchor="start" x="4631.68" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
-     2020-11-18 21:42:46.037281
+    <text text-anchor="start" x="4646.18" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+     2020-11-24 20:56:58.3401
     </text> 
-    <polygon fill="none" stroke="#888888" points="4544.18,-4 4544.18,-44 4809.18,-44 4809.18,-4 4544.18,-4" /> <!-- allocationtoken_a08ccbef --> 
+    <polygon fill="none" stroke="#888888" points="4559.18,-4 4559.18,-44 4809.18,-44 4809.18,-4 4559.18,-4" /> <!-- allocationtoken_a08ccbef --> 
     <g id="node1" class="node"> 
      <title>allocationtoken_a08ccbef</title> 
      <polygon fill="#ebcef2" stroke="transparent" points="2959.5,-2661.15 2959.5,-2680.15 3158.5,-2680.15 3158.5,-2661.15 2959.5,-2661.15" />