mirror of
https://github.com/google/nomulus
synced 2026-07-18 22:12:24 +00:00
Use cache for RDAP searches for host by superord domain (#3145)
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.
This commit is contained in:
@@ -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.
|
||||
*
|
||||
* <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)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Domain> domainSetBuilder =
|
||||
ImmutableSortedSet.orderedBy(Comparator.comparing(Domain::getDomainName));
|
||||
int numHostKeysSearched = 0;
|
||||
@@ -465,7 +464,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
|
||||
replicaTm()
|
||||
.transact(
|
||||
() -> {
|
||||
for (VKey<Host> hostKey : hostKeys) {
|
||||
for (VKey<Host> hostKey : chunk) {
|
||||
CriteriaQueryBuilder<Domain> queryBuilder =
|
||||
CriteriaQueryBuilder.create(replicaTm(), Domain.class)
|
||||
.whereFieldContains("nsHosts", hostKey)
|
||||
|
||||
@@ -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<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;
|
||||
@@ -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> 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;
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<RdapNameserverActi
|
||||
@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);
|
||||
|
||||
@@ -29,6 +29,7 @@ import static google.registry.util.DateTimeUtils.minusDays;
|
||||
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.rdap.RdapSearchResults.IncompletenessWarningType;
|
||||
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 @@ class RdapNameserverSearchActionTest extends RdapSearchActionTestCase<RdapNamese
|
||||
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 @@ class RdapNameserverSearchActionTest extends RdapSearchActionTestCase<RdapNamese
|
||||
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 @@ class RdapNameserverSearchActionTest extends RdapSearchActionTestCase<RdapNamese
|
||||
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 @@ class RdapNameserverSearchActionTest extends RdapSearchActionTestCase<RdapNamese
|
||||
"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 @@ class RdapNameserverSearchActionTest extends RdapSearchActionTestCase<RdapNamese
|
||||
.putAll("ADDRESSTYPE", "v6", "ADDRESS", "bad:f00d:cafe::15:beef")
|
||||
.load("rdap_host_linked.json")));
|
||||
assertThat(response.getStatus()).isEqualTo(200);
|
||||
verifyMetrics(2);
|
||||
verifyMetrics(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user