Use cache for RDAP searches for host by superord domain

core/src/main/java/google/registry/model/ForeignKeyUtils.java

@@ -16,6 +16,7 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.config.RegistryConfig.getEppResourceCachingDuration;
 import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntries;
 import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
@@ -400,4 +401,24 @@ public static <E extends EppResource> Optional<E> loadResourceByCache(
             .filter(e -> now.isBefore(e.getDeletionTime()))
             .map(e -> e.cloneProjectedAtTime(now));
   }
+
+  /**
+   * Loads the last created version of multiple {@link EppResource}s from the replica database by
+   * foreign keys, using a cache.
+   *
+   * <p>This method ignores the config setting for caching, and is reserved for use cases that can
+   * tolerate slightly stale data.
+   */
+  @SuppressWarnings("unchecked")
+  public static <E extends EppResource> ImmutableMap<String, E> loadResourcesByCache(
+      Class<E> clazz, Collection<String> foreignKeys, Instant now) {
+    ImmutableSet<VKey<? extends EppResource>> vkeys =
+        foreignKeys.stream().map(fk -> VKey.create(clazz, fk)).collect(toImmutableSet());
+    return foreignKeyToResourceCache.getAll(vkeys).entrySet().stream()
+        .filter(e -> e.getValue().isPresent() && now.isBefore(e.getValue().get().getDeletionTime()))
+        .collect(
+            toImmutableMap(
+                e -> (String) e.getKey().getKey(),
+                e -> (E) e.getValue().get().cloneProjectedAtTime(now)));
+  }
 }

core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java

@@ -454,9 +454,8 @@ private DomainSearchResponse searchByNameserverRefs(
     // We must break the query up into chunks, because the in operator is limited to 30 subqueries.
     // Since it is possible for the same domain to show up more than once in our result list (if
     // we do a wildcard nameserver search that returns multiple nameservers used by the same
-    // domain), we must create a set of resulting {@link Domain} objects. Use a sorted set,
-    // and fetch all domains, to make sure that we can return the first domains in alphabetical
-    // order.
+    // domain), we must create a set of resulting {@link Domain}s. Use a sorted set, fetch all
+    // domains, to make sure that we can return the first domains in alphabetical order.
     ImmutableSortedSet.Builder<Domain> domainSetBuilder =
         ImmutableSortedSet.orderedBy(Comparator.comparing(Domain::getDomainName));
     int numHostKeysSearched = 0;
@@ -465,7 +464,7 @@ private DomainSearchResponse searchByNameserverRefs(
       replicaTm()
           .transact(
               () -> {
-                for (VKey<Host> hostKey : hostKeys) {
+                for (VKey<Host> hostKey : chunk) {
                   CriteriaQueryBuilder<Domain> queryBuilder =
                       CriteriaQueryBuilder.create(replicaTm(), Domain.class)
                           .whereFieldContains("nsHosts", hostKey)

core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java

@@ -185,29 +185,38 @@ private NameserverSearchResponse searchByNameUsingSuperordinateDomain(
       throw new UnprocessableEntityException(
           "A suffix after a wildcard in a nameserver lookup must be an in-bailiwick domain");
     }
-    List<Host> hostList = new ArrayList<>();
+    List<String> matchingFqhns = new ArrayList<>();
     for (String fqhn : ImmutableSortedSet.copyOf(domain.get().getSubordinateHosts())) {
       if (cursorString.isPresent() && (fqhn.compareTo(cursorString.get()) <= 0)) {
         continue;
       }
       // We can't just check that the host name starts with the initial query string, because
       // then the query ns.exam*.example.com would match against nameserver ns.example.com.
       if (partialStringQuery.matches(fqhn)) {
-        Optional<Host> host =
-            ForeignKeyUtils.loadResourceByCache(Host.class, fqhn, getRequestTime());
-        if (shouldBeVisible(host)) {
-          hostList.add(host.get());
+        matchingFqhns.add(fqhn);
+      }
+    }
+    List<Host> hostList = new ArrayList<>();
+    int chunkSize = getStandardQuerySizeLimit();
+    // Batch load from cache in chunks to avoid sequential N+1 database queries on cache misses.
+    for (List<String> fqhnChunk : Iterables.partition(matchingFqhns, chunkSize)) {
+      if (hostList.size() > rdapResultSetMaxSize) {
+        break;
+      }
+      ImmutableMap<String, Host> cachedHosts =
+          ForeignKeyUtils.loadResourcesByCache(Host.class, fqhnChunk, getRequestTime());
+      for (String fqhn : fqhnChunk) {
+        Host host = cachedHosts.get(fqhn);
+        if (host != null && shouldBeVisible(host)) {
+          hostList.add(host);
           if (hostList.size() > rdapResultSetMaxSize) {
             break;
           }
         }
       }
     }
     return makeSearchResults(
-        hostList,
-        IncompletenessWarningType.COMPLETE,
-        domain.get().getSubordinateHosts().size(),
-        CursorType.NAME);
+        hostList, IncompletenessWarningType.COMPLETE, hostList.size(), CursorType.NAME);
   }
 
   /**

core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java

@@ -144,4 +144,17 @@ void testSuccess_loadHostKeysCached_cacheIsStale() {
                 fakeClock.now()))
         .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", host1.createVKey()));
   }
+
+  @Test
+  void testSuccess_loadResourcesByCache_skipsDeletedAndNonexistent() {
+    Host host1 = persistActiveHost("ns1.example.com");
+    Host host2 = persistActiveHost("ns2.example.com");
+    persistResource(host2.asBuilder().setDeletionTime(minusDays(fakeClock.now(), 1)).build());
+    assertThat(
+            ForeignKeyUtils.loadResourcesByCache(
+                Host.class,
+                ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"),
+                fakeClock.now()))
+        .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", host1));
+  }
 }

core/src/test/java/google/registry/rdap/RdapNameserverActionTest.java

@@ -25,6 +25,7 @@
 import static google.registry.util.DateTimeUtils.minusYears;
 import static org.mockito.Mockito.verify;
 
+import google.registry.model.host.Host;
 import google.registry.model.registrar.Registrar;
 import google.registry.rdap.RdapMetrics.EndpointType;
 import google.registry.rdap.RdapMetrics.SearchType;
@@ -111,13 +112,16 @@ void testValidNameserver_works() {
   @Test
   void testNameserver_tldTithHyphenOn3And4_works() {
     createTld("zz--main-2166");
-    persistResource(makePunycodedHost("ns1.cat.zz--main-2166", "1.2.3.4", null, "TheRegistrar"));
+    Host host =
+        persistResource(
+            makePunycodedHost("ns1.cat.zz--main-2166", "1.2.3.4", null, "TheRegistrar"));
     assertAboutJson()
         .that(generateActualJson("ns1.cat.zz--main-2166"))
         .isEqualTo(
             addPermanentBoilerplateNotices(
                 jsonFileBuilder()
-                    .addNameserver("ns1.cat.zz--main-2166", "ns1.cat.zz--main-2166", "F-ROID")
+                    .addNameserver(
+                        "ns1.cat.zz--main-2166", "ns1.cat.zz--main-2166", host.getRepoId())
                     .putAll("ADDRESSTYPE", "v4", "ADDRESS", "1.2.3.4", "STATUS", "active")
                     .load("rdap_host.json")));
     assertThat(response.getStatus()).isEqualTo(200);

core/src/test/java/google/registry/rdap/RdapNameserverSearchActionTest.java

@@ -29,6 +29,7 @@
 import static google.registry.util.DateTimeUtils.minusMonths;
 import static google.registry.util.DateTimeUtils.minusYears;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.mockito.Mockito.clearInvocations;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
@@ -46,6 +47,7 @@
 import google.registry.testing.FakeResponse;
 import google.registry.testing.FullFieldsTestEntityHelper;
 import java.net.URLDecoder;
+import java.time.Instant;
 import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -428,7 +430,7 @@ void testNameMatch_nsstar_cat_lol_notFound_differentRegistrarRequested() {
     action.registrarParam = Optional.of("unicoderegistrar");
     generateActualJsonWithName("ns*.cat.lol");
     assertThat(response.getStatus()).isEqualTo(404);
-    verifyErrorMetrics(Optional.of(2L), 404);
+    verifyErrorMetrics(Optional.of(0L), 404);
   }
 
   @Test
@@ -445,6 +447,30 @@ void testNameMatch_star_cat_lol_found() {
     verifyMetrics(2);
   }
 
+  @Test
+  void testNameMatch_star_cat_lol_usesForeignKeyCache() {
+    generateActualJsonWithName("*.cat.lol");
+    assertThat(response.getStatus()).isEqualTo(200);
+    verifyMetrics(2);
+    clearInvocations(rdapMetrics);
+
+    Instant newTransferTime = clock.now();
+    persistResource(hostNs1CatLol.asBuilder().setLastTransferTime(newTransferTime).build());
+    clock.advanceOneMilli();
+
+    action.response = new FakeResponse();
+    generateActualJsonWithName("*.cat.lol");
+    assertThat(response.getStatus()).isEqualTo(200);
+    verifyMetrics(2);
+
+    JsonObject searchResults =
+        parseJsonObject(response.getPayload())
+            .getAsJsonArray("nameserverSearchResults")
+            .get(0)
+            .getAsJsonObject();
+    assertThat(searchResults.toString()).doesNotContain(newTransferTime.toString());
+  }
+
   @Test
   void testNameMatch_star_cat_lol_found_sameRegistrarRequested() {
     action.registrarParam = Optional.of("TheRegistrar");
@@ -458,7 +484,7 @@ void testNameMatch_star_cat_lol_notFound_differentRegistrarRequested() {
     action.registrarParam = Optional.of("unicoderegistrar");
     generateActualJsonWithName("*.cat.lol");
     assertThat(response.getStatus()).isEqualTo(404);
-    verifyErrorMetrics(Optional.of(2L), 404);
+    verifyErrorMetrics(Optional.of(0L), 404);
   }
 
   @Test
@@ -521,7 +547,7 @@ void testNameMatch_reallyTruncatedResultSet() {
                 "rdap_truncated_hosts.json", "QUERY", "name=nsx*.cat.lol&cursor=bnN4NC5jYXQubG9s"));
     assertThat(response.getStatus()).isEqualTo(200);
     // When searching names, we look for additional matches, in case some are not visible.
-    verifyMetrics(9, IncompletenessWarningType.TRUNCATED);
+    verifyMetrics(5, IncompletenessWarningType.TRUNCATED);
   }
 
   @Test
@@ -536,7 +562,7 @@ void testNameMatchDeletedHost_foundTheOtherHost() {
                     .putAll("ADDRESSTYPE", "v6", "ADDRESS", "bad:f00d:cafe::15:beef")
                     .load("rdap_host_linked.json")));
     assertThat(response.getStatus()).isEqualTo(200);
-    verifyMetrics(2);
+    verifyMetrics(1);
   }
 
   @Test