Disable whois caching in nomulus tool (#980)

* Disable whois caching in nomulus tool

The whois commands previously served output generated from cached EppResource
objects in most cases.  While this is entirely appropriate from server-side,
it is less useful when these commands are run from nomulus tool and, in fact,
when run from the "shell" command this results in changes that have been
applied from the shell not being visible from a subsequent "whois".  The
command may instead serve information on an earlier, cached version of the
resource instead of the latest version.

This implementation uses dagger for parameterization of cached/non-cached
modes.  I did consider the possibility of simply parameterizing the query
commands in all cases as discussed, however, having gone down the
daggerization path and having gotten it to work, I have to say I find this
approach preferrable.  There's really no case for identifying
cached/non-cached on a per-command basis and doing so would require
propagating the flag throughout all levels of the API and all callsites.

Tested: In addition to the new unit test which explicitly verifies the
caching/noncaching behavior of the new commands, tested the actual failing
sequence from "nomulus -e sandbox shell" and verified that the correct results
are served after a mutation.

* Fixed copyright year

* More copyright date fixes

* Added WhoisCommandFactoryTest to fragile tests

I suspect that this test has some contention with other tests, it's not clear
why.

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 {

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);

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.
+ *
+ * <p>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);
+  }
+}

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<WhoisResponse> getResponse(InternetDomainName domainName, DateTime now) {
-    return loadByForeignKeyCached(DomainBase.class, domainName.toString(), now)
+    Optional<DomainBase> domainResource =
-        .map(domain -> new DomainWhoisResponse(domain, fullOutput, whoisRedactedEmailText, now));
+        cached
+            ? loadByForeignKeyCached(DomainBase.class, domainName.toString(), now)
+            : loadByForeignKey(DomainBase.class, domainName.toString(), now);
+    return domainResource.map(
+        domain -> new DomainWhoisResponse(domain, fullOutput, whoisRedactedEmailText, now));
   }
 }

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<WhoisResponse> getResponse(InternetDomainName hostName, DateTime now) {
-    return loadByForeignKeyCached(HostResource.class, hostName.toString(), now)
+    Optional<HostResource> hostResource =
-        .map(host -> new NameserverWhoisResponse(host, now));
+        cached
+            ? loadByForeignKeyCached(HostResource.class, hostName.toString(), now)
+            : loadByForeignKey(HostResource.class, hostName.toString(), now);
+    return hostResource.map(host -> new NameserverWhoisResponse(host, now));
   }
 }

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).
+ *
+ * <h3>Dependencies</h3>
+ *
+ * <ul>
+ *   <li>{@link google.registry.request.RequestModule RequestModule}
+ * </ul>
+ *
+ * @see "google.registry.tools.RegistryToolComponent"
+ */
+@Module
+public final class NonCachingWhoisModule extends BaseWhoisModule {
+  @Provides
+  @Config("whoisCommandFactory")
+  static WhoisCommandFactory provideWhoisCommandFactory() {
+    return WhoisCommandFactory.createNonCached();
+  }
+}

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<Map<String, Registrar>> REGISTRAR_BY_NORMALIZED_NAME_CACHE =
-      memoizeWithShortExpiration(
+      memoizeWithShortExpiration(RegistrarLookupCommand::loadRegistrarMap);
-          () -> {
-            Map<String, Registrar> 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<String> 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);
-          });
 
   @VisibleForTesting
   final String registrarName;
 
-  RegistrarLookupCommand(String registrarName) {
+  RegistrarLookupCommand(String registrarName, boolean cached) {
     checkArgument(!isNullOrEmpty(registrarName), "registrarName");
     this.registrarName = registrarName;
+    this.cached = cached;
+  }
+
+  static Map<String, Registrar> loadRegistrarMap() {
+    Map<String, Registrar> 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<String> 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 =
+    Map<String, Registrar> registrars =
-        REGISTRAR_BY_NORMALIZED_NAME_CACHE.get().get(normalizeRegistrarName(registrarName));
+        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.");

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);
   }
 }

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 {
+public final class WhoisModule extends BaseWhoisModule {
-
-  @Provides
-  @SuppressWarnings("CloseableProvides")
-  static Reader provideHttpInputReader(HttpServletRequest req) {
-    try {
-      return req.getReader();
-    } catch (IOException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
   @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);
-  }
 }

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

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 =

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");
+  }
+}

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 =

core/src/test/java/google/registry/whois/WhoisReaderTest.java

@@ -48,7 +48,7 @@ class WhoisReaderTest {
   @SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"})
   <T> 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());
   }