From 74873f90c8bdcd1c795180d2badef8645e4e7da8 Mon Sep 17 00:00:00 2001
From: mountford <mountford@google.com>
Date: Thu, 26 Oct 2017 11:05:32 -0700
Subject: [PATCH] Order RDAP domain searches by TLD in domain name order

I am not happy that another index is required, but the Pantheon console shows that domain indexes are much smaller than the other indexes (because there are fewer domains), so it's not adding an appreciable amount of storage space.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173561771
---
 .../default/WEB-INF/datastore-indexes.xml     |  6 ++++++
 .../registry/rdap/RdapDomainSearchAction.java | 16 ++++++++++-----
 .../rdap/RdapDomainSearchActionTest.java      | 20 +++++++++++++++++--
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml b/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml
index 922f1a822..abb5d595a 100644
--- a/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml
+++ b/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml
@@ -77,6 +77,12 @@
         <property name="currentSponsorClientId" direction="asc"/>
         <property name="fullyQualifiedDomainName" direction="asc"/>
     </datastore-index>
+    <datastore-index kind="DomainBase" ancestor="false" source="manual">
+        <property name="^i" direction="asc"/>
+        <property name="currentSponsorClientId" direction="asc"/>
+        <property name="tld" direction="asc"/>
+        <property name="fullyQualifiedDomainName" direction="asc"/>
+    </datastore-index>
     <datastore-index kind="DomainBase" ancestor="false" source="manual">
         <property name="^i" direction="asc"/>
         <property name="tld" direction="asc"/>
diff --git a/java/google/registry/rdap/RdapDomainSearchAction.java b/java/google/registry/rdap/RdapDomainSearchAction.java
index 677d8c0fd..40ad7d96f 100644
--- a/java/google/registry/rdap/RdapDomainSearchAction.java
+++ b/java/google/registry/rdap/RdapDomainSearchAction.java
@@ -238,12 +238,18 @@ public class RdapDomainSearchAction extends RdapActionBase {
 
   /** Searches for domains by domain name with a TLD suffix. */
   private RdapSearchResults searchByDomainNameByTld(String tld, DateTime now) {
-    // Since we aren't searching on fullyQualifiedDomainName, we can perform our one allowed
-    // inequality query on deletion time.
+    // Even though we are not searching on fullyQualifiedDomainName, we want the results to come
+    // back ordered by name, so we are still in the same boat as
+    // searchByDomainNameWithInitialString, unable to perform an inequality query on deletion time.
+    // Don't use queryItems, because it doesn't handle pending deletes.
     Query<DomainResource> query =
-        queryItems(
-            DomainResource.class, "tld", tld, shouldIncludeDeleted(), rdapResultSetMaxSize + 1);
-    return makeSearchResults(getMatchingResources(query, shouldIncludeDeleted(), now), now);
+        ofy()
+            .load()
+            .type(DomainResource.class)
+            .filter("tld", tld)
+            .order("fullyQualifiedDomainName")
+            .limit(RESULT_SET_SIZE_SCALING_FACTOR * rdapResultSetMaxSize);
+    return makeSearchResults(getMatchingResources(query, true, now), now);
   }
 
   /**
diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java
index a03fc5e3d..6c4e4b33f 100644
--- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java
+++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java
@@ -855,7 +855,7 @@ public class RdapDomainSearchActionTest {
   @Test
   public void testDomainMatch_star_lol_found() throws Exception {
     assertThat(generateActualJson(RequestType.NAME, "*.lol"))
-        .isEqualTo(generateExpectedJsonForTwoDomains("cat2.lol", "17-LOL", "cat.lol", "C-LOL"));
+        .isEqualTo(generateExpectedJsonForTwoDomains("cat.lol", "C-LOL", "cat2.lol", "17-LOL"));
     assertThat(response.getStatus()).isEqualTo(200);
   }
 
@@ -863,7 +863,7 @@ public class RdapDomainSearchActionTest {
   public void testDomainMatch_star_lol_found_sameRegistrarRequested() throws Exception {
     action.registrarParam = Optional.of("evilregistrar");
     assertThat(generateActualJson(RequestType.NAME, "*.lol"))
-        .isEqualTo(generateExpectedJsonForTwoDomains("cat2.lol", "17-LOL", "cat.lol", "C-LOL"));
+        .isEqualTo(generateExpectedJsonForTwoDomains("cat.lol", "C-LOL", "cat2.lol", "17-LOL"));
     assertThat(response.getStatus()).isEqualTo(200);
   }
 
@@ -1056,6 +1056,22 @@ public class RdapDomainSearchActionTest {
         "rdap_domains_four_truncated.json");
   }
 
+  @Test
+  public void testDomainMatch_tldSearchOrderedProperly() throws Exception {
+    createManyDomainsAndHosts(4, 1, 2);
+    assertThat(generateActualJson(RequestType.NAME, "*.lol"))
+        .isEqualTo(readMultiDomainFile(
+            "rdap_domains_four_truncated.json",
+            "cat.lol",
+            "C-LOL",
+            "cat2.lol",
+            "17-LOL",
+            "domain1.lol",
+            "46-LOL",
+            "domain2.lol",
+            "45-LOL"));
+  }
+
   @Test
   public void testDomainMatch_reallyTruncatedResultsSet() throws Exception {
     // Don't use 10 or more domains for this test, because domain10.lol will come before