From e2f8c0565d0609ee61711168e8123a6f61ae7a95 Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Thu, 9 Jul 2026 17:14:45 -0400 Subject: [PATCH] Use cache for RDAP searches for host by superord domain this allows us to only do one query instead of looping over the hosts and doing queries one by one, while still leveraging the cache. --- .../registry/model/ForeignKeyUtils.java | 21 ++++++++++++ .../registry/rdap/RdapDomainSearchAction.java | 7 ++-- .../rdap/RdapNameserverSearchAction.java | 27 ++++++++++----- .../registry/model/ForeignKeyUtilsTest.java | 13 +++++++ .../rdap/RdapNameserverActionTest.java | 8 +++-- .../rdap/RdapNameserverSearchActionTest.java | 34 ++++++++++++++++--- 6 files changed, 91 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/google/registry/model/ForeignKeyUtils.java b/core/src/main/java/google/registry/model/ForeignKeyUtils.java index 9b7169912eb..ece9a743e7c 100644 --- a/core/src/main/java/google/registry/model/ForeignKeyUtils.java +++ b/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 Optional 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. + * + *

This method ignores the config setting for caching, and is reserved for use cases that can + * tolerate slightly stale data. + */ + @SuppressWarnings("unchecked") + public static ImmutableMap loadResourcesByCache( + Class clazz, Collection foreignKeys, Instant now) { + ImmutableSet> 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))); + } } diff --git a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java index b56df0faaed..260867675d7 100644 --- a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/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 domainSetBuilder = ImmutableSortedSet.orderedBy(Comparator.comparing(Domain::getDomainName)); int numHostKeysSearched = 0; @@ -465,7 +464,7 @@ private DomainSearchResponse searchByNameserverRefs( replicaTm() .transact( () -> { - for (VKey hostKey : hostKeys) { + for (VKey hostKey : chunk) { CriteriaQueryBuilder queryBuilder = CriteriaQueryBuilder.create(replicaTm(), Domain.class) .whereFieldContains("nsHosts", hostKey) diff --git a/core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java b/core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java index 5a15107767a..1444d15963e 100644 --- a/core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java @@ -185,7 +185,7 @@ private NameserverSearchResponse searchByNameUsingSuperordinateDomain( throw new UnprocessableEntityException( "A suffix after a wildcard in a nameserver lookup must be an in-bailiwick domain"); } - List hostList = new ArrayList<>(); + List matchingFqhns = new ArrayList<>(); for (String fqhn : ImmutableSortedSet.copyOf(domain.get().getSubordinateHosts())) { if (cursorString.isPresent() && (fqhn.compareTo(cursorString.get()) <= 0)) { continue; @@ -193,10 +193,22 @@ private NameserverSearchResponse searchByNameUsingSuperordinateDomain( // 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 = - ForeignKeyUtils.loadResourceByCache(Host.class, fqhn, getRequestTime()); - if (shouldBeVisible(host)) { - hostList.add(host.get()); + matchingFqhns.add(fqhn); + } + } + List hostList = new ArrayList<>(); + int chunkSize = getStandardQuerySizeLimit(); + // Batch load from cache in chunks to avoid sequential N+1 database queries on cache misses. + for (List fqhnChunk : Iterables.partition(matchingFqhns, chunkSize)) { + if (hostList.size() > rdapResultSetMaxSize) { + break; + } + ImmutableMap 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; } @@ -204,10 +216,7 @@ private NameserverSearchResponse searchByNameUsingSuperordinateDomain( } } return makeSearchResults( - hostList, - IncompletenessWarningType.COMPLETE, - domain.get().getSubordinateHosts().size(), - CursorType.NAME); + hostList, IncompletenessWarningType.COMPLETE, hostList.size(), CursorType.NAME); } /** diff --git a/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java b/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java index b6125046803..bf0a62b81cc 100644 --- a/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java +++ b/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)); + } } diff --git a/core/src/test/java/google/registry/rdap/RdapNameserverActionTest.java b/core/src/test/java/google/registry/rdap/RdapNameserverActionTest.java index 25570e65887..1f5231df114 100644 --- a/core/src/test/java/google/registry/rdap/RdapNameserverActionTest.java +++ b/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); diff --git a/core/src/test/java/google/registry/rdap/RdapNameserverSearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapNameserverSearchActionTest.java index 12d642e4004..20688dc82f0 100644 --- a/core/src/test/java/google/registry/rdap/RdapNameserverSearchActionTest.java +++ b/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