diff --git a/core/src/main/java/google/registry/model/ForeignKeyUtils.java b/core/src/main/java/google/registry/model/ForeignKeyUtils.java index 9b7169912..ece9a743e 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 @@ package google.registry.model; 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 final class ForeignKeyUtils { .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 b56df0faa..260867675 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 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // 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 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { 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 5a1510776..1444d1596 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 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { 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 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { // 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 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { } } 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 b61250468..bf0a62b81 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 @@ class ForeignKeyUtilsTest { 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 25570e658..1f5231df1 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.minusMonths; 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 @@ class RdapNameserverActionTest extends RdapActionBaseTestCase