Make RefreshDnsForAllDomains SQL-aware (#1197)

Also marks a few mapreduce actions as @Deprecated as they are no longer
needed in SQL.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1197)
<!-- Reviewable:end -->

core/src/main/java/google/registry/backup/DeleteOldCommitLogsAction.java

@@ -66,6 +66,8 @@ import org.joda.time.Duration;
     service = Action.Service.BACKEND,
     path = "/_dr/task/deleteOldCommitLogs",
     auth = Auth.AUTH_INTERNAL_OR_ADMIN)
+// No longer needed in SQL. Subject to future removal.
+@Deprecated
 public final class DeleteOldCommitLogsAction implements Runnable {
 
   private static final int NUM_MAP_SHARDS = 20;

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

@@ -54,6 +54,8 @@ import javax.inject.Inject;
     service = Action.Service.BACKEND,
     path = "/_dr/task/resaveAllEppResources",
     auth = Auth.AUTH_INTERNAL_OR_ADMIN)
+// No longer needed in SQL. Subject to future removal.
+@Deprecated
 public class ResaveAllEppResourcesAction implements Runnable {
 
   @Inject MapreduceRunner mrRunner;

core/src/main/java/google/registry/tools/server/KillAllCommitLogsAction.java

@@ -49,6 +49,8 @@ import javax.inject.Inject;
     path = "/_dr/task/killAllCommitLogs",
     method = POST,
     auth = Auth.AUTH_INTERNAL_OR_ADMIN)
+// No longer needed in SQL. Subject to future removal.
+@Deprecated
 public class KillAllCommitLogsAction implements Runnable {
 
   @Inject MapreduceRunner mrRunner;

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java

@@ -18,6 +18,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static google.registry.mapreduce.inputs.EppResourceInputs.createEntityInput;
 import static google.registry.model.EppResourceUtils.isActive;
 import static google.registry.model.registry.Registries.assertTldsExist;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.request.RequestParameters.PARAM_TLDS;
 
 import com.google.appengine.tools.mapreduce.Mapper;
@@ -32,11 +34,12 @@ import google.registry.request.Action;
 import google.registry.request.Parameter;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
+import google.registry.util.Clock;
 import google.registry.util.NonFinalForTesting;
 import java.util.Random;
 import javax.inject.Inject;
+import org.apache.http.HttpStatus;
 import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
 import org.joda.time.Duration;
 
 /**
@@ -74,6 +77,8 @@ public class RefreshDnsForAllDomainsAction implements Runnable {
   @Parameter("smearMinutes")
   int smearMinutes;
 
+  @Inject DnsQueue dnsQueue;
+  @Inject Clock clock;
   @Inject Random random;
 
   @Inject
@@ -83,14 +88,41 @@ public class RefreshDnsForAllDomainsAction implements Runnable {
   public void run() {
     assertTldsExist(tlds);
     checkArgument(smearMinutes > 0, "Must specify a positive number of smear minutes");
-    mrRunner
+    if (tm().isOfy()) {
-        .setJobName("Refresh DNS for all domains")
+      mrRunner
-        .setModuleName("tools")
+          .setJobName("Refresh DNS for all domains")
-        .setDefaultMapShards(10)
+          .setModuleName("tools")
-        .runMapOnly(
+          .setDefaultMapShards(10)
-            new RefreshDnsForAllDomainsActionMapper(tlds, smearMinutes, random),
+          .runMapOnly(
-            ImmutableList.of(createEntityInput(DomainBase.class)))
+              new RefreshDnsForAllDomainsActionMapper(tlds, smearMinutes, random, clock.nowUtc()),
-        .sendLinkToMapreduceConsole(response);
+              ImmutableList.of(createEntityInput(DomainBase.class)))
+          .sendLinkToMapreduceConsole(response);
+    } else {
+      tm().transact(
+              () ->
+                  jpaTm()
+                      .query(
+                          "SELECT fullyQualifiedDomainName FROM Domain "
+                              + "WHERE tld IN (:tlds) "
+                              + "AND deletionTime > :now",
+                          String.class)
+                      .setParameter("tlds", tlds)
+                      .setParameter("now", clock.nowUtc())
+                      .getResultStream()
+                      .forEach(
+                          domainName -> {
+                            try {
+                              // Smear the task execution time over the next N minutes.
+                              dnsQueue.addDomainRefreshTask(
+                                  domainName,
+                                  Duration.standardMinutes(random.nextInt(smearMinutes)));
+                            } catch (Throwable t) {
+                              logger.atSevere().withCause(t).log(
+                                  "Error while enqueuing DNS refresh for domain %s", domainName);
+                              response.setStatus(HttpStatus.SC_INTERNAL_SERVER_ERROR);
+                            }
+                          }));
+    }
   }
 
   /** Mapper to refresh DNS for all active domain resources. */
@@ -104,19 +136,21 @@ public class RefreshDnsForAllDomainsAction implements Runnable {
     private final ImmutableSet<String> tlds;
     private final int smearMinutes;
     private final Random random;
+    private final DateTime now;
 
     RefreshDnsForAllDomainsActionMapper(
-        ImmutableSet<String> tlds, int smearMinutes, Random random) {
+        ImmutableSet<String> tlds, int smearMinutes, Random random, DateTime now) {
       this.tlds = tlds;
       this.smearMinutes = smearMinutes;
       this.random = random;
+      this.now = now;
     }
 
     @Override
     public void map(final DomainBase domain) {
       String domainName = domain.getDomainName();
       if (tlds.contains(domain.getTld())) {
-        if (isActive(domain, DateTime.now(DateTimeZone.UTC))) {
+        if (isActive(domain, now)) {
           try {
             // Smear the task execution time over the next N minutes.
             dnsQueue.addDomainRefreshTask(
@@ -124,7 +158,7 @@ public class RefreshDnsForAllDomainsAction implements Runnable {
             getContext().incrementCounter("active domains refreshed");
           } catch (Throwable t) {
             logger.atSevere().withCause(t).log(
-                "Error while refreshing DNS for domain %s", domainName);
+                "Error while enqueuing DNS refresh for domain %s", domainName);
             getContext().incrementCounter("active domains errored");
           }
         } else {

core/src/main/java/google/registry/tools/server/ResaveAllHistoryEntriesAction.java

@@ -42,6 +42,8 @@ import javax.inject.Inject;
     service = Action.Service.TOOLS,
     path = "/_dr/task/resaveAllHistoryEntries",
     auth = Auth.AUTH_INTERNAL_OR_ADMIN)
+// No longer needed in SQL. Subject to future removal.
+@Deprecated
 public class ResaveAllHistoryEntriesAction implements Runnable {
 
   @Inject MapreduceRunner mrRunner;

core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java

@@ -18,37 +18,52 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
 import static google.registry.testing.DatabaseHelper.persistDeletedDomain;
-import static org.joda.time.DateTimeZone.UTC;
+import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued;
 import static org.joda.time.Duration.standardMinutes;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
 import com.google.common.collect.ImmutableSet;
 import google.registry.dns.DnsQueue;
+import google.registry.model.ofy.Ofy;
+import google.registry.testing.DualDatabaseTest;
+import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectExtension;
+import google.registry.testing.TestOfyAndSql;
+import google.registry.testing.TestSqlOnly;
 import google.registry.testing.mapreduce.MapreduceTestCase;
 import google.registry.tools.server.RefreshDnsForAllDomainsAction.RefreshDnsForAllDomainsActionMapper;
 import java.util.Random;
+import org.apache.http.HttpStatus;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.ArgumentCaptor;
 
 /** Unit tests for {@link RefreshDnsForAllDomainsAction}. */
+@DualDatabaseTest
 public class RefreshDnsForAllDomainsActionTest
     extends MapreduceTestCase<RefreshDnsForAllDomainsAction> {
 
-  @RegisterExtension public final InjectExtension inject = new InjectExtension();
+  private final FakeClock clock = new FakeClock(DateTime.parse("2020-02-02T02:02:02Z"));
-
   private final DnsQueue dnsQueue = mock(DnsQueue.class);
   private DnsQueue origDnsQueue;
+  private FakeResponse response = new FakeResponse();
+
+  @Order(Order.DEFAULT - 1)
+  @RegisterExtension
+  public final InjectExtension inject =
+      new InjectExtension().withStaticFieldOverride(Ofy.class, "clock", clock);
 
   @BeforeEach
   void beforeEach() {
@@ -60,6 +75,9 @@ public class RefreshDnsForAllDomainsActionTest
     action.random.setSeed(123L);
     action.mrRunner = makeDefaultRunner();
     action.response = new FakeResponse();
+    action.clock = clock;
+    action.dnsQueue = dnsQueue;
+    action.response = response;
 
     createTld("bar");
   }
@@ -70,58 +88,74 @@ public class RefreshDnsForAllDomainsActionTest
         .isEqualTo(dnsQueue);
   }
 
-  private void runMapreduce() throws Exception {
+  private void runAction() throws Exception {
     action.run();
     executeTasksUntilEmpty("mapreduce");
   }
 
-  @Test
+  @TestSqlOnly
+  void test_runAction_errorEnqueuingToDnsQueue() throws Exception {
+    persistActiveDomain("foo.bar");
+    persistActiveDomain("baz.bar");
+    persistActiveDomain("low.bar");
+    action.tlds = ImmutableSet.of("bar");
+    DnsQueue faultyQueue = spy(origDnsQueue);
+    doThrow(new RuntimeException("Error enqueuing task."))
+        .when(faultyQueue)
+        .addDomainRefreshTask(eq("baz.bar"), any(Duration.class));
+    action.dnsQueue = faultyQueue;
+    runAction();
+    assertDnsTasksEnqueued("foo.bar", "low.bar");
+    assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_INTERNAL_SERVER_ERROR);
+  }
+
+  @TestOfyAndSql
   void test_runAction_successfullyEnqueuesDnsRefreshes() throws Exception {
     persistActiveDomain("foo.bar");
     persistActiveDomain("low.bar");
     action.tlds = ImmutableSet.of("bar");
-    runMapreduce();
+    runAction();
     verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO);
     verify(dnsQueue).addDomainRefreshTask("low.bar", Duration.ZERO);
   }
 
-  @Test
+  @TestOfyAndSql
   void test_runAction_smearsOutDnsRefreshes() throws Exception {
     persistActiveDomain("foo.bar");
     persistActiveDomain("low.bar");
     action.tlds = ImmutableSet.of("bar");
     action.smearMinutes = 1000;
-    runMapreduce();
+    runAction();
     ArgumentCaptor<Duration> captor = ArgumentCaptor.forClass(Duration.class);
     verify(dnsQueue).addDomainRefreshTask(eq("foo.bar"), captor.capture());
     verify(dnsQueue).addDomainRefreshTask(eq("low.bar"), captor.capture());
     assertThat(captor.getAllValues()).containsExactly(standardMinutes(450), standardMinutes(782));
   }
 
-  @Test
+  @TestOfyAndSql
   void test_runAction_doesntRefreshDeletedDomain() throws Exception {
     persistActiveDomain("foo.bar");
-    persistDeletedDomain("deleted.bar", DateTime.now(UTC).minusYears(1));
+    persistDeletedDomain("deleted.bar", clock.nowUtc().minusYears(1));
     action.tlds = ImmutableSet.of("bar");
-    runMapreduce();
+    runAction();
     verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO);
     verify(dnsQueue, never()).addDomainRefreshTask("deleted.bar", Duration.ZERO);
   }
 
-  @Test
+  @TestOfyAndSql
   void test_runAction_ignoresDomainsOnOtherTlds() throws Exception {
     createTld("baz");
     persistActiveDomain("foo.bar");
     persistActiveDomain("low.bar");
     persistActiveDomain("ignore.baz");
     action.tlds = ImmutableSet.of("bar");
-    runMapreduce();
+    runAction();
     verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO);
     verify(dnsQueue).addDomainRefreshTask("low.bar", Duration.ZERO);
     verify(dnsQueue, never()).addDomainRefreshTask("ignore.baz", Duration.ZERO);
   }
 
-  @Test
+  @TestOfyAndSql
   void test_smearMinutesMustBeSpecified() {
     action.tlds = ImmutableSet.of("bar");
     action.smearMinutes = 0;