diff --git a/core/build.gradle b/core/build.gradle index d2755d8b6..19c9a1631 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -76,6 +76,9 @@ def fragileTestPatterns = [ "google/registry/model/tmch/ClaimsListShardTest.*", // Creates large object (64MBytes), occasionally throws OOM error. "google/registry/model/server/KmsSecretRevisionTest.*", + // Changes cache timeouts and for some reason appears to have contention + // with other tests. + "google/registry/whois/WhoisCommandFactoryTest.*", ] + dockerIncompatibleTestPatterns sourceSets { diff --git a/core/src/main/java/google/registry/tools/RegistryToolComponent.java b/core/src/main/java/google/registry/tools/RegistryToolComponent.java index cb68eb7be..31c0d6895 100644 --- a/core/src/main/java/google/registry/tools/RegistryToolComponent.java +++ b/core/src/main/java/google/registry/tools/RegistryToolComponent.java @@ -43,7 +43,7 @@ import google.registry.request.Modules.UrlFetchTransportModule; import google.registry.request.Modules.UserServiceModule; import google.registry.tools.AuthModule.LocalCredentialModule; import google.registry.util.UtilsModule; -import google.registry.whois.WhoisModule; +import google.registry.whois.NonCachingWhoisModule; import javax.annotation.Nullable; import javax.inject.Singleton; @@ -81,7 +81,7 @@ import javax.inject.Singleton; UserServiceModule.class, UtilsModule.class, VoidDnsWriterModule.class, - WhoisModule.class + NonCachingWhoisModule.class }) interface RegistryToolComponent { void inject(AckPollMessagesCommand command); diff --git a/core/src/main/java/google/registry/whois/BaseWhoisModule.java b/core/src/main/java/google/registry/whois/BaseWhoisModule.java new file mode 100644 index 000000000..aee86b24c --- /dev/null +++ b/core/src/main/java/google/registry/whois/BaseWhoisModule.java @@ -0,0 +1,52 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.whois; + +import dagger.Module; +import dagger.Provides; +import google.registry.util.Clock; +import google.registry.whois.WhoisMetrics.WhoisMetric; +import java.io.IOException; +import java.io.Reader; +import javax.servlet.http.HttpServletRequest; + +/** + * Dagger base module for the whois package. + * + *

Provides whois objects common to both the normal ({@link WhoisModule}) and non-caching ({@link + * NonCachingWhoisModule}) implementations. + */ +@Module +class BaseWhoisModule { + + @Provides + @SuppressWarnings("CloseableProvides") + static Reader provideHttpInputReader(HttpServletRequest req) { + try { + return req.getReader(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + /** + * Provides a {@link WhoisMetrics.WhoisMetric.Builder} with the startTimestamp already + * initialized. + */ + @Provides + static WhoisMetric.Builder provideEppMetricBuilder(Clock clock) { + return WhoisMetric.builderForRequest(clock); + } +} diff --git a/core/src/main/java/google/registry/whois/DomainLookupCommand.java b/core/src/main/java/google/registry/whois/DomainLookupCommand.java index 57e2a5680..789d0dc41 100644 --- a/core/src/main/java/google/registry/whois/DomainLookupCommand.java +++ b/core/src/main/java/google/registry/whois/DomainLookupCommand.java @@ -14,6 +14,7 @@ package google.registry.whois; +import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.EppResourceUtils.loadByForeignKeyCached; import com.google.common.net.InternetDomainName; @@ -25,20 +26,27 @@ import org.joda.time.DateTime; public class DomainLookupCommand extends DomainOrHostLookupCommand { private final boolean fullOutput; + private final boolean cached; private final String whoisRedactedEmailText; public DomainLookupCommand( InternetDomainName domainName, boolean fullOutput, + boolean cached, String whoisRedactedEmailText) { super(domainName, "Domain"); this.fullOutput = fullOutput; + this.cached = cached; this.whoisRedactedEmailText = whoisRedactedEmailText; } @Override protected Optional getResponse(InternetDomainName domainName, DateTime now) { - return loadByForeignKeyCached(DomainBase.class, domainName.toString(), now) - .map(domain -> new DomainWhoisResponse(domain, fullOutput, whoisRedactedEmailText, now)); + Optional domainResource = + cached + ? loadByForeignKeyCached(DomainBase.class, domainName.toString(), now) + : loadByForeignKey(DomainBase.class, domainName.toString(), now); + return domainResource.map( + domain -> new DomainWhoisResponse(domain, fullOutput, whoisRedactedEmailText, now)); } } diff --git a/core/src/main/java/google/registry/whois/NameserverLookupByHostCommand.java b/core/src/main/java/google/registry/whois/NameserverLookupByHostCommand.java index cad8a3d6b..3ca6432d3 100644 --- a/core/src/main/java/google/registry/whois/NameserverLookupByHostCommand.java +++ b/core/src/main/java/google/registry/whois/NameserverLookupByHostCommand.java @@ -14,6 +14,7 @@ package google.registry.whois; +import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.EppResourceUtils.loadByForeignKeyCached; import com.google.common.net.InternetDomainName; @@ -24,13 +25,19 @@ import org.joda.time.DateTime; /** Represents a WHOIS lookup on a nameserver based on its hostname. */ public class NameserverLookupByHostCommand extends DomainOrHostLookupCommand { - NameserverLookupByHostCommand(InternetDomainName hostName) { + boolean cached; + + NameserverLookupByHostCommand(InternetDomainName hostName, boolean cached) { super(hostName, "Nameserver"); + this.cached = cached; } @Override protected Optional getResponse(InternetDomainName hostName, DateTime now) { - return loadByForeignKeyCached(HostResource.class, hostName.toString(), now) - .map(host -> new NameserverWhoisResponse(host, now)); + Optional hostResource = + cached + ? loadByForeignKeyCached(HostResource.class, hostName.toString(), now) + : loadByForeignKey(HostResource.class, hostName.toString(), now); + return hostResource.map(host -> new NameserverWhoisResponse(host, now)); } } diff --git a/core/src/main/java/google/registry/whois/NonCachingWhoisModule.java b/core/src/main/java/google/registry/whois/NonCachingWhoisModule.java new file mode 100644 index 000000000..6e4285bc4 --- /dev/null +++ b/core/src/main/java/google/registry/whois/NonCachingWhoisModule.java @@ -0,0 +1,39 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.whois; + +import dagger.Module; +import dagger.Provides; +import google.registry.config.RegistryConfig.Config; + +/** + * Whois module for systems that require that we not cache EPP resources (e.g. the nomulus tool). + * + *

Dependencies

+ * + * + * + * @see "google.registry.tools.RegistryToolComponent" + */ +@Module +public final class NonCachingWhoisModule extends BaseWhoisModule { + @Provides + @Config("whoisCommandFactory") + static WhoisCommandFactory provideWhoisCommandFactory() { + return WhoisCommandFactory.createNonCached(); + } +} diff --git a/core/src/main/java/google/registry/whois/RegistrarLookupCommand.java b/core/src/main/java/google/registry/whois/RegistrarLookupCommand.java index e90456c6f..51d801afe 100644 --- a/core/src/main/java/google/registry/whois/RegistrarLookupCommand.java +++ b/core/src/main/java/google/registry/whois/RegistrarLookupCommand.java @@ -39,61 +39,66 @@ final class RegistrarLookupCommand implements WhoisCommand { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + /** True if the command should return cached responses. */ + private boolean cached; + /** * Cache of a map from a stripped-down (letters and digits only) name to the registrar. This map * includes only active, publicly visible registrars, because the others should be invisible to * WHOIS. */ private static final Supplier> REGISTRAR_BY_NORMALIZED_NAME_CACHE = - memoizeWithShortExpiration( - () -> { - Map map = new HashMap<>(); - // Use the normalized registrar name as a key, and ignore inactive and hidden - // registrars. - for (Registrar registrar : Registrar.loadAllCached()) { - if (!registrar.isLiveAndPubliclyVisible() || registrar.getRegistrarName() == null) { - continue; - } - String normalized = normalizeRegistrarName(registrar.getRegistrarName()); - if (map.put(normalized, registrar) != null) { - logger.atWarning().log( - "%s appeared as a normalized registrar name for more than one registrar.", - normalized); - } - } - // Use the normalized registrar name without its last word as a key, assuming there are - // multiple words in the name. This allows searches without LLC or INC, etc. Only insert - // if there isn't already a mapping for this string, so that if there's a registrar with - // a - // two word name (Go Daddy) and no business-type suffix and another registrar with just - // that first word as its name (Go), the latter will win. - for (Registrar registrar : ImmutableList.copyOf(map.values())) { - if (registrar.getRegistrarName() == null) { - continue; - } - List words = - Splitter.on(CharMatcher.whitespace()).splitToList(registrar.getRegistrarName()); - if (words.size() > 1) { - String normalized = - normalizeRegistrarName(Joiner.on("").join(words.subList(0, words.size() - 1))); - map.putIfAbsent(normalized, registrar); - } - } - return ImmutableMap.copyOf(map); - }); + memoizeWithShortExpiration(RegistrarLookupCommand::loadRegistrarMap); @VisibleForTesting final String registrarName; - RegistrarLookupCommand(String registrarName) { + RegistrarLookupCommand(String registrarName, boolean cached) { checkArgument(!isNullOrEmpty(registrarName), "registrarName"); this.registrarName = registrarName; + this.cached = cached; + } + + static Map loadRegistrarMap() { + Map map = new HashMap<>(); + // Use the normalized registrar name as a key, and ignore inactive and hidden + // registrars. + for (Registrar registrar : Registrar.loadAll()) { + if (!registrar.isLiveAndPubliclyVisible() || registrar.getRegistrarName() == null) { + continue; + } + String normalized = normalizeRegistrarName(registrar.getRegistrarName()); + if (map.put(normalized, registrar) != null) { + logger.atWarning().log( + "%s appeared as a normalized registrar name for more than one registrar.", normalized); + } + } + // Use the normalized registrar name without its last word as a key, assuming there are + // multiple words in the name. This allows searches without LLC or INC, etc. Only insert + // if there isn't already a mapping for this string, so that if there's a registrar with + // a + // two word name (Go Daddy) and no business-type suffix and another registrar with just + // that first word as its name (Go), the latter will win. + for (Registrar registrar : ImmutableList.copyOf(map.values())) { + if (registrar.getRegistrarName() == null) { + continue; + } + List words = + Splitter.on(CharMatcher.whitespace()).splitToList(registrar.getRegistrarName()); + if (words.size() > 1) { + String normalized = + normalizeRegistrarName(Joiner.on("").join(words.subList(0, words.size() - 1))); + map.putIfAbsent(normalized, registrar); + } + } + return ImmutableMap.copyOf(map); } @Override public WhoisResponse executeQuery(DateTime now) throws WhoisException { - Registrar registrar = - REGISTRAR_BY_NORMALIZED_NAME_CACHE.get().get(normalizeRegistrarName(registrarName)); + Map registrars = + cached ? REGISTRAR_BY_NORMALIZED_NAME_CACHE.get() : loadRegistrarMap(); + Registrar registrar = registrars.get(normalizeRegistrarName(registrarName)); // If a registrar is in the cache, we know it must be active and publicly visible. if (registrar == null) { throw new WhoisException(now, SC_NOT_FOUND, "No registrar found."); diff --git a/core/src/main/java/google/registry/whois/WhoisCommandFactory.java b/core/src/main/java/google/registry/whois/WhoisCommandFactory.java index f511a72e3..1ee498eb1 100644 --- a/core/src/main/java/google/registry/whois/WhoisCommandFactory.java +++ b/core/src/main/java/google/registry/whois/WhoisCommandFactory.java @@ -26,10 +26,30 @@ import java.net.InetAddress; */ public class WhoisCommandFactory { + /** True if the commands should return cached responses. */ + private boolean cached = true; + + /** Public default constructor, needed so we can construct this from the class name. */ + public WhoisCommandFactory() {} + + private WhoisCommandFactory(boolean cached) { + this.cached = cached; + } + + /** Create a command factory that does not rely on entity caches. */ + static WhoisCommandFactory createNonCached() { + return new WhoisCommandFactory(false); + } + + /** Create a command factory that may rely on entity caches. */ + static WhoisCommandFactory createCached() { + return new WhoisCommandFactory(true); + } + /** Returns a new {@link WhoisCommand} to perform a domain lookup on the specified domain name. */ public WhoisCommand domainLookup( InternetDomainName domainName, boolean fullOutput, String whoisRedactedEmailText) { - return new DomainLookupCommand(domainName, fullOutput, whoisRedactedEmailText); + return new DomainLookupCommand(domainName, fullOutput, cached, whoisRedactedEmailText); } /** @@ -43,7 +63,7 @@ public class WhoisCommandFactory { * Returns a new {@link WhoisCommand} to perform a nameserver lookup on the specified host name. */ public WhoisCommand nameserverLookupByHost(InternetDomainName hostName) { - return new NameserverLookupByHostCommand(hostName); + return new NameserverLookupByHostCommand(hostName, cached); } /** @@ -51,6 +71,6 @@ public class WhoisCommandFactory { * name. */ public WhoisCommand registrarLookup(String registrar) { - return new RegistrarLookupCommand(registrar); + return new RegistrarLookupCommand(registrar, cached); } } diff --git a/core/src/main/java/google/registry/whois/WhoisModule.java b/core/src/main/java/google/registry/whois/WhoisModule.java index bcbd928d1..8836762ba 100644 --- a/core/src/main/java/google/registry/whois/WhoisModule.java +++ b/core/src/main/java/google/registry/whois/WhoisModule.java @@ -20,11 +20,6 @@ import static google.registry.util.TypeUtils.instantiate; import dagger.Module; import dagger.Provides; import google.registry.config.RegistryConfig.Config; -import google.registry.util.Clock; -import google.registry.whois.WhoisMetrics.WhoisMetric; -import java.io.IOException; -import java.io.Reader; -import javax.servlet.http.HttpServletRequest; /** * Dagger module for the whois package. @@ -38,31 +33,11 @@ import javax.servlet.http.HttpServletRequest; * @see "google.registry.module.frontend.FrontendComponent" */ @Module -public final class WhoisModule { - - @Provides - @SuppressWarnings("CloseableProvides") - static Reader provideHttpInputReader(HttpServletRequest req) { - try { - return req.getReader(); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - +public final class WhoisModule extends BaseWhoisModule { @Provides @Config("whoisCommandFactory") static WhoisCommandFactory provideWhoisCommandFactory( @Config("whoisCommandFactoryClass") String factoryClass) { return instantiate(getClassFromString(factoryClass, WhoisCommandFactory.class)); } - - /** - * Provides a {@link WhoisMetrics.WhoisMetric.Builder} with the startTimestamp already - * initialized. - */ - @Provides - static WhoisMetric.Builder provideEppMetricBuilder(Clock clock) { - return WhoisMetric.builderForRequest(clock); - } } diff --git a/core/src/main/java/google/registry/whois/WhoisReader.java b/core/src/main/java/google/registry/whois/WhoisReader.java index 218d60891..49038af24 100644 --- a/core/src/main/java/google/registry/whois/WhoisReader.java +++ b/core/src/main/java/google/registry/whois/WhoisReader.java @@ -194,7 +194,7 @@ class WhoisReader { if (!tld.isPresent()) { // This target is not under any configured TLD, so just try it as a registrar name. logger.atInfo().log("Attempting registrar lookup using %s as a registrar", arg1); - return new RegistrarLookupCommand(arg1); + return commandFactory.registrarLookup(arg1); } // If the target is exactly one level above the TLD, then this is a second level domain diff --git a/core/src/test/java/google/registry/whois/WhoisActionTest.java b/core/src/test/java/google/registry/whois/WhoisActionTest.java index 54181b306..ea57f6783 100644 --- a/core/src/test/java/google/registry/whois/WhoisActionTest.java +++ b/core/src/test/java/google/registry/whois/WhoisActionTest.java @@ -96,7 +96,7 @@ public class WhoisActionTest { whoisAction.input = new StringReader(input); whoisAction.response = response; whoisAction.whoisReader = - new WhoisReader(new WhoisCommandFactory(), "Please contact registrar"); + new WhoisReader(WhoisCommandFactory.createCached(), "Please contact registrar"); whoisAction.whoisMetrics = new WhoisMetrics(); whoisAction.metricBuilder = WhoisMetric.builderForRequest(clock); whoisAction.disclaimer = diff --git a/core/src/test/java/google/registry/whois/WhoisCommandFactoryTest.java b/core/src/test/java/google/registry/whois/WhoisCommandFactoryTest.java new file mode 100644 index 000000000..6405f0ca0 --- /dev/null +++ b/core/src/test/java/google/registry/whois/WhoisCommandFactoryTest.java @@ -0,0 +1,240 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.whois; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.testing.DatabaseHelper.newDomainBase; +import static google.registry.testing.DatabaseHelper.newHostResource; +import static google.registry.testing.DatabaseHelper.newRegistry; +import static google.registry.testing.DatabaseHelper.persistNewRegistrar; +import static google.registry.testing.DatabaseHelper.persistResource; + +import com.google.common.collect.ImmutableSet; +import com.google.common.net.InternetDomainName; +import google.registry.config.RegistryConfig; +import google.registry.model.domain.DomainBase; +import google.registry.model.host.HostResource; +import google.registry.model.registrar.Registrar; +import google.registry.testing.AppEngineExtension; +import google.registry.testing.FakeClock; +import google.registry.testing.TestCacheExtension; +import java.net.InetAddress; +import org.joda.time.Duration; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +class WhoisCommandFactoryTest { + + FakeClock clock = new FakeClock(); + + @RegisterExtension + final AppEngineExtension appEngine = + AppEngineExtension.builder().withDatastoreAndCloudSql().withClock(clock).build(); + + @RegisterExtension + final TestCacheExtension testCacheExtension = + new TestCacheExtension.Builder().withEppResourceCache(Duration.millis(1000000000)).build(); + + WhoisCommandFactory noncachedFactory = WhoisCommandFactory.createNonCached(); + WhoisCommandFactory cachedFactory = WhoisCommandFactory.createCached(); + DomainBase domain; + HostResource host; + Registrar otherRegistrar; + + int origSingletonCacheRefreshSeconds; + + @BeforeEach + void setUp() throws Exception { + persistResource(newRegistry("tld", "TLD")); + host = + newHostResource("ns.example.tld") + .asBuilder() + .setInetAddresses(ImmutableSet.of(InetAddress.getByName("1.2.3.4"))) + .build(); + persistResource(host); + domain = newDomainBase("example.tld", host); + persistResource(domain); + otherRegistrar = persistNewRegistrar("OtherRegistrar"); + otherRegistrar = + persistResource( + otherRegistrar + .asBuilder() + .setState(Registrar.State.ACTIVE) + .setPhoneNumber("+1.2223334444") + .build()); + + // In addition to the TestCacheExtension, we have to set a long singleton cache timeout. + RegistryConfig.CONFIG_SETTINGS.get().caching.singletonCacheRefreshSeconds = 1000000; + } + + @AfterEach + void tearDown() { + // Restore the singleton cache timeout. For some reason, this doesn't work if we store the + // original value in an instance variable (I suspect there may be some overlap in test + // execution) so just restore to zero. + RegistryConfig.CONFIG_SETTINGS.get().caching.singletonCacheRefreshSeconds = 0; + } + + @Test + void testNonCached_NameserverLookupByHostCommand() throws Exception { + WhoisResponse response = + noncachedFactory + .nameserverLookupByHost(InternetDomainName.from("ns.example.tld")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + + // Note that we can't use persistResource() for these as that clears the cache. + tm().transact( + () -> + tm().put( + host.asBuilder() + .setPersistedCurrentSponsorClientId("OtherRegistrar") + .build())); + response = + noncachedFactory + .nameserverLookupByHost(InternetDomainName.from("ns.example.tld")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: OtherRegistrar name"); + } + + @Test + void testCached_NameserverLookupByHostCommand() throws Exception { + WhoisResponse response = + cachedFactory + .nameserverLookupByHost(InternetDomainName.from("ns.example.tld")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + + tm().transact( + () -> + tm().put( + host.asBuilder() + .setPersistedCurrentSponsorClientId("OtherRegistrar") + .build())); + response = + cachedFactory + .nameserverLookupByHost(InternetDomainName.from("ns.example.tld")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + } + + @Test + void testNonCached_DomainLookupCommand() throws Exception { + WhoisResponse response = + noncachedFactory + .domainLookup(InternetDomainName.from("example.tld"), true, "REDACTED") + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + + tm().transact( + () -> + tm().put( + domain + .asBuilder() + .setPersistedCurrentSponsorClientId("OtherRegistrar") + .build())); + response = + noncachedFactory + .domainLookup(InternetDomainName.from("example.tld"), true, "REDACTED") + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: OtherRegistrar name"); + } + + @Test + void testCached_DomainLookupCommand() throws Exception { + WhoisResponse response = + cachedFactory + .domainLookup(InternetDomainName.from("example.tld"), true, "REDACTED") + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + + tm().transact( + () -> + tm().put( + domain + .asBuilder() + .setPersistedCurrentSponsorClientId("OtherRegistrar") + .build())); + response = + cachedFactory + .domainLookup(InternetDomainName.from("example.tld"), true, "REDACTED") + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + } + + @Test + void testNonCached_RegistrarLookupCommand() throws Exception { + WhoisResponse response = + noncachedFactory.registrarLookup("OtherRegistrar").executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Phone Number: +1.2223334444"); + + tm().transact( + () -> tm().put(otherRegistrar.asBuilder().setPhoneNumber("+1.2345677890").build())); + response = noncachedFactory.registrarLookup("OtherRegistrar").executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Phone Number: +1.2345677890"); + } + + @Test + void testCached_RegistrarLookupCommand() throws Exception { + WhoisResponse response = + cachedFactory.registrarLookup("OtherRegistrar").executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Phone Number: +1.2223334444"); + + tm().transact( + () -> tm().put(otherRegistrar.asBuilder().setPhoneNumber("+1.2345677890").build())); + response = cachedFactory.registrarLookup("OtherRegistrar").executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Phone Number: +1.2223334444"); + } + + @Test + void testNonCached_NameserverLookupByIpCommand() throws Exception { + // Note that this lookup currently doesn't cache the hosts, so there's no point in testing the + // "cached" case. This test is here so that it will fail if anyone adds caching. + WhoisResponse response = + noncachedFactory + .nameserverLookupByIp(InetAddress.getByName("1.2.3.4")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: The Registrar"); + + tm().transact( + () -> + tm().put( + host.asBuilder() + .setPersistedCurrentSponsorClientId("OtherRegistrar") + .build())); + response = + noncachedFactory + .nameserverLookupByIp(InetAddress.getByName("1.2.3.4")) + .executeQuery(clock.nowUtc()); + assertThat(response.getResponse(false, "").plainTextOutput()) + .contains("Registrar: OtherRegistrar"); + } +} diff --git a/core/src/test/java/google/registry/whois/WhoisHttpActionTest.java b/core/src/test/java/google/registry/whois/WhoisHttpActionTest.java index c6f9e526d..3fd0c1dfd 100644 --- a/core/src/test/java/google/registry/whois/WhoisHttpActionTest.java +++ b/core/src/test/java/google/registry/whois/WhoisHttpActionTest.java @@ -78,7 +78,7 @@ class WhoisHttpActionTest { whoisAction.requestPath = WhoisHttpAction.PATH + pathInfo; whoisAction.response = response; whoisAction.whoisReader = - new WhoisReader(new WhoisCommandFactory(), "Please contact registrar"); + new WhoisReader(WhoisCommandFactory.createCached(), "Please contact registrar"); whoisAction.whoisMetrics = new WhoisMetrics(); whoisAction.metricBuilder = WhoisMetric.builderForRequest(clock); whoisAction.disclaimer = diff --git a/core/src/test/java/google/registry/whois/WhoisReaderTest.java b/core/src/test/java/google/registry/whois/WhoisReaderTest.java index e1b5ee8e0..cf212b7b9 100644 --- a/core/src/test/java/google/registry/whois/WhoisReaderTest.java +++ b/core/src/test/java/google/registry/whois/WhoisReaderTest.java @@ -48,7 +48,7 @@ class WhoisReaderTest { @SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"}) T readCommand(String commandStr) throws Exception { return (T) - new WhoisReader(new WhoisCommandFactory(), "Please contact registrar") + new WhoisReader(WhoisCommandFactory.createCached(), "Please contact registrar") .readCommand(new StringReader(commandStr), false, clock.nowUtc()); }