Update ListDomainsAction to SQL (#1036)

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

@@ -18,6 +18,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 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.Action.Method.GET;
 import static google.registry.request.Action.Method.POST;
 import static google.registry.request.RequestParameters.PARAM_TLDS;
@@ -74,34 +76,50 @@ public final class ListDomainsAction extends ListObjectsAction<DomainBase> {
   public ImmutableSet<DomainBase> loadObjects() {
     checkArgument(!tlds.isEmpty(), "Must specify TLDs to query");
     assertTldsExist(tlds);
+    ImmutableList<DomainBase> domains = tm().isOfy() ? loadDomainsOfy() : loadDomainsSql();
+    return ImmutableSet.copyOf(domains.reverse());
+  }
+
+  private ImmutableList<DomainBase> loadDomainsOfy() {
     DateTime now = clock.nowUtc();
     ImmutableList.Builder<DomainBase> domainsBuilder = new ImmutableList.Builder<>();
-    for (List<String> tldsBatch : Lists.partition(tlds.asList(), maxNumSubqueries)) {
-      domainsBuilder.addAll(
-          ofy()
-              .load()
-              .type(DomainBase.class)
-              .filter("tld in", tldsBatch)
-              // Get the N most recently created domains (requires ordering in descending order).
-              .order("-creationTime")
-              .limit(limit)
-              .list()
-              .stream()
-              .map(EppResourceUtils.transformAtTime(now))
-              // Deleted entities must be filtered out post-query because queries don't allow
-              // ordering with two filters.
-              .filter(d -> d.getDeletionTime().isAfter(now))
-              .collect(toImmutableList()));
-    }
     // Combine the batches together by sorting all domains together with newest first, applying the
     // limit, and then reversing for display order.
-    return ImmutableSet.copyOf(
-        domainsBuilder
-            .build()
-            .stream()
-            .sorted(comparing(EppResource::getCreationTime).reversed())
-            .limit(limit)
-            .collect(toImmutableList())
-            .reverse());
+    for (List<String> tldsBatch : Lists.partition(tlds.asList(), maxNumSubqueries)) {
+      ofy()
+          .load()
+          .type(DomainBase.class)
+          .filter("tld in", tldsBatch)
+          // Get the N most recently created domains (requires ordering in descending order).
+          .order("-creationTime")
+          .limit(limit)
+          .list()
+          .stream()
+          .map(EppResourceUtils.transformAtTime(now))
+          // Deleted entities must be filtered out post-query because queries don't allow
+          // ordering with two filters.
+          .filter(d -> d.getDeletionTime().isAfter(now))
+          .forEach(domainsBuilder::add);
+    }
+    return domainsBuilder.build().stream()
+        .sorted(comparing(EppResource::getCreationTime).reversed())
+        .limit(limit)
+        .collect(toImmutableList());
+  }
+
+  private ImmutableList<DomainBase> loadDomainsSql() {
+    return jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                    .query(
+                        "FROM Domain WHERE tld IN (:tlds) AND deletionTime > "
+                            + "current_timestamp() ORDER BY creationTime DESC",
+                        DomainBase.class)
+                    .setParameter("tlds", tlds)
+                    .setMaxResults(limit)
+                    .getResultStream()
+                    .map(EppResourceUtils.transformAtTime(jpaTm().getTransactionTime()))
+                    .collect(toImmutableList()));
   }
 }

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

@@ -19,13 +19,15 @@ import static google.registry.testing.DatabaseHelper.createTlds;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
 
 import com.google.common.collect.ImmutableSet;
+import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.FakeClock;
+import google.registry.testing.TestOfyAndSql;
 import java.util.Optional;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ListDomainsAction}. */
+@DualDatabaseTest
 class ListDomainsActionTest extends ListActionTestCase {
 
   private ListDomainsAction action;
@@ -38,7 +40,7 @@ class ListDomainsActionTest extends ListActionTestCase {
     action.limit = Integer.MAX_VALUE;
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_invalidRequest_missingTlds() {
     action.tlds = ImmutableSet.of();
     testRunError(
@@ -49,7 +51,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^Must specify TLDs to query$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_invalidRequest_invalidTld() {
     action.tlds = ImmutableSet.of("%%%badtld%%%");
     testRunError(
@@ -60,13 +62,13 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^TLDs do not exist: %%%badtld%%%$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_noParameters() {
     action.tlds = ImmutableSet.of("foo");
     testRunSuccess(action, null, null, null);
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithIdOnly() {
     action.tlds = ImmutableSet.of("foo");
     createTlds("bar", "sim");
@@ -84,7 +86,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example2.foo$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_multipleTlds() {
     action.tlds = ImmutableSet.of("bar", "foo");
     createTlds("bar", "sim");
@@ -102,7 +104,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example2.foo$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_moreTldsThanMaxNumSubqueries() {
     ListDomainsAction.maxNumSubqueries = 2;
     createTlds("baa", "bab", "bac", "bad");
@@ -125,7 +127,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^domain3.bac$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithIdOnlyNoHeader() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -139,7 +141,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example2.foo$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithIdOnlyExplicitHeader() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -155,7 +157,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example2.foo\\s*$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithRepoId() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -171,7 +173,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example3.foo\\s+4-FOO\\s*$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithRepoIdNoHeader() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -185,7 +187,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example3.foo  4-FOO$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithRepoIdExplicitHeader() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -201,7 +203,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example3.foo\\s+4-FOO\\s*$");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithWildcard() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -217,7 +219,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example3.foo\\s+.*4-FOO");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_twoLinesWithWildcardAndAnotherField() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example1.foo", DateTime.parse("2010-03-04T16:00:00Z"));
@@ -233,7 +235,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^example3.foo\\s+.*4-FOO");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_withBadField_returnsError() {
     action.tlds = ImmutableSet.of("foo");
     persistActiveDomain("example2.foo");
@@ -246,7 +248,7 @@ class ListDomainsActionTest extends ListActionTestCase {
         "^Field 'badfield' not found - recognized fields are:");
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_limitFiltersOutOldestDomains() {
     createTlds("bar", "baz");
     action.tlds = ImmutableSet.of("foo", "bar");