Make Registrar load methods return Optionals instead of Nullables

This makes the code more understandable from callsites, and also forces
users of this function to deal with the situation where the registrar
with a given client ID might not be present (it was previously silently
NPEing from some of the callsites).

This also adds a test helper method loadRegistrar(clientId) that retains
the old functionality for terseness in tests. It also fixes some instances
of using the load method with the wrong cachedness -- some uses in high-
traffic situations (WHOIS) that should have caching, but also low-traffic
reporting that don't benefit from caching so might as well always be
current.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162990468

java/google/registry/export/PublishDetailReportAction.java

@@ -16,6 +16,7 @@ package google.registry.export;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.google.appengine.tools.cloudstorage.GcsFilename;
 import com.google.common.collect.ImmutableMap;
@@ -52,7 +53,9 @@ public final class PublishDetailReportAction implements Runnable, JsonAction {
   /** Endpoint to which JSON should be sent for this servlet.  See {@code web.xml}. */
   public static final String PATH = "/_dr/publishDetailReport";
 
-  /** Name of parameter indicating the registrar for which this report will be published. */
+  /**
+   * Name of parameter indicating the registrar client ID for which this report will be published.
+   */
   public static final String REGISTRAR_ID_PARAM = "registrar";
 
   /** Name of parameter providing a name for the report file placed in Drive (the base name). */
@@ -83,9 +86,12 @@ public final class PublishDetailReportAction implements Runnable, JsonAction {
     try {
       logger.infofmt("Publishing detail report for parameters: %s", json);
       String registrarId = getParam(json, REGISTRAR_ID_PARAM);
-      Registrar registrar = checkArgumentNotNull(Registrar.loadByClientIdCached(registrarId),
+      Registrar registrar =
-          "Registrar %s not found", registrarId);
+          checkArgumentPresent(
-      String driveFolderId = checkArgumentNotNull(registrar.getDriveFolderId(),
+              Registrar.loadByClientId(registrarId), "Registrar %s not found", registrarId);
+      String driveFolderId =
+          checkArgumentNotNull(
+              registrar.getDriveFolderId(),
               "No drive folder associated with registrar " + registrarId);
       String gcsBucketName = getParam(json, GCS_BUCKET_PARAM);
       String gcsObjectName =

java/google/registry/flows/domain/DomainFlowUtils.java

@@ -231,7 +231,7 @@ public class DomainFlowUtils {
   /** Check if the registrar running the flow has access to the TLD in question. */
   public static void checkAllowedAccessToTld(String clientId, String tld)
       throws EppException {
-    if (!Registrar.loadByClientIdCached(clientId).getAllowedTlds().contains(tld)) {
+    if (!Registrar.loadByClientIdCached(clientId).get().getAllowedTlds().contains(tld)) {
       throw new DomainFlowUtils.NotAuthorizedForTldException(tld);
     }
   }
@@ -433,7 +433,7 @@ public class DomainFlowUtils {
   static void verifyPremiumNameIsNotBlocked(
       String domainName, DateTime priceTime, String clientId) throws EppException {
     if (isDomainPremium(domainName, priceTime)) {
-      if (Registrar.loadByClientIdCached(clientId).getBlockPremiumNames()) {
+      if (Registrar.loadByClientIdCached(clientId).get().getBlockPremiumNames()) {
         throw new PremiumNameBlockedException();
       }
     }

java/google/registry/flows/session/LoginFlow.java

@@ -17,6 +17,7 @@ package google.registry.flows.session;
 import static com.google.common.collect.Sets.difference;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
 import google.registry.flows.EppException;
 import google.registry.flows.EppException.AuthenticationErrorClosingConnectionException;
@@ -116,14 +117,14 @@ public class LoginFlow implements Flow {
       }
       serviceExtensionUrisBuilder.add(uri);
     }
-    Registrar registrar = Registrar.loadByClientIdCached(login.getClientId());
+    Optional<Registrar> registrar = Registrar.loadByClientIdCached(login.getClientId());
-    if (registrar == null) {
+    if (!registrar.isPresent()) {
       throw new BadRegistrarClientIdException(login.getClientId());
     }
 
     // AuthenticationErrorExceptions will propagate up through here.
     try {
-      credentials.validate(registrar, login.getPassword());
+      credentials.validate(registrar.get(), login.getPassword());
     } catch (AuthenticationErrorException e) {
       sessionMetadata.incrementFailedLoginAttempts();
       if (sessionMetadata.getFailedLoginAttempts() > MAX_FAILED_LOGIN_ATTEMPTS_PER_CONNECTION) {
@@ -132,7 +133,7 @@ public class LoginFlow implements Flow {
         throw e;
       }
     }
-    if (registrar.getState().equals(Registrar.State.PENDING)) {
+    if (registrar.get().getState().equals(Registrar.State.PENDING)) {
       throw new RegistrarAccountNotActiveException();
     }
     if (login.getNewPassword() != null) {  // We don't support in-band password changes.

java/google/registry/model/registrar/Registrar.java

@@ -36,8 +36,10 @@ import static google.registry.util.X509Utils.getCertificateHash;
 import static google.registry.util.X509Utils.loadCertificate;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
+import com.google.common.base.Strings;
 import com.google.common.base.Supplier;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
@@ -901,15 +903,16 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
     return CACHE_BY_CLIENT_ID.get().values();
   }
 
-  /** Load a registrar entity by its client id directly from Datastore. */
+  /** Loads and returns a registrar entity by its client id directly from Datastore. */
-  @Nullable
+  public static Optional<Registrar> loadByClientId(String clientId) {
-  public static Registrar loadByClientId(String clientId) {
+    checkArgument(!Strings.isNullOrEmpty(clientId), "clientId must be specified");
-    return ofy().load().type(Registrar.class).parent(getCrossTldKey()).id(clientId).now();
+    return Optional.fromNullable(
+        ofy().load().type(Registrar.class).parent(getCrossTldKey()).id(clientId).now());
   }
 
-  /** Load a registrar entity by its client id using an in-memory cache. */
+  /** Loads and returns a registrar entity by its client id using an in-memory cache. */
-  @Nullable
+  public static Optional<Registrar> loadByClientIdCached(String clientId) {
-  public static Registrar loadByClientIdCached(String clientId) {
+    checkArgument(!Strings.isNullOrEmpty(clientId), "clientId must be specified");
-    return CACHE_BY_CLIENT_ID.get().get(clientId);
+    return Optional.fromNullable(CACHE_BY_CLIENT_ID.get().get(clientId));
   }
 }

java/google/registry/rde/imports/RdeImportUtils.java

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static java.nio.charset.StandardCharsets.US_ASCII;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -163,10 +164,10 @@ public class RdeImportUtils {
         // validate that all registrars exist
         while (parser.nextRegistrar()) {
           XjcRdeRegistrar registrar = parser.getRegistrar();
-          if (Registrar.loadByClientIdCached(registrar.getId()) == null) {
+          checkArgumentPresent(
-            throw new IllegalArgumentException(
+              Registrar.loadByClientIdCached(registrar.getId()),
-                String.format("Registrar '%s' not found in the registry", registrar.getId()));
+              "Registrar '%s' not found in the registry",
-          }
+              registrar.getId());
         }
       } catch (XMLStreamException | JAXBException e) {
         throw new IllegalArgumentException(

java/google/registry/tmch/LordnTask.java

@@ -16,6 +16,7 @@ package google.registry.tmch;
 
 import static com.google.appengine.api.taskqueue.QueueFactory.getQueue;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.google.appengine.api.taskqueue.LeaseOptions;
@@ -26,6 +27,7 @@ import com.google.appengine.api.taskqueue.TaskOptions.Method;
 import com.google.appengine.api.taskqueue.TransientFailureException;
 import com.google.apphosting.api.DeadlineExceededException;
 import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.Uninterruptibles;
@@ -150,11 +152,10 @@ public class LordnTask {
 
   /** Retrieves the IANA identifier for a registrar based on the client id. */
   private static String getIanaIdentifier(String clientId) {
-     Registrar registrar = checkNotNull(
+    Optional<Registrar> registrar = Registrar.loadByClientIdCached(clientId);
-         Registrar.loadByClientIdCached(clientId),
+    checkState(registrar.isPresent(), "No registrar found for client id: %s", clientId);
-         "No registrar found for client id: %s", clientId);
     // Return the string "null" for null identifiers, since some Registrar.Types such as OTE will
     // have null iana ids.
-    return String.valueOf(registrar.getIanaIdentifier());
+    return String.valueOf(registrar.get().getIanaIdentifier());
   }
 }

java/google/registry/tools/CreateAuctionCreditsCommand.java

@@ -17,8 +17,8 @@ package google.registry.tools;
 import static com.google.common.base.CaseFormat.UPPER_CAMEL;
 import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE;
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.model.registry.Registries.assertTldExists;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
@@ -157,13 +157,17 @@ final class CreateAuctionCreditsCommand extends MutatingCommand {
       List<String> fields = splitCsvLine(line);
       checkArgument(CsvHeader.getHeaders().size() == fields.size(), "Wrong number of fields");
       try {
-        String registrarId = fields.get(CsvHeader.AFFILIATE.ordinal());
+        String clientId = fields.get(CsvHeader.AFFILIATE.ordinal());
-        Registrar registrar = checkNotNull(
+        Registrar registrar =
-            Registrar.loadByClientId(registrarId), "Registrar %s not found", registrarId);
+            checkArgumentPresent(
+                Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
         CurrencyUnit tldCurrency = Registry.get(tld).getCurrency();
         CurrencyUnit currency = CurrencyUnit.of((fields.get(CsvHeader.CURRENCY_CODE.ordinal())));
-        checkArgument(tldCurrency.equals(currency),
+        checkArgument(
-            "Credit in wrong currency (%s should be %s)", currency, tldCurrency);
+            tldCurrency.equals(currency),
+            "Credit in wrong currency (%s should be %s)",
+            currency,
+            tldCurrency);
         // We use BigDecimal and BigMoney to preserve fractional currency units when computing the
         // total amount of each credit (since auction credits are percentages of winning bids).
         BigDecimal creditAmount = new BigDecimal(fields.get(CsvHeader.COMMISSIONS.ordinal()));

java/google/registry/tools/CreateCreditBalanceCommand.java

@@ -16,6 +16,7 @@ package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -34,7 +35,7 @@ final class CreateCreditBalanceCommand extends MutatingCommand {
       names = "--registrar",
       description = "Client ID of the registrar owning the credit to create a new balance for",
       required = true)
-  private String registrarId;
+  private String clientId;
 
   @Parameter(
       names = "--credit_id",
@@ -56,14 +57,15 @@ final class CreateCreditBalanceCommand extends MutatingCommand {
 
   @Override
   public void init() throws Exception {
-    Registrar registrar = checkNotNull(
+    Registrar registrar =
-        Registrar.loadByClientId(registrarId), "Registrar %s not found", registrarId);
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
     RegistrarCredit credit = ofy().load()
         .type(RegistrarCredit.class)
         .parent(registrar)
         .id(creditId)
         .now();
-    checkNotNull(credit, "Registrar credit for %s with ID %s not found", registrarId, creditId);
+    checkNotNull(credit, "Registrar credit for %s with ID %s not found", clientId, creditId);
     RegistrarCreditBalance newBalance = new RegistrarCreditBalance.Builder()
         .setParent(credit)
         .setEffectiveTime(effectiveTime)

java/google/registry/tools/CreateCreditCommand.java

@@ -14,7 +14,7 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
@@ -35,7 +35,7 @@ final class CreateCreditCommand extends MutatingCommand {
       names = "--registrar",
       description = "Client ID of the registrar who will be awarded this credit",
       required = true)
-  private String registrarId;
+  private String clientId;
 
   @Parameter(
       names = "--type",
@@ -70,8 +70,9 @@ final class CreateCreditCommand extends MutatingCommand {
   @Override
   protected void init() throws Exception {
     DateTime now = DateTime.now(UTC);
-    Registrar registrar = checkNotNull(
+    Registrar registrar =
-        Registrar.loadByClientId(registrarId), "Registrar %s not found", registrarId);
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
     RegistrarCredit credit = new RegistrarCredit.Builder()
         .setParent(registrar)
         .setType(type)

java/google/registry/tools/CreateRegistrarCommand.java

@@ -79,13 +79,18 @@ final class CreateRegistrarCommand extends CreateOrUpdateRegistrarCommand
           "Client identifier (%s) can only contain lowercase letters, numbers, and hyphens",
           clientId);
     }
-    checkState(Registrar.loadByClientId(clientId) == null, "Registrar %s already exists", clientId);
+    checkState(
+        !Registrar.loadByClientId(clientId).isPresent(), "Registrar %s already exists", clientId);
     List<Registrar> collisions =
-        newArrayList(filter(Registrar.loadAll(), new Predicate<Registrar>() {
+        newArrayList(
+            filter(
+                Registrar.loadAll(),
+                new Predicate<Registrar>() {
                   @Override
                   public boolean apply(Registrar registrar) {
                     return normalizeClientId(registrar.getClientId()).equals(clientId);
-          }}));
+                  }
+                }));
     if (!collisions.isEmpty()) {
       throw new IllegalArgumentException(String.format(
           "The registrar client identifier %s normalizes identically to existing registrar %s",

java/google/registry/tools/CreateRegistrarGroupsCommand.java

@@ -15,7 +15,7 @@
 package google.registry.tools;
 
 import static com.google.common.collect.Iterables.transform;
-import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -53,8 +53,9 @@ public class CreateRegistrarGroupsCommand extends ConfirmingCommand
   @Override
   protected void init() throws IOException {
     for (String clientId : clientIds) {
-      Registrar registrar = Registrar.loadByClientId(clientId);
+      Registrar registrar =
-      checkArgumentNotNull(registrar, "Could not load registrar with id " + clientId);
+          checkArgumentPresent(
+              Registrar.loadByClientId(clientId), "Could not load registrar with id %s", clientId);
       registrars.add(registrar);
     }
   }

java/google/registry/tools/DeleteCreditCommand.java

@@ -16,6 +16,7 @@ package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -31,7 +32,7 @@ final class DeleteCreditCommand extends MutatingCommand {
       names = "--registrar",
       description = "Client ID of the registrar owning the credit to delete",
       required = true)
-  private String registrarId;
+  private String clientId;
 
   @Parameter(
       names = "--credit_id",
@@ -42,13 +43,14 @@ final class DeleteCreditCommand extends MutatingCommand {
   @Override
   protected void init() throws Exception {
     Registrar registrar =
-        checkNotNull(Registrar.loadByClientId(registrarId), "Registrar %s not found", registrarId);
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
     RegistrarCredit credit = ofy().load()
         .type(RegistrarCredit.class)
         .parent(registrar)
         .id(creditId)
         .now();
-    checkNotNull(credit, "Registrar credit for %s with ID %s not found", registrarId, creditId);
+    checkNotNull(credit, "Registrar credit for %s with ID %s not found", clientId, creditId);
     stageEntityChange(credit, null);
 
     for (RegistrarCreditBalance balance :

java/google/registry/tools/EppToolCommand.java

@@ -22,7 +22,7 @@ import static com.google.common.collect.Maps.filterValues;
 import static com.google.common.io.Resources.getResource;
 import static google.registry.model.registry.Registries.findTldForNameOrThrow;
 import static google.registry.tools.CommandUtilities.addHeader;
-import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static google.registry.xml.XmlTransformer.prettyPrint;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -100,8 +100,8 @@ abstract class EppToolCommand extends ConfirmingCommand implements ServerSideCom
   }
 
   protected void addXmlCommand(String clientId, String xml) {
-    checkArgumentNotNull(Registrar.loadByClientId(clientId),
+    checkArgumentPresent(
-        "Registrar with client ID %s not found", clientId);
+        Registrar.loadByClientId(clientId), "Registrar with client ID %s not found", clientId);
     commands.add(new XmlEppParameters(clientId, xml));
   }
 

java/google/registry/tools/GenerateAuctionDataCommand.java

@@ -132,9 +132,9 @@ final class GenerateAuctionDataCommand implements RemoteApiCommand {
 
     // Output records for the registrars of any applications we emitted above.
     for (String clientId : registrars) {
-      Registrar registrar =
+      Optional<Registrar> registrar = Registrar.loadByClientId(clientId);
-          checkNotNull(Registrar.loadByClientId(clientId), "Registrar %s does not exist", clientId);
+      checkState(registrar.isPresent(), "Registrar %s does not exist", clientId);
-      result.add(emitRegistrar(registrar));
+      result.add(emitRegistrar(registrar.get()));
     }
 
     Files.write(output, result, UTF_8);

java/google/registry/tools/GetRegistrarCommand.java

@@ -14,7 +14,7 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkState;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -34,9 +34,9 @@ final class GetRegistrarCommand implements RemoteApiCommand {
   @Override
   public void run() {
     for (String clientId : mainParameters) {
-      Registrar registrar = Registrar.loadByClientId(clientId);
+      Registrar registrar =
-      checkState(registrar != null, "Registrar does not exist");
+          checkArgumentPresent(
-
+              Registrar.loadByClientId(clientId), "Registrar with id %s does not exist", clientId);
       System.out.println(registrar);
     }
   }

java/google/registry/tools/LoadTestCommand.java

@@ -92,11 +92,11 @@ class LoadTestCommand extends ConfirmingCommand implements ServerSideCommand {
 
     // Check validity of TLD and Client Id.
     if (!Registries.getTlds().contains(tld)) {
-      System.err.println("No such TLD: " + tld);
+      System.err.printf("No such TLD: %s\n", tld);
       return false;
     }
-    if (Registrar.loadByClientId(clientId) == null) {
+    if (!Registrar.loadByClientId(clientId).isPresent()) {
-      System.err.println("No such client: " + clientId);
+      System.err.printf("No such client: %s\n", clientId);
       return false;
     }
 

java/google/registry/tools/PublishDetailReportCommand.java

@@ -14,7 +14,7 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -34,7 +34,7 @@ public class PublishDetailReportCommand extends ConfirmingCommand
       names = "--registrar_id",
       description = "Client identifier of the registrar to publish the report for",
       required = true)
-  private String registrarId;
+  private String clientId;
 
   @Parameter(
       names = "--report_name",
@@ -73,8 +73,9 @@ public class PublishDetailReportCommand extends ConfirmingCommand
     // TODO(b/18611424): Fix PublishDetailReportAction to take fewer error-prone parameters.
     gcsFolderPrefix =
         (gcsFolder.isEmpty() || gcsFolder.endsWith("/")) ? gcsFolder : gcsFolder + "/";
-    Registrar registrar = checkNotNull(
+    Registrar registrar =
-        Registrar.loadByClientId(registrarId), "Registrar with ID %s not found", registrarId);
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar with ID %s not found", clientId);
     driveFolderUrl = String.format(DRIVE_FOLDER_URL_TEMPLATE, registrar.getDriveFolderId());
   }
 
@@ -82,7 +83,7 @@ public class PublishDetailReportCommand extends ConfirmingCommand
   protected String prompt() {
     String gcsFile = String.format("gs://%s/%s%s", gcsBucket, gcsFolderPrefix, reportName);
     return "Publish detail report:\n"
-        + " - Registrar: " + registrarId + "\n"
+        + " - Registrar: " + clientId + "\n"
         + " - Drive folder: " + driveFolderUrl + "\n"
         + " - GCS file: " + gcsFile;
   }
@@ -90,7 +91,7 @@ public class PublishDetailReportCommand extends ConfirmingCommand
   @Override
   protected String execute() throws Exception {
     final ImmutableMap<String, String> params = ImmutableMap.of(
-        PublishDetailReportAction.REGISTRAR_ID_PARAM, registrarId,
+        PublishDetailReportAction.REGISTRAR_ID_PARAM, clientId,
         PublishDetailReportAction.DETAIL_REPORT_NAME_PARAM, reportName,
         PublishDetailReportAction.GCS_FOLDER_PREFIX_PARAM, gcsFolderPrefix,
         PublishDetailReportAction.GCS_BUCKET_PARAM, gcsBucket);

java/google/registry/tools/RegistrarContactCommand.java

@@ -20,6 +20,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.Iterables.transform;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.beust.jcommander.Parameter;
@@ -151,7 +152,8 @@ final class RegistrarContactCommand extends MutatingCommand {
         "Must specify exactly one client identifier: %s", ImmutableList.copyOf(mainParameters));
     String clientId = mainParameters.get(0);
     Registrar registrar =
-        checkNotNull(Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
     // If the contact_type parameter is not specified, we should not make any changes.
     if (contactTypeNames == null) {
       contactTypes = null;

java/google/registry/tools/UpdateApplicationStatusCommand.java

@@ -20,6 +20,7 @@ import static google.registry.model.EppResourceUtils.loadDomainApplication;
 import static google.registry.model.domain.launch.ApplicationStatus.ALLOCATED;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -66,7 +67,7 @@ final class UpdateApplicationStatusCommand extends MutatingCommand {
 
   @Override
   protected void init() throws Exception {
-    checkArgumentNotNull(
+    checkArgumentPresent(
         Registrar.loadByClientId(clientId), "Registrar with client ID %s not found", clientId);
     for (final String applicationId : ids) {
       ofy().transact(new VoidWork() {

java/google/registry/tools/UpdateRegistrarCommand.java

@@ -14,7 +14,7 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameters;
 import google.registry.model.registrar.Registrar;
@@ -25,7 +25,7 @@ final class UpdateRegistrarCommand extends CreateOrUpdateRegistrarCommand {
 
   @Override
   Registrar getOldRegistrar(String clientId) {
-    return checkNotNull(
+    return checkArgumentPresent(
         Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
   }
 }

java/google/registry/tools/ValidateLoginCredentialsCommand.java

@@ -17,6 +17,7 @@ package google.registry.tools;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static google.registry.util.X509Utils.getCertificateHash;
 import static google.registry.util.X509Utils.loadCertificate;
 import static java.nio.charset.StandardCharsets.US_ASCII;
@@ -75,7 +76,9 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand {
       clientCertificateHash = getCertificateHash(
           loadCertificate(new String(Files.readAllBytes(clientCertificatePath), US_ASCII)));
     }
-    Registrar registrar = Registrar.loadByClientId(clientId);
+    Registrar registrar =
+        checkArgumentPresent(
+            Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
     new TlsCredentials(clientCertificateHash, Optional.of(clientIpAddress), null)
         .validate(registrar, password);
     checkState(!registrar.getState().equals(Registrar.State.PENDING), "Account pending");

java/google/registry/tools/VerifyOteCommand.java

@@ -16,8 +16,8 @@ package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Predicates.notNull;
-import static com.google.common.base.Verify.verifyNotNull;
 import static google.registry.model.registrar.Registrar.loadByClientId;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -79,7 +79,8 @@ final class VerifyOteCommand implements ServerSideCommand {
       // OT&E registrars are created with clientIDs of registrarName-[1-4], but this command is
       // passed registrarName.  So check the existence of the first persisted registrar to see if
       // the input is valid.
-      verifyNotNull(loadByClientId(clientId + "-1"), "Registrar %s does not exist.", clientId);
+      checkArgumentPresent(
+          loadByClientId(clientId + "-1"), "Registrar %s does not exist.", clientId);
     }
     Collection<String> registrars =
         mainParameters.isEmpty() ? getAllRegistrarNames() : mainParameters;

java/google/registry/tools/server/CreateGroupsAction.java

@@ -121,12 +121,12 @@ public class CreateGroupsAction implements Runnable {
     if (!clientId.isPresent()) {
       respondToBadRequest("Error creating Google Groups, missing parameter: clientId");
     }
-    final Registrar registrar = Registrar.loadByClientId(clientId.get());
+    Optional<Registrar> registrar = Registrar.loadByClientId(clientId.get());
-    if (registrar == null) {
+    if (!registrar.isPresent()) {
       respondToBadRequest(String.format(
           "Error creating Google Groups; could not find registrar with id %s", clientId.get()));
     }
-    return registrar;
+    return registrar.get();
   }
 
   private void respondToBadRequest(String message) {

java/google/registry/ui/server/registrar/ConsoleUiAction.java

@@ -16,6 +16,7 @@ package google.registry.ui.server.registrar;
 
 import static com.google.common.net.HttpHeaders.LOCATION;
 import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
 import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
 import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
@@ -118,9 +119,12 @@ public final class ConsoleUiAction implements Runnable {
               .render());
       return;
     }
-    Registrar registrar = Registrar.loadByClientIdCached(sessionUtils.getRegistrarClientId(req));
+    String clientId = sessionUtils.getRegistrarClientId(req);
+    Registrar registrar =
+        checkArgumentPresent(
+            Registrar.loadByClientIdCached(clientId), "Registrar %s does not exist", clientId);
     data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail()));
-    data.put("clientId", registrar.getClientId());
+    data.put("clientId", clientId);
     data.put("showPaymentLink", registrar.getBillingMethod() == Registrar.BillingMethod.BRAINTREE);
 
     String payload = TOFU_SUPPLIER.get()

java/google/registry/ui/server/registrar/SessionUtils.java

@@ -17,6 +17,7 @@ package google.registry.ui.server.registrar;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Verify.verify;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.google.appengine.api.users.User;
 import com.google.common.base.Optional;
@@ -60,7 +61,9 @@ public class SessionUtils {
     if (!checkRegistrarConsoleLogin(request, authResult.userAuthInfo().get().user())) {
       throw new ForbiddenException("Not authorized to access Registrar Console");
     }
-    return Registrar.loadByClientIdCached(getRegistrarClientId(request));
+    String clientId = getRegistrarClientId(request);
+    return checkArgumentPresent(
+        Registrar.loadByClientIdCached(clientId), "Registrar %s not found", clientId);
   }
 
   /**
@@ -129,8 +132,7 @@ public class SessionUtils {
       return Optional.absent();
     }
     String registrarClientId = contact.getParent().getName();
-    Optional<Registrar> result =
+    Optional<Registrar> result = Registrar.loadByClientIdCached(registrarClientId);
-        Optional.fromNullable(Registrar.loadByClientIdCached(registrarClientId));
     if (!result.isPresent()) {
       logger.severefmt(
           "A contact record exists for non-existent registrar: %s.", Key.create(contact));
@@ -140,12 +142,12 @@ public class SessionUtils {
 
   /** @see #hasAccessToRegistrar(Registrar, String) */
   private static boolean hasAccessToRegistrar(String clientId, final String gaeUserId) {
-    Registrar registrar = Registrar.loadByClientIdCached(clientId);
+    Optional<Registrar> registrar = Registrar.loadByClientIdCached(clientId);
-    if (registrar == null) {
+    if (!registrar.isPresent()) {
       logger.warningfmt("Registrar '%s' disappeared from Datastore!", clientId);
       return false;
     }
-    return hasAccessToRegistrar(registrar, gaeUserId);
+    return hasAccessToRegistrar(registrar.get(), gaeUserId);
   }
 
   /** Returns {@code true} if {@code gaeUserId} is listed in contacts. */

java/google/registry/util/PreconditionsUtils.java

@@ -16,6 +16,7 @@ package google.registry.util;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import com.google.common.base.Optional;
 import javax.annotation.Nullable;
 
 /** Utility methods related to preconditions checking. */
@@ -45,4 +46,28 @@ public class PreconditionsUtils {
     checkArgument(reference != null, errorMessageTemplate, errorMessageArgs);
     return reference;
   }
+
+  /** Checks if the provided Optional is present, returns its value if so, and throws IAE if not. */
+  public static <T> T checkArgumentPresent(Optional<T> reference) {
+    checkArgumentNotNull(reference);
+    checkArgument(reference.isPresent());
+    return reference.get();
+  }
+
+  /** Checks if the provided Optional is present, returns its value if so, and throws IAE if not. */
+  public static <T> T checkArgumentPresent(Optional<T> reference, @Nullable Object errorMessage) {
+    checkArgumentNotNull(reference, errorMessage);
+    checkArgument(reference.isPresent(), errorMessage);
+    return reference.get();
+  }
+
+  /** Checks if the provided Optional is present, returns its value if so, and throws IAE if not. */
+  public static <T> T checkArgumentPresent(
+      Optional<T> reference,
+      @Nullable String errorMessageTemplate,
+      @Nullable Object... errorMessageArgs) {
+    checkArgumentNotNull(reference, errorMessageTemplate, errorMessageArgs);
+    checkArgument(reference.isPresent(), errorMessageTemplate, errorMessageArgs);
+    return reference.get();
+  }
 }

java/google/registry/whois/DomainWhoisResponse.java

@@ -15,6 +15,7 @@
 package google.registry.whois;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.tryFind;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.CollectionUtils.isNullOrEmpty;
@@ -67,11 +68,13 @@ final class DomainWhoisResponse extends WhoisResponseImpl {
 
   @Override
   public WhoisResponseResults getResponse(final boolean preferUnicode, String disclaimer) {
-    Registrar registrar =
+    Optional<Registrar> registrarOptional =
-        checkNotNull(
+        Registrar.loadByClientIdCached(domain.getCurrentSponsorClientId());
-            Registrar.loadByClientId(domain.getCurrentSponsorClientId()),
+    checkState(
+        registrarOptional.isPresent(),
         "Could not load registrar %s",
         domain.getCurrentSponsorClientId());
+    Registrar registrar = registrarOptional.get();
     Optional<RegistrarContact> abuseContact =
         Iterables.tryFind(
             registrar.getContacts(),

java/google/registry/whois/NameserverWhoisResponse.java

@@ -15,9 +15,11 @@
 package google.registry.whois;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.net.InetAddresses;
 import google.registry.model.host.HostResource;
@@ -53,8 +55,8 @@ final class NameserverWhoisResponse extends WhoisResponseImpl {
                   .cloneProjectedAtTime(getTimestamp())
                   .getCurrentSponsorClientId()
               : host.getPersistedCurrentSponsorClientId();
-      Registrar registrar =
+      Optional<Registrar> registrar = Registrar.loadByClientIdCached(clientId);
-          checkNotNull(Registrar.loadByClientId(clientId), "Could not load registrar %s", clientId);
+      checkState(registrar.isPresent(), "Could not load registrar %s", clientId);
       emitter
           .emitField(
               "Server Name", maybeFormatHostname(host.getFullyQualifiedHostName(), preferUnicode))
@@ -67,9 +69,9 @@ final class NameserverWhoisResponse extends WhoisResponseImpl {
                   return InetAddresses.toAddrString(addr);
                 }
               })
-          .emitField("Registrar", registrar.getRegistrarName())
+          .emitField("Registrar", registrar.get().getRegistrarName())
-          .emitField("Registrar WHOIS Server", registrar.getWhoisServer())
+          .emitField("Registrar WHOIS Server", registrar.get().getWhoisServer())
-          .emitField("Registrar URL", registrar.getReferralUrl());
+          .emitField("Registrar URL", registrar.get().getReferralUrl());
       if (i < hosts.size() - 1) {
         emitter.emitNewline();
       }

javatests/google/registry/export/PublishDetailReportActionTest.java

@@ -19,6 +19,7 @@ import static google.registry.export.PublishDetailReportAction.DETAIL_REPORT_NAM
 import static google.registry.export.PublishDetailReportAction.GCS_BUCKET_PARAM;
 import static google.registry.export.PublishDetailReportAction.GCS_FOLDER_PREFIX_PARAM;
 import static google.registry.export.PublishDetailReportAction.REGISTRAR_ID_PARAM;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.mockito.Matchers.any;
@@ -34,7 +35,6 @@ import com.google.appengine.tools.cloudstorage.GcsServiceFactory;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.MediaType;
 import google.registry.gcs.GcsUtils;
-import google.registry.model.registrar.Registrar;
 import google.registry.request.HttpException.BadRequestException;
 import google.registry.request.HttpException.InternalServerErrorException;
 import google.registry.storage.drive.DriveConnection;
@@ -76,8 +76,7 @@ public class PublishDetailReportActionTest {
         anyString(), any(MediaType.class), anyString(), any(byte[].class)))
             .thenReturn("drive-id-123");
 
-    persistResource(
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setDriveFolderId("0B-12345").build());
-        Registrar.loadByClientId("TheRegistrar").asBuilder().setDriveFolderId("0B-12345").build());
 
     // Persist an empty GCS file to the local GCS service so that failure tests won't fail
     // prematurely on the file not existing.
@@ -156,7 +155,7 @@ public class PublishDetailReportActionTest {
   @Test
   public void testFailure_registrarHasNoDriveFolder() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar").asBuilder().setDriveFolderId(null).build());
+        loadRegistrar("TheRegistrar").asBuilder().setDriveFolderId(null).build());
     thrown.expect(BadRequestException.class, "drive folder");
     action.handleJsonRequest(ImmutableMap.of(
         REGISTRAR_ID_PARAM, "TheRegistrar",

javatests/google/registry/export/SyncGroupMembersActionTest.java

@@ -20,6 +20,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registrar.RegistrarContact.Type.ADMIN;
 import static google.registry.model.registrar.RegistrarContact.Type.MARKETING;
 import static google.registry.model.registrar.RegistrarContact.Type.TECH;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
@@ -103,19 +104,15 @@ public class SyncGroupMembersActionTest {
 
   @Test
   public void test_doPost_noneModified() throws Exception {
-    persistResource(Registrar.loadByClientId("NewRegistrar")
+    persistResource(
-        .asBuilder()
+        loadRegistrar("NewRegistrar").asBuilder().setContactsRequireSyncing(false).build());
-        .setContactsRequireSyncing(false)
+    persistResource(
-        .build());
+        loadRegistrar("TheRegistrar").asBuilder().setContactsRequireSyncing(false).build());
-    persistResource(Registrar.loadByClientId("TheRegistrar")
-        .asBuilder()
-        .setContactsRequireSyncing(false)
-        .build());
     runAction();
     verify(response).setStatus(SC_OK);
     verify(response).setPayload("NOT_MODIFIED No registrar contacts have been updated "
         + "since the last time servlet ran.\n");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
   }
 
   @Test
@@ -127,7 +124,7 @@ public class SyncGroupMembersActionTest {
         Role.MEMBER);
     verify(response).setStatus(SC_OK);
     verify(response).setPayload("OK Group memberships successfully updated.\n");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
   }
 
   @Test
@@ -138,7 +135,7 @@ public class SyncGroupMembersActionTest {
     verify(connection).removeMemberFromGroup(
         "newregistrar-primary-contacts@domain-registry.example", "defunct@example.com");
     verify(response).setStatus(SC_OK);
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
   }
 
   @Test
@@ -146,7 +143,7 @@ public class SyncGroupMembersActionTest {
     when(connection.getMembersOfGroup("newregistrar-primary-contacts@domain-registry.example"))
         .thenReturn(ImmutableSet.of("defunct@example.com", "janedoe@theregistrar.com"));
     ofy().deleteWithoutBackup()
-        .entities(Registrar.loadByClientId("NewRegistrar").getContacts())
+        .entities(loadRegistrar("NewRegistrar").getContacts())
         .now();
     runAction();
     verify(connection).removeMemberFromGroup(
@@ -154,7 +151,7 @@ public class SyncGroupMembersActionTest {
     verify(connection).removeMemberFromGroup(
         "newregistrar-primary-contacts@domain-registry.example", "janedoe@theregistrar.com");
     verify(response).setStatus(SC_OK);
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
   }
 
   @Test
@@ -169,14 +166,14 @@ public class SyncGroupMembersActionTest {
         .thenReturn(ImmutableSet.<String> of());
     persistResource(
         new RegistrarContact.Builder()
-            .setParent(Registrar.loadByClientId("NewRegistrar"))
+            .setParent(loadRegistrar("NewRegistrar"))
             .setName("Binary Star")
             .setEmailAddress("binarystar@example.tld")
             .setTypes(ImmutableSet.of(ADMIN, MARKETING))
             .build());
     persistResource(
         new RegistrarContact.Builder()
-            .setParent(Registrar.loadByClientId("TheRegistrar"))
+            .setParent(loadRegistrar("TheRegistrar"))
             .setName("Hexadecimal")
             .setEmailAddress("hexadecimal@snow.fall")
             .setTypes(ImmutableSet.of(TECH))
@@ -223,8 +220,8 @@ public class SyncGroupMembersActionTest {
         .getMembersOfGroup("theregistrar-primary-contacts@domain-registry.example");
     verify(response).setStatus(SC_INTERNAL_SERVER_ERROR);
     verify(response).setPayload("FAILED Error occurred while updating registrar contacts.\n");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
-    assertThat(Registrar.loadByClientId("TheRegistrar").getContactsRequireSyncing()).isTrue();
+    assertThat(loadRegistrar("TheRegistrar").getContactsRequireSyncing()).isTrue();
   }
 
   @Test
@@ -240,6 +237,6 @@ public class SyncGroupMembersActionTest {
         Role.MEMBER);
     verify(response).setStatus(SC_OK);
     verify(response).setPayload("OK Group memberships successfully updated.\n");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
   }
 }

javatests/google/registry/flows/CheckApiActionTest.java

@@ -16,13 +16,13 @@ package google.registry.flows;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistReservedList;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.google.common.collect.ImmutableSet;
 import google.registry.flows.EppTestComponent.FakesAndMocksModule;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeResponse;
@@ -108,10 +108,7 @@ public class CheckApiActionTest {
   public void testFailure_unauthorizedTld() throws Exception {
     createTld("foo");
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar").asBuilder().setAllowedTlds(ImmutableSet.of("foo")).build());
-            .asBuilder()
-            .setAllowedTlds(ImmutableSet.of("foo"))
-            .build());
     assertThat(getCheckResponse("timmy.example")).containsExactly(
         "status", "error",
         "reason", "Registrar is not authorized to access the TLD example");

javatests/google/registry/flows/EppLoginTlsTest.java

@@ -14,11 +14,11 @@
 
 package google.registry.flows;
 
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.base.Optional;
-import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.CertificateSamples;
 import org.joda.time.DateTime;
@@ -45,11 +45,15 @@ public class EppLoginTlsTest extends EppTestCase {
 
   @Before
   public void initTest() throws Exception {
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH)
             .build());
     // Set a cert for the second registrar, or else any cert will be allowed for login.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("TheRegistrar")
+            .asBuilder()
             .setClientCertificateHash(CertificateSamples.SAMPLE_CERT2_HASH)
             .build());
   }
@@ -102,7 +106,9 @@ public class EppLoginTlsTest extends EppTestCase {
   public void testGoodPrimaryCertificate() throws Exception {
     setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH);
     DateTime now = DateTime.now(UTC);
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setClientCertificate(CertificateSamples.SAMPLE_CERT, now)
             .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, now)
             .build());
@@ -113,7 +119,9 @@ public class EppLoginTlsTest extends EppTestCase {
   public void testGoodFailoverCertificate() throws Exception {
     setClientCertificateHash(CertificateSamples.SAMPLE_CERT2_HASH);
     DateTime now = DateTime.now(UTC);
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setClientCertificate(CertificateSamples.SAMPLE_CERT, now)
             .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, now)
             .build());
@@ -124,7 +132,9 @@ public class EppLoginTlsTest extends EppTestCase {
   public void testMissingPrimaryCertificateButHasFailover_usesFailover() throws Exception {
     setClientCertificateHash(CertificateSamples.SAMPLE_CERT2_HASH);
     DateTime now = DateTime.now(UTC);
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setClientCertificate(null, now)
             .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, now)
             .build());
@@ -135,7 +145,9 @@ public class EppLoginTlsTest extends EppTestCase {
   public void testRegistrarHasNoCertificatesOnFile_disablesCertChecking() throws Exception {
     setClientCertificateHash("laffo");
     DateTime now = DateTime.now(UTC);
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setClientCertificate(null, now)
             .setFailoverClientCertificate(null, now)
             .build());

javatests/google/registry/flows/EppLoginUserTest.java

@@ -15,11 +15,11 @@
 package google.registry.flows;
 
 import static com.google.appengine.api.users.UserServiceFactory.getUserService;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.google.appengine.api.users.User;
 import com.google.common.collect.ImmutableSet;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.UserInfo;
@@ -42,8 +42,9 @@ public class EppLoginUserTest extends EppTestCase {
   @Before
   public void initTest() throws Exception {
     User user = getUserService().getCurrentUser();
-    persistResource(new RegistrarContact.Builder()
+    persistResource(
-        .setParent(Registrar.loadByClientId("NewRegistrar"))
+        new RegistrarContact.Builder()
+            .setParent(loadRegistrar("NewRegistrar"))
             .setEmailAddress(user.getEmail())
             .setGaeUserId(user.getUserId())
             .setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN))

javatests/google/registry/flows/domain/ClaimsCheckFlowTest.java

@@ -16,6 +16,7 @@ package google.registry.flows.domain;
 
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.google.common.collect.ImmutableMap;
@@ -28,7 +29,6 @@ import google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException
 import google.registry.flows.domain.DomainFlowUtils.TldDoesNotExistException;
 import google.registry.flows.exceptions.TooManyResourceChecksException;
 import google.registry.model.domain.DomainResource;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import org.junit.Before;
@@ -114,7 +114,7 @@ public class ClaimsCheckFlowTest extends ResourceFlowTestCase<ClaimsCheckFlow, D
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -127,7 +127,7 @@ public class ClaimsCheckFlowTest extends ResourceFlowTestCase<ClaimsCheckFlow, D
     persistClaimsList(
         ImmutableMap.of("example2", "2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001"));
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java

@@ -23,6 +23,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.deleteTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
@@ -109,13 +110,11 @@ import google.registry.model.domain.launch.ApplicationStatus;
 import google.registry.model.domain.launch.LaunchNotice;
 import google.registry.model.domain.launch.LaunchPhase;
 import google.registry.model.domain.secdns.DelegationSignerData;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.registry.label.ReservedList;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.smd.SignedMarkRevocationList;
-import google.registry.testing.DatastoreHelper;
 import google.registry.tmch.TmchCertificateAuthority;
 import google.registry.tmch.TmchXmlSignature;
 import java.security.GeneralSecurityException;
@@ -405,9 +404,7 @@ public class DomainApplicationCreateFlowTest
     persistContactsAndHosts();
     clock.advanceOneMilli();
     // Modify the Registrar to block premium names.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
-        .setBlockPremiumNames(true)
-        .build());
     thrown.expect(PremiumNameBlockedException.class);
     runFlow();
   }
@@ -820,9 +817,7 @@ public class DomainApplicationCreateFlowTest
     persistContactsAndHosts();
     clock.advanceOneMilli();
     // Modify the Registrar to block premium names.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
-        .setBlockPremiumNames(true)
-        .build());
     runSuperuserFlow("domain_create_landrush_premium_response.xml");
   }
 
@@ -1078,7 +1073,8 @@ public class DomainApplicationCreateFlowTest
 
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
-    DatastoreHelper.persistResource(Registrar.loadByClientId("TheRegistrar")
+    persistResource(
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -1133,7 +1129,8 @@ public class DomainApplicationCreateFlowTest
 
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
-    DatastoreHelper.persistResource(Registrar.loadByClientId("TheRegistrar")
+    persistResource(
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java

@@ -20,6 +20,7 @@ import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.newHostResource;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
@@ -45,7 +46,6 @@ import google.registry.model.domain.DomainApplication;
 import google.registry.model.domain.launch.LaunchPhase;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry.TldState;
 import org.junit.Before;
 import org.junit.Test;
@@ -165,7 +165,7 @@ public class DomainApplicationDeleteFlowTest
     persistResource(
         newDomainApplication("example.tld").asBuilder().setRepoId("1-TLD").build());
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -178,7 +178,7 @@ public class DomainApplicationDeleteFlowTest
     persistResource(
         newDomainApplication("example.tld").asBuilder().setRepoId("1-TLD").build());
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java

@@ -21,6 +21,7 @@ import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.generateNewDomainRoid;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
@@ -69,7 +70,6 @@ import google.registry.model.domain.launch.ApplicationStatus;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.reporting.HistoryEntry;
@@ -575,7 +575,7 @@ public class DomainApplicationUpdateFlowTest
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -588,7 +588,7 @@ public class DomainApplicationUpdateFlowTest
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainCheckFlowTest.java

@@ -16,6 +16,7 @@ package google.registry.flows.domain;
 
 import static google.registry.model.eppoutput.CheckData.DomainCheck.create;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
@@ -53,11 +54,9 @@ import google.registry.flows.exceptions.TooManyResourceChecksException;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.launch.ApplicationStatus;
 import google.registry.model.domain.launch.LaunchPhase;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.registry.label.ReservedList;
-import google.registry.testing.DatastoreHelper;
 import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -285,8 +284,8 @@ public class DomainCheckFlowTest
 
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
-    DatastoreHelper.persistResource(
+    persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -297,8 +296,8 @@ public class DomainCheckFlowTest
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistActiveDomain("example2.tld");
-    DatastoreHelper.persistResource(
+    persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainCreateFlowTest.java

@@ -28,6 +28,7 @@ import static google.registry.testing.DatastoreHelper.assertPollMessagesForResou
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.deleteTld;
 import static google.registry.testing.DatastoreHelper.getHistoryEntries;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newContactResource;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.newHostResource;
@@ -122,7 +123,6 @@ import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.reporting.HistoryEntry;
@@ -1166,9 +1166,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
     setEppInput("domain_create_premium.xml");
     persistContactsAndHosts("net");
     // Modify the Registrar to block premium names.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
-        .setBlockPremiumNames(true)
-        .build());
     runFlowAssertResponse(
         CommitMode.LIVE,
         UserPrivileges.SUPERUSER,
@@ -1342,7 +1340,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
     setEppInput("domain_create_premium.xml");
     persistContactsAndHosts("net");
     // Modify the Registrar to block premium names.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(loadRegistrar("TheRegistrar").asBuilder()
         .setBlockPremiumNames(true)
         .build());
     thrown.expect(PremiumNameBlockedException.class);
@@ -1559,7 +1557,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
   public void testFailure_notAuthorizedForTld() throws Exception {
     createTld("irrelevant", "IRR");
     DatastoreHelper.persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of("irrelevant"))
             .build());

javatests/google/registry/flows/domain/DomainDeleteFlowTest.java

@@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyPollMessage;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.newHostResource;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
@@ -64,7 +65,6 @@ import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostResource;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.reporting.HistoryEntry;
@@ -691,7 +691,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
   public void testFailure_notAuthorizedForTld() throws Exception {
     setupSuccessfulTest();
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -703,7 +703,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     setupSuccessfulTest();
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainRenewFlowTest.java

@@ -20,6 +20,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.assertBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
@@ -56,7 +57,6 @@ import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import java.util.Map;
@@ -622,7 +622,7 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -634,7 +634,7 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java

@@ -20,6 +20,7 @@ import static google.registry.testing.DatastoreHelper.assertBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
@@ -58,7 +59,6 @@ import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import java.util.Map;
@@ -329,7 +329,7 @@ public class DomainRestoreRequestFlowTest extends
     persistPendingDeleteDomain();
     // Modify the Registrar to block premium names.
     persistResource(
-        Registrar.loadByClientId("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
+        loadRegistrar("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
     runFlowAssertResponse(
         CommitMode.LIVE,
         UserPrivileges.SUPERUSER,
@@ -432,7 +432,9 @@ public class DomainRestoreRequestFlowTest extends
 
   @Test
   public void testFailure_notInRedemptionPeriod() throws Exception {
-    persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
+    persistResource(
+        newDomainResource(getUniqueIdFromCommand())
+            .asBuilder()
             .setDeletionTime(clock.nowUtc().plusDays(4))
             .setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE))
             .build());
@@ -505,7 +507,7 @@ public class DomainRestoreRequestFlowTest extends
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -517,15 +519,13 @@ public class DomainRestoreRequestFlowTest extends
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
     persistPendingDeleteDomain();
     runFlowAssertResponse(
-        CommitMode.LIVE,
+        CommitMode.LIVE, UserPrivileges.SUPERUSER, readFile("domain_update_response.xml"));
-        UserPrivileges.SUPERUSER,
-        readFile("domain_update_response.xml"));
   }
 
   @Test
@@ -534,8 +534,7 @@ public class DomainRestoreRequestFlowTest extends
     setEppInput("domain_update_restore_request_premium.xml");
     persistPendingDeleteDomain();
     // Modify the Registrar to block premium names.
-    persistResource(
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
-        Registrar.loadByClientId("TheRegistrar").asBuilder().setBlockPremiumNames(true).build());
     thrown.expect(PremiumNameBlockedException.class);
     runFlow();
   }

javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java

@@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyPollMessage;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
 import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries;
@@ -55,7 +56,6 @@ import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
@@ -415,7 +415,7 @@ public class DomainTransferApproveFlowTest
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -426,7 +426,7 @@ public class DomainTransferApproveFlowTest
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java

@@ -20,6 +20,7 @@ import static google.registry.testing.DatastoreHelper.createPollMessageForImplic
 import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
 import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries;
@@ -38,7 +39,6 @@ import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
 import google.registry.model.transfer.TransferStatus;
@@ -291,7 +291,7 @@ public class DomainTransferCancelFlowTest
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -302,7 +302,7 @@ public class DomainTransferCancelFlowTest
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java

@@ -20,6 +20,7 @@ import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyPollMessage;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
 import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries;
@@ -41,7 +42,6 @@ import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
@@ -158,7 +158,7 @@ public class DomainTransferRejectFlowTest
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -169,7 +169,7 @@ public class DomainTransferRejectFlowTest
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java

@@ -21,6 +21,7 @@ import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyPollMessage;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
@@ -66,7 +67,6 @@ import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferResponse;
@@ -563,7 +563,7 @@ public class DomainTransferRequestFlowTest
   public void testFailure_notAuthorizedForTld() throws Exception {
     setupDomain("example", "tld");
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -575,7 +575,7 @@ public class DomainTransferRequestFlowTest
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     setupDomain("example", "tld");
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -694,7 +694,7 @@ public class DomainTransferRequestFlowTest
     clock.advanceOneMilli();
     // Modify the Registrar to block premium names.
     persistResource(
-        Registrar.loadByClientId("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
+        loadRegistrar("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
     // We don't verify the results; just check that the flow doesn't fail.
     runTest("domain_transfer_request_premium.xml", UserPrivileges.SUPERUSER);
   }
@@ -788,7 +788,7 @@ public class DomainTransferRequestFlowTest
     setupDomain("rich", "example");
     // Modify the Registrar to block premium names.
     persistResource(
-        Registrar.loadByClientId("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
+        loadRegistrar("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
     thrown.expect(PremiumNameBlockedException.class);
     doFailingTest("domain_transfer_request_premium.xml");
   }

javatests/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.assertBillingEvents;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
@@ -82,7 +83,6 @@ import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import org.joda.money.Money;
@@ -974,7 +974,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());
@@ -987,7 +987,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   @Test
   public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.<String>of())
             .build());

javatests/google/registry/flows/session/LoginFlowTestCase.java

@@ -15,6 +15,7 @@
 package google.registry.flows.session;
 
 import static google.registry.testing.DatastoreHelper.deleteResource;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import google.registry.flows.EppException.UnimplementedExtensionException;
@@ -42,7 +43,7 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
   @Before
   public void initRegistrar() {
     sessionMetadata.setClientId(null);  // Don't implicitly log in (all other flows need to).
-    registrar = Registrar.loadByClientId("NewRegistrar");
+    registrar = loadRegistrar("NewRegistrar");
     registrarBuilder = registrar.asBuilder();
   }
 

javatests/google/registry/flows/session/LoginFlowViaConsoleTest.java

@@ -15,6 +15,7 @@
 package google.registry.flows.session;
 
 
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.google.appengine.api.users.User;
@@ -72,7 +73,7 @@ public class LoginFlowViaConsoleTest extends LoginFlowTestCase {
   }
 
   private void persistLinkedAccount(String email, String gaeUserId) {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     RegistrarContact c = new RegistrarContact.Builder()
         .setParent(registrar)
         .setEmailAddress(email)

javatests/google/registry/model/billing/RegistrarBillingEntryTest.java

@@ -15,13 +15,13 @@
 package google.registry.model.billing;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.money.CurrencyUnit.USD;
 
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.Key;
 import google.registry.model.EntityTestCase;
-import google.registry.model.registrar.Registrar;
 import google.registry.testing.ExceptionRule;
 import org.joda.money.CurrencyMismatchException;
 import org.joda.money.Money;
@@ -44,7 +44,7 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
         persistResource(
             new RegistrarBillingEntry.Builder()
                 .setPrevious(null)
-                .setParent(Registrar.loadByClientId("NewRegistrar"))
+                .setParent(loadRegistrar("NewRegistrar"))
                 .setCreated(DateTime.parse("1984-12-18TZ"))
                 .setTransactionId("goblin-market")
                 .setDescription("USD Invoice for December 1984")
@@ -59,14 +59,14 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
     RegistrarBillingEntry entry =
         new RegistrarBillingEntry.Builder()
             .setPrevious(null)
-            .setParent(Registrar.loadByClientId("NewRegistrar"))
+            .setParent(loadRegistrar("NewRegistrar"))
             .setCreated(DateTime.parse("1984-12-18TZ"))
             .setTransactionId("goblin-market")
             .setDescription("USD Invoice for December 1984")
             .setAmount(Money.parse("USD 10.00"))
             .build();
     assertThat(entry.getId()).isEqualTo(1L);
-    assertThat(entry.getParent()).isEqualTo(Key.create(Registrar.loadByClientId("NewRegistrar")));
+    assertThat(entry.getParent()).isEqualTo(Key.create(loadRegistrar("NewRegistrar")));
     assertThat(entry.getCreated()).isEqualTo(DateTime.parse("1984-12-18TZ"));
     assertThat(entry.getTransactionId()).isEqualTo("goblin-market");
     assertThat(entry.getDescription()).isEqualTo("USD Invoice for December 1984");
@@ -79,7 +79,7 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
     assertThat(
             new RegistrarBillingEntry.Builder()
                 .setPrevious(null)
-                .setParent(Registrar.loadByClientId("NewRegistrar"))
+                .setParent(loadRegistrar("NewRegistrar"))
                 .setCreated(DateTime.parse("1984-12-18TZ"))
                 .setTransactionId("goblin-market")
                 .setDescription("USD Invoice for December 1984")
@@ -105,12 +105,12 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
         .setPrevious(
             new RegistrarBillingEntry.Builder()
                 .setPrevious(null)
-                .setParent(Registrar.loadByClientId("NewRegistrar"))
+                .setParent(loadRegistrar("NewRegistrar"))
                 .setCreated(DateTime.parse("1984-12-18TZ"))
                 .setDescription("USD Invoice for December")
                 .setAmount(Money.parse("USD 10.00"))
                 .build())
-        .setParent(Registrar.loadByClientId("NewRegistrar"))
+        .setParent(loadRegistrar("NewRegistrar"))
         .setCreated(DateTime.parse("1984-12-17TZ"))
         .setTransactionId("goblin")
         .setDescription("USD Invoice for August")
@@ -125,12 +125,12 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
         .setPrevious(
             new RegistrarBillingEntry.Builder()
                 .setPrevious(null)
-                .setParent(Registrar.loadByClientId("NewRegistrar"))
+                .setParent(loadRegistrar("NewRegistrar"))
                 .setCreated(DateTime.parse("1984-12-18TZ"))
                 .setDescription("USD Invoice for December")
                 .setAmount(Money.parse("USD 10.00"))
                 .build())
-        .setParent(Registrar.loadByClientId("TheRegistrar"))
+        .setParent(loadRegistrar("TheRegistrar"))
         .setCreated(DateTime.parse("1984-12-17TZ"))
         .setTransactionId("goblin")
         .setDescription("USD Invoice for August")
@@ -145,12 +145,12 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
         .setPrevious(
             new RegistrarBillingEntry.Builder()
                 .setPrevious(null)
-                .setParent(Registrar.loadByClientId("NewRegistrar"))
+                .setParent(loadRegistrar("NewRegistrar"))
                 .setCreated(DateTime.parse("1984-12-18TZ"))
                 .setDescription("USD Invoice for December")
                 .setAmount(Money.parse("USD 10.00"))
                 .build())
-        .setParent(Registrar.loadByClientId("NewRegistrar"))
+        .setParent(loadRegistrar("NewRegistrar"))
         .setCreated(DateTime.parse("1984-12-17TZ"))
         .setTransactionId("goblin")
         .setDescription("JPY Invoice for August")
@@ -163,7 +163,7 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
     thrown.expect(IllegalArgumentException.class, "Amount can't be zero");
     new RegistrarBillingEntry.Builder()
         .setPrevious(null)
-        .setParent(Registrar.loadByClientId("NewRegistrar"))
+        .setParent(loadRegistrar("NewRegistrar"))
         .setCreated(DateTime.parse("1984-12-18TZ"))
         .setDescription("USD Invoice for December")
         .setAmount(Money.zero(USD))
@@ -175,7 +175,7 @@ public final class RegistrarBillingEntryTest extends EntityTestCase {
     assertThat(
         new RegistrarBillingEntry.Builder()
             .setPrevious(null)
-            .setParent(Registrar.loadByClientId("NewRegistrar"))
+            .setParent(loadRegistrar("NewRegistrar"))
             .setTransactionId("")
             .setCreated(DateTime.parse("1984-12-18TZ"))
             .setDescription("USD Invoice for December 1984")

javatests/google/registry/model/billing/RegistrarBillingUtilsTest.java

@@ -16,6 +16,7 @@ package google.registry.model.billing;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResources;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
@@ -57,7 +58,7 @@ public final class RegistrarBillingUtilsTest {
   @Before
   public void before() throws Exception {
     inject.setStaticField(Ofy.class, "clock", clock);
-    registrar = Registrar.loadByClientId("NewRegistrar");
+    registrar = loadRegistrar("NewRegistrar");
     createTlds("xn--q9jyb4c", "com", "net");
     persistResource(
         Registry.get("xn--q9jyb4c").asBuilder()

javatests/google/registry/model/billing/RegistrarCreditBalanceTest.java

@@ -17,6 +17,7 @@ package google.registry.model.billing;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
@@ -53,7 +54,7 @@ public class RegistrarCreditBalanceTest extends EntityTestCase {
   @Before
   public void setUp() throws Exception {
     createTld("tld");
-    theRegistrar = Registrar.loadByClientId("TheRegistrar");
+    theRegistrar = loadRegistrar("TheRegistrar");
     unpersistedCredit = makeCredit(theRegistrar, clock.nowUtc());
     credit = persistResource(makeCredit(theRegistrar, clock.nowUtc()));
     balance = persistResource(

javatests/google/registry/model/registrar/RegistrarTest.java

@@ -401,7 +401,7 @@ public class RegistrarTest extends EntityTestCase {
     ofy().transact(new VoidWork() {
       @Override
       public void vrun() {
-        assertThat(Registrar.loadByClientIdCached("registrar")).isNotNull();
+        assertThat(Registrar.loadByClientIdCached("registrar")).isPresent();
         // Load something as a control to make sure we are seeing loaded keys in the session cache.
         ofy().load().entity(abuseAdminContact).now();
         assertThat(ofy().getSessionKeys()).contains(Key.create(abuseAdminContact));
@@ -409,7 +409,31 @@ public class RegistrarTest extends EntityTestCase {
       }});
     ofy().clearSessionCache();
     // Conversely, loads outside of a transaction should end up in the session cache.
-    assertThat(Registrar.loadByClientIdCached("registrar")).isNotNull();
+    assertThat(Registrar.loadByClientIdCached("registrar")).isPresent();
     assertThat(ofy().getSessionKeys()).contains(Key.create(registrar));
   }
+
+  @Test
+  public void testFailure_loadByClientId_clientIdIsNull() throws Exception {
+    thrown.expect(IllegalArgumentException.class, "clientId must be specified");
+    Registrar.loadByClientId(null);
+  }
+
+  @Test
+  public void testFailure_loadByClientId_clientIdIsEmpty() throws Exception {
+    thrown.expect(IllegalArgumentException.class, "clientId must be specified");
+    Registrar.loadByClientId("");
+  }
+
+  @Test
+  public void testFailure_loadByClientIdCached_clientIdIsNull() throws Exception {
+    thrown.expect(IllegalArgumentException.class, "clientId must be specified");
+    Registrar.loadByClientIdCached(null);
+  }
+
+  @Test
+  public void testFailure_loadByClientIdCached_clientIdIsEmpty() throws Exception {
+    thrown.expect(IllegalArgumentException.class, "clientId must be specified");
+    Registrar.loadByClientIdCached("");
+  }
 }

javatests/google/registry/rde/RdeMarshallerTest.java

@@ -15,9 +15,9 @@
 package google.registry.rde;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.xml.ValidationMode.STRICT;
 
-import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ShardableTestCase;
 import google.registry.xml.XmlTestUtils;
@@ -41,8 +41,7 @@ public class RdeMarshallerTest extends ShardableTestCase {
   @Test
   public void testMarshalRegistrar_validData_producesXmlFragment() throws Exception {
     DepositFragment fragment =
-        new RdeMarshaller(STRICT)
+        new RdeMarshaller(STRICT).marshalRegistrar(loadRegistrar("TheRegistrar"));
-            .marshalRegistrar(Registrar.loadByClientId("TheRegistrar"));
     assertThat(fragment.type()).isEqualTo(RdeResourceType.REGISTRAR);
     assertThat(fragment.error()).isEmpty();
     String expected = ""
@@ -85,8 +84,7 @@ public class RdeMarshallerTest extends ShardableTestCase {
   @Test
   public void testMarshalRegistrar_unicodeCharacters_dontGetMangled() throws Exception {
     DepositFragment fragment =
-        new RdeMarshaller(STRICT)
+        new RdeMarshaller(STRICT).marshalRegistrar(loadRegistrar("TheRegistrar"));
-            .marshalRegistrar(Registrar.loadByClientId("TheRegistrar"));
     assertThat(fragment.xml()).contains("123 Example Bőulevard");
   }
 }

javatests/google/registry/server/Fixture.java

@@ -18,6 +18,7 @@ import static google.registry.model.domain.DesignatedContact.Type.ADMIN;
 import static google.registry.model.domain.DesignatedContact.Type.BILLING;
 import static google.registry.model.domain.DesignatedContact.Type.TECH;
 import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newContactResource;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
@@ -135,7 +136,8 @@ public enum Fixture {
               .build());
 
       persistResource(
-          Registrar.loadByClientId("TheRegistrar").asBuilder()
+          loadRegistrar("TheRegistrar")
+              .asBuilder()
               .setAllowedTlds(ImmutableSet.of("example", "xn--q9jyb4c"))
               .setBillingMethod(Registrar.BillingMethod.BRAINTREE)
               .build());

javatests/google/registry/testing/DatastoreHelper.java

@@ -35,6 +35,7 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static google.registry.util.DomainNameUtils.ACE_PREFIX_REGEX;
 import static google.registry.util.DomainNameUtils.getTldFromDomainName;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
 import static java.util.Arrays.asList;
 import static org.joda.money.CurrencyUnit.USD;
@@ -429,13 +430,13 @@ public class DatastoreHelper {
   }
 
   public static void allowRegistrarAccess(String clientId, String tld) {
-    Registrar registrar = Registrar.loadByClientId(clientId);
+    Registrar registrar = loadRegistrar(clientId);
     persistResource(
         registrar.asBuilder().setAllowedTlds(union(registrar.getAllowedTlds(), tld)).build());
   }
 
   private static void disallowRegistrarAccess(String clientId, String tld) {
-    Registrar registrar = Registrar.loadByClientId(clientId);
+    Registrar registrar = loadRegistrar(clientId);
     persistResource(
         registrar.asBuilder().setAllowedTlds(difference(registrar.getAllowedTlds(), tld)).build());
   }
@@ -1023,5 +1024,13 @@ public class DatastoreHelper {
     }
   }
 
+  /** Loads and returns the registrar with the given client ID, or throws IAE if not present. */
+  public static Registrar loadRegistrar(String clientId) {
+    return checkArgumentPresent(
+        Registrar.loadByClientId(clientId),
+        "Error in tests: Registrar %s does not exist",
+        clientId);
+  }
+
   private DatastoreHelper() {}
 }

javatests/google/registry/tmch/LordnTaskTest.java

@@ -17,6 +17,7 @@ package google.registry.tmch;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistDomainAndEnqueueLordn;
 import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
@@ -37,7 +38,6 @@ import com.googlecode.objectify.VoidWork;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.launch.LaunchNotice;
 import google.registry.model.ofy.Ofy;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.Type;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -141,7 +141,7 @@ public class LordnTaskTest {
     ofy().transact(new VoidWork() {
       @Override
       public void vrun() {
-        ofy().save().entity(Registrar.loadByClientId("TheRegistrar").asBuilder()
+        ofy().save().entity(loadRegistrar("TheRegistrar").asBuilder()
             .setType(Type.OTE)
             .setIanaIdentifier(null)
             .build());
@@ -159,12 +159,13 @@ public class LordnTaskTest {
   @Test
   public void test_enqueueDomainResourceTask_throwsExceptionOnInvalidRegistrar() throws Exception {
     DateTime time = DateTime.parse("2010-05-01T10:11:12Z");
-    DomainResource domain = newDomainBuilder(time)
+    DomainResource domain =
+        newDomainBuilder(time)
             .setRepoId("9000-EXAMPLE")
             .setCreationClientId("nonexistentRegistrar")
             .build();
-    thrown.expect(NullPointerException.class,
+    thrown.expect(
-        "No registrar found for client id: nonexistentRegistrar");
+        IllegalStateException.class, "No registrar found for client id: nonexistentRegistrar");
     persistDomainAndEnqueueLordn(domain);
   }
 

javatests/google/registry/tmch/NordnUploadActionTest.java

@@ -20,6 +20,7 @@ import static com.google.common.net.HttpHeaders.LOCATION;
 import static com.google.common.net.MediaType.FORM_DATA;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistDomainAndEnqueueLordn;
 import static google.registry.testing.DatastoreHelper.persistResource;
@@ -42,7 +43,6 @@ import com.google.common.collect.ImmutableList;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.launch.LaunchNotice;
 import google.registry.model.ofy.Ofy;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -109,8 +109,7 @@ public class NordnUploadActionTest {
     when(httpResponse.getResponseCode()).thenReturn(SC_ACCEPTED);
     when(httpResponse.getHeadersUncombined())
         .thenReturn(ImmutableList.of(new HTTPHeader(LOCATION, "http://trololol")));
-    persistResource(
+    persistResource(loadRegistrar("TheRegistrar").asBuilder().setIanaIdentifier(99999L).build());
-        Registrar.loadByClientId("TheRegistrar").asBuilder().setIanaIdentifier(99999L).build());
     createTld("tld");
     persistResource(Registry.get("tld").asBuilder().setLordnUsername("lolcat").build());
     lordnRequestInitializer.marksdbLordnPassword = Optional.of("attack");

javatests/google/registry/tools/CreateCreditBalanceCommandTest.java

@@ -18,6 +18,7 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.DateTimeZone.UTC;
@@ -44,7 +45,7 @@ public class CreateCreditBalanceCommandTest extends CommandTestCase<CreateCredit
 
   @Before
   public void setUp() {
-    registrar = Registrar.loadByClientId("TheRegistrar");
+    registrar = loadRegistrar("TheRegistrar");
     createTld("tld");
     assertThat(Registry.get("tld").getCurrency()).isEqualTo(USD);
     credit = persistResource(
@@ -79,7 +80,7 @@ public class CreateCreditBalanceCommandTest extends CommandTestCase<CreateCredit
 
   @Test
   public void testFailure_nonexistentParentRegistrar() throws Exception {
-    thrown.expect(NullPointerException.class, "FakeRegistrar");
+    thrown.expect(IllegalArgumentException.class, "Registrar FakeRegistrar not found");
     runCommandForced(
         "--registrar=FakeRegistrar",
         "--credit_id=" + creditId,

javatests/google/registry/tools/CreateCreditCommandTest.java

@@ -18,6 +18,7 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.DateTimeZone.UTC;
 
@@ -41,7 +42,7 @@ public class CreateCreditCommandTest extends CommandTestCase<CreateCreditCommand
   @Before
   public void setUp() {
     createTld("tld");
-    registrar = Registrar.loadByClientId("TheRegistrar");
+    registrar = loadRegistrar("TheRegistrar");
     assertThat(Registry.get("tld").getCurrency()).isEqualTo(USD);
   }
 
@@ -92,7 +93,7 @@ public class CreateCreditCommandTest extends CommandTestCase<CreateCreditCommand
 
   @Test
   public void testFailure_nonexistentParentRegistrar() throws Exception {
-    thrown.expect(NullPointerException.class, "FakeRegistrar");
+    thrown.expect(IllegalArgumentException.class, "Registrar FakeRegistrar not found");
     runCommandForced(
         "--registrar=FakeRegistrar",
         "--type=PROMOTION",

javatests/google/registry/tools/CreateRegistrarCommandTest.java

@@ -27,6 +27,7 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 import com.beust.jcommander.ParameterException;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Range;
 import com.google.common.net.MediaType;
@@ -73,8 +74,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
     // Clear the cache so that the CreateAutoTimestamp field gets reloaded.
     ofy().clearSessionCache();
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrarOptional = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrarOptional).isPresent();
+    Registrar registrar = registrarOptional.get();
     assertThat(registrar.testPassword("some_password")).isTrue();
     assertThat(registrar.getType()).isEqualTo(Registrar.Type.REAL);
     assertThat(registrar.getIanaIdentifier()).isEqualTo(8);
@@ -110,9 +112,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.testPassword("some_password")).isTrue();
+    assertThat(registrar.get().testPassword("some_password")).isTrue();
   }
 
   @Test
@@ -130,9 +132,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getType()).isEqualTo(Registrar.Type.TEST);
+    assertThat(registrar.get().getType()).isEqualTo(Registrar.Type.TEST);
   }
 
   @Test
@@ -152,9 +154,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getState()).isEqualTo(Registrar.State.SUSPENDED);
+    assertThat(registrar.get().getState()).isEqualTo(Registrar.State.SUSPENDED);
   }
 
   @Test
@@ -177,9 +179,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getAllowedTlds()).containsExactly("xn--q9jyb4c", "foobar");
+    assertThat(registrar.get().getAllowedTlds()).containsExactly("xn--q9jyb4c", "foobar");
   }
 
   @Test
@@ -224,8 +226,8 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
     assertInStdout("Registrar created, but groups creation failed with error");
     assertInStdout("BAD ROBOT NO COOKIE");
   }
@@ -267,10 +269,11 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getIpAddressWhitelist())
+    assertThat(registrar.get().getIpAddressWhitelist())
-        .containsExactlyElementsIn(registrar.getIpAddressWhitelist()).inOrder();
+        .containsExactlyElementsIn(registrar.get().getIpAddressWhitelist())
+        .inOrder();
   }
 
   @Test
@@ -290,9 +293,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getIpAddressWhitelist()).isEmpty();
+    assertThat(registrar.get().getIpAddressWhitelist()).isEmpty();
   }
 
   @Test
@@ -312,9 +315,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getClientCertificateHash())
+    assertThat(registrar.get().getClientCertificateHash())
         .isEqualTo(CertificateSamples.SAMPLE_CERT_HASH);
   }
 
@@ -335,10 +338,10 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getClientCertificate()).isNull();
+    assertThat(registrar.get().getClientCertificate()).isNull();
-    assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
+    assertThat(registrar.get().getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
   }
 
   @Test
@@ -358,8 +361,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrarOptional = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrarOptional).isPresent();
+    Registrar registrar = registrarOptional.get();
     assertThat(registrar.getClientCertificate()).isNull();
     assertThat(registrar.getClientCertificateHash()).isNull();
     assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT);
@@ -382,9 +386,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getIanaIdentifier()).isEqualTo(12345);
+    assertThat(registrar.get().getIanaIdentifier()).isEqualTo(12345);
   }
 
   @Test
@@ -404,9 +408,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getBillingIdentifier()).isEqualTo(12345);
+    assertThat(registrar.get().getBillingIdentifier()).isEqualTo(12345);
   }
 
   @Test
@@ -426,9 +430,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getBillingAccountMap())
+    assertThat(registrar.get().getBillingAccountMap())
         .containsExactly(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz");
   }
 
@@ -473,10 +477,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getBillingAccountMap())
+    assertThat(registrar.get().getBillingAccountMap()).containsExactly(CurrencyUnit.JPY, "789xyz");
-        .containsExactly(CurrencyUnit.JPY, "789xyz");
   }
 
   @Test
@@ -497,8 +500,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--icann_referral_email=foo@bar.test",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrarOptional = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrarOptional).isPresent();
+    Registrar registrar = registrarOptional.get();
     assertThat(registrar.getLocalizedAddress()).isNotNull();
     assertThat(registrar.getLocalizedAddress().getStreet()).hasSize(3);
     assertThat(registrar.getLocalizedAddress().getStreet().get(0)).isEqualTo("1234 Main St");
@@ -527,9 +531,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getEmailAddress()).isEqualTo("foo@foo.foo");
+    assertThat(registrar.get().getEmailAddress()).isEqualTo("foo@foo.foo");
   }
 
   @Test
@@ -549,9 +553,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getUrl()).isEqualTo("http://foo.foo");
+    assertThat(registrar.get().getUrl()).isEqualTo("http://foo.foo");
   }
 
   @Test
@@ -571,9 +575,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getPhoneNumber()).isEqualTo("+1.2125556342");
+    assertThat(registrar.get().getPhoneNumber()).isEqualTo("+1.2125556342");
   }
 
   @Test
@@ -597,8 +601,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrarOptional = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrarOptional).isPresent();
+    Registrar registrar = registrarOptional.get();
     assertThat(registrar.getIanaIdentifier()).isNull();
     assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
@@ -630,8 +635,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrarOptional = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrarOptional).isPresent();
+    Registrar registrar = registrarOptional.get();
     assertThat(registrar.getIanaIdentifier()).isNull();
     assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
@@ -658,9 +664,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getBlockPremiumNames()).isTrue();
+    assertThat(registrar.get().getBlockPremiumNames()).isTrue();
   }
 
   @Test
@@ -680,9 +686,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getBlockPremiumNames()).isFalse();
+    assertThat(registrar.get().getBlockPremiumNames()).isFalse();
   }
 
   @Test
@@ -740,9 +746,9 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--cc US",
         "clientz");
 
-    Registrar registrar = Registrar.loadByClientId("clientz");
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isNotNull();
+    assertThat(registrar).isPresent();
-    assertThat(registrar.getFaxNumber()).isEqualTo("+1.2125556342");
+    assertThat(registrar.get().getFaxNumber()).isEqualTo("+1.2125556342");
   }
 
   @Test

javatests/google/registry/tools/DeleteCreditCommandTest.java

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.money.CurrencyUnit.USD;
 
@@ -47,7 +48,7 @@ public class DeleteCreditCommandTest extends CommandTestCase<DeleteCreditCommand
 
   @Before
   public void setUp() {
-    registrar = Registrar.loadByClientId("TheRegistrar");
+    registrar = loadRegistrar("TheRegistrar");
     createTld("tld");
     assertThat(Registry.get("tld").getCurrency()).isEqualTo(USD);
     DateTime creditCreationTime = Registry.get("tld").getCreationTime().plusMillis(1);
@@ -110,7 +111,7 @@ public class DeleteCreditCommandTest extends CommandTestCase<DeleteCreditCommand
 
   @Test
   public void testFailure_nonexistentParentRegistrar() throws Exception {
-    thrown.expect(NullPointerException.class, "FakeRegistrar");
+    thrown.expect(IllegalArgumentException.class, "Registrar FakeRegistrar not found");
     runCommandForced("--registrar=FakeRegistrar", "--credit_id=" + creditAId);
   }
 

javatests/google/registry/tools/GetRegistrarCommandTest.java

@@ -34,7 +34,7 @@ public class GetRegistrarCommandTest extends CommandTestCase<GetRegistrarCommand
 
   @Test
   public void testFailure_registrarDoesNotExist() throws Exception {
-    thrown.expect(IllegalStateException.class);
+    thrown.expect(IllegalArgumentException.class, "Registrar with id ClientZ does not exist");
     runCommand("ClientZ");
   }
 
@@ -46,7 +46,7 @@ public class GetRegistrarCommandTest extends CommandTestCase<GetRegistrarCommand
 
   @Test
   public void testFailure_oneRegistrarDoesNotExist() throws Exception {
-    thrown.expect(IllegalStateException.class);
+    thrown.expect(IllegalArgumentException.class, "Registrar with id ClientZ does not exist");
     runCommand("NewRegistrar", "ClientZ");
   }
 }

javatests/google/registry/tools/RegistrarContactCommandTest.java

@@ -19,6 +19,7 @@ import static google.registry.model.registrar.RegistrarContact.Type.ABUSE;
 import static google.registry.model.registrar.RegistrarContact.Type.ADMIN;
 import static google.registry.model.registrar.RegistrarContact.Type.TECH;
 import static google.registry.model.registrar.RegistrarContact.Type.WHOIS;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResources;
@@ -45,7 +46,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testList() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     RegistrarContact.updateContacts(
         registrar,
         ImmutableSet.of(
@@ -70,7 +71,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testUpdate() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     ImmutableList<RegistrarContact> contacts = ImmutableList.of(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -93,8 +94,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--visible_in_whois_as_tech=false",
         "--visible_in_domain_whois_as_abuse=false",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact).isEqualTo(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -111,7 +111,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testUpdate_enableConsoleAccess() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -123,14 +123,13 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--email=jane.doe@example.com",
         "--allow_console_access=true",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getGaeUserId()).matches("-?[0-9]+");
   }
 
   @Test
   public void testUpdate_disableConsoleAccess() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -143,14 +142,13 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--email=judith.doe@example.com",
         "--allow_console_access=false",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getGaeUserId()).isNull();
   }
 
   @Test
   public void testUpdate_unsetOtherWhoisAbuseFlags() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -172,7 +170,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--visible_in_domain_whois_as_abuse=true",
         "NewRegistrar");
     ImmutableList<RegistrarContact> registrarContacts =
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList();
+        loadRegistrar("NewRegistrar").getContacts().asList();
     for (RegistrarContact registrarContact : registrarContacts) {
       if (registrarContact.getName().equals("John Doe")) {
         assertThat(registrarContact.getVisibleInDomainWhoisAsAbuse()).isTrue();
@@ -184,7 +182,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testUpdate_cannotUnsetOnlyWhoisAbuseContact() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -204,14 +202,13 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
     } catch (IllegalArgumentException e) {
       assertThat(e).hasMessageThat().contains("Cannot clear visible_in_domain_whois_as_abuse flag");
     }
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getVisibleInDomainWhoisAsAbuse()).isTrue();
   }
 
   @Test
   public void testUpdate_emptyCommandModifiesNothing() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     RegistrarContact existingContact = persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -226,8 +223,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
             .setVisibleInDomainWhoisAsAbuse(true)
             .build());
     runCommandForced("--mode=UPDATE", "--email=john.doe@example.com", "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getEmailAddress()).isEqualTo(existingContact.getEmailAddress());
     assertThat(registrarContact.getName()).isEqualTo(existingContact.getName());
     assertThat(registrarContact.getGaeUserId()).isEqualTo(existingContact.getGaeUserId());
@@ -244,7 +240,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testUpdate_listOfTypesWorks() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -263,14 +259,13 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--email=john.doe@example.com",
         "--contact_type=ADMIN,TECH",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getTypes()).containsExactly(ADMIN, TECH);
   }
 
   @Test
   public void testUpdate_clearAllTypes() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistSimpleResource(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -283,14 +278,13 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--email=john.doe@example.com",
         "--contact_type=",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getTypes()).isEmpty();
   }
 
   @Test
   public void testCreate_withAdminType() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("NewRegistrar");
+    Registrar registrar = loadRegistrar("NewRegistrar");
     runCommandForced(
         "--mode=CREATE",
         "--name=Jim Doe",
@@ -300,8 +294,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--visible_in_whois_as_tech=false",
         "--visible_in_domain_whois_as_abuse=true",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact).isEqualTo(
         new RegistrarContact.Builder()
             .setParent(registrar)
@@ -317,18 +310,17 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
 
   @Test
   public void testDelete() throws Exception {
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContacts()).isNotEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getContacts()).isNotEmpty();
     runCommandForced(
         "--mode=DELETE",
         "--email=janedoe@theregistrar.com",
         "NewRegistrar");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContacts()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getContacts()).isEmpty();
   }
 
   @Test
   public void testDelete_failsOnDomainWhoisAbuseContact() throws Exception {
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(0);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(0);
     persistSimpleResource(
         registrarContact.asBuilder().setVisibleInDomainWhoisAsAbuse(true).build());
     try {
@@ -341,7 +333,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
     } catch (IllegalArgumentException e) {
       assertThat(e).hasMessageThat().contains("Cannot delete the domain WHOIS abuse contact");
     }
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContacts()).isNotEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getContacts()).isNotEmpty();
   }
 
   @Test
@@ -353,37 +345,26 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--allow_console_access=true",
         "--contact_type=ADMIN,ABUSE",
         "NewRegistrar");
-    RegistrarContact registrarContact =
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getGaeUserId()).matches("-?[0-9]+");
   }
 
   @Test
   public void testCreate_withNoContactTypes() throws Exception {
     runCommandForced(
-        "--mode=CREATE",
+        "--mode=CREATE", "--name=Jim Doe", "--email=jim.doe@example.com", "NewRegistrar");
-        "--name=Jim Doe",
+    RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
-        "--email=jim.doe@example.com",
-        "NewRegistrar");
-    RegistrarContact registrarContact =
-        Registrar.loadByClientId("NewRegistrar").getContacts().asList().get(1);
     assertThat(registrarContact.getTypes()).isEmpty();
   }
 
   @Test
   public void testCreate_syncingRequiredSetToTrue() throws Exception {
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar").asBuilder().setContactsRequireSyncing(false).build());
-            .asBuilder()
-            .setContactsRequireSyncing(false)
-            .build());
 
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
     runCommandForced(
-        "--mode=CREATE",
+        "--mode=CREATE", "--name=Jim Doe", "--email=jim.doe@example.com", "NewRegistrar");
-        "--name=Jim Doe",
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isTrue();
-        "--email=jim.doe@example.com",
-        "NewRegistrar");
-    assertThat(Registrar.loadByClientId("NewRegistrar").getContactsRequireSyncing()).isTrue();
   }
 }

javatests/google/registry/tools/ResaveEnvironmentEntitiesCommandTest.java

@@ -19,6 +19,7 @@ import static com.google.common.collect.Iterables.transform;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 
 import com.google.common.base.Function;
 import google.registry.model.ImmutableObject;
@@ -70,11 +71,11 @@ public class ResaveEnvironmentEntitiesCommandTest
     assertThat(savedEntities)
         .containsExactly(
             // The Registrars and RegistrarContacts are created by AppEngineRule.
-            Registrar.loadByClientId("TheRegistrar"),
+            loadRegistrar("TheRegistrar"),
-            Registrar.loadByClientId("NewRegistrar"),
+            loadRegistrar("NewRegistrar"),
             Registry.get("tld"),
-            getOnlyElement(Registrar.loadByClientId("TheRegistrar").getContacts()),
+            getOnlyElement(loadRegistrar("TheRegistrar").getContacts()),
-            getOnlyElement(Registrar.loadByClientId("NewRegistrar").getContacts()));
+            getOnlyElement(loadRegistrar("NewRegistrar").getContacts()));
   }
 
   @SafeVarargs

javatests/google/registry/tools/SetupOteCommandTest.java

@@ -19,6 +19,7 @@ import static google.registry.model.registrar.Registrar.State.ACTIVE;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistPremiumList;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.time.DateTimeZone.UTC;
@@ -94,7 +95,7 @@ public class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
       String allowedTld,
       String password,
       ImmutableList<CidrAddressBlock> ipWhitelist) {
-    Registrar registrar = Registrar.loadByClientId(registrarName);
+    Registrar registrar = loadRegistrar(registrarName);
     assertThat(registrar).isNotNull();
     assertThat(registrar.getAllowedTlds()).containsExactlyElementsIn(ImmutableSet.of(allowedTld));
     assertThat(registrar.getRegistrarName()).isEqualTo(registrarName);
@@ -321,7 +322,7 @@ public class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
 
   @Test
   public void testFailure_registrarExists() throws Exception {
-    Registrar registrar = Registrar.loadByClientId("TheRegistrar").asBuilder()
+    Registrar registrar = loadRegistrar("TheRegistrar").asBuilder()
         .setClientId("blobio-1")
         .setRegistrarName("blobio-1")
         .build();

javatests/google/registry/tools/UniformRapidSuspensionCommandTest.java

@@ -14,6 +14,7 @@
 
 package google.registry.tools;
 
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
@@ -25,7 +26,6 @@ import com.googlecode.objectify.Key;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
-import google.registry.model.registrar.Registrar;
 import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
 import org.junit.Before;
 import org.junit.Test;
@@ -42,9 +42,8 @@ public class UniformRapidSuspensionCommandTest
   @Before
   public void initResources() {
     // Since the command's history client ID must be CharlestonRoad, resave TheRegistrar that way.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(
-        .setClientId("CharlestonRoad")
+        loadRegistrar("TheRegistrar").asBuilder().setClientId("CharlestonRoad").build());
-        .build());
     ns1 = persistActiveHost("ns1.example.com");
     ns2 = persistActiveHost("ns2.example.com");
     urs1 = persistActiveHost("urs1.example.com");

javatests/google/registry/tools/UpdateApplicationStatusCommandTest.java

@@ -21,6 +21,7 @@ import static google.registry.model.domain.launch.ApplicationStatus.PENDING_ALLO
 import static google.registry.model.domain.launch.ApplicationStatus.REJECTED;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.newContactResourceWithRoid;
 import static google.registry.testing.DatastoreHelper.newDomainApplication;
 import static google.registry.testing.DatastoreHelper.persistResource;
@@ -35,7 +36,6 @@ import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.reporting.HistoryEntry;
 import org.joda.time.DateTime;
 import org.junit.Before;
@@ -52,9 +52,8 @@ public class UpdateApplicationStatusCommandTest
   public void init() {
     // Since the command's history client ID defaults to CharlestonRoad, resave TheRegistrar as
     // CharlestonRoad so we don't have to pass in --history_client_id everywhere below.
-    persistResource(Registrar.loadByClientId("TheRegistrar").asBuilder()
+    persistResource(
-        .setClientId("CharlestonRoad")
+        loadRegistrar("TheRegistrar").asBuilder().setClientId("CharlestonRoad").build());
-        .build());
 
     createTld("xn--q9jyb4c");
     domainApplication = persistResource(newDomainApplication(

javatests/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -14,13 +14,12 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.registrar.Registrar.loadByClientId;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.time.DateTimeZone.UTC;
 
@@ -44,28 +43,28 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_password() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").testPassword("some_password")).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").testPassword("some_password")).isFalse();
     runCommand("--password=some_password", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").testPassword("some_password")).isTrue();
+    assertThat(loadRegistrar("NewRegistrar").testPassword("some_password")).isTrue();
   }
 
   @Test
   public void testSuccess_registrarType() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setType(Registrar.Type.OTE)
             .setIanaIdentifier(null)
             .build());
-    assertThat(loadByClientId("NewRegistrar").getType()).isEqualTo(Type.OTE);
+    assertThat(loadRegistrar("NewRegistrar").getType()).isEqualTo(Type.OTE);
     runCommand("--registrar_type=TEST", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getType()).isEqualTo(Type.TEST);
+    assertThat(loadRegistrar("NewRegistrar").getType()).isEqualTo(Type.TEST);
   }
 
   @Test
   public void testFailure_noPasscodeOnChangeToReal() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setType(Registrar.Type.OTE)
             .setIanaIdentifier(null)
@@ -77,21 +76,21 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_registrarState() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getState()).isEqualTo(State.ACTIVE);
+    assertThat(loadRegistrar("NewRegistrar").getState()).isEqualTo(State.ACTIVE);
     runCommand("--registrar_state=SUSPENDED", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getState()).isEqualTo(State.SUSPENDED);
+    assertThat(loadRegistrar("NewRegistrar").getState()).isEqualTo(State.SUSPENDED);
   }
 
   @Test
   public void testSuccess_allowedTlds() throws Exception {
     createTlds("xn--q9jyb4c", "foobar");
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
     runCommand("--allowed_tlds=xn--q9jyb4c,foobar", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getAllowedTlds())
+    assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .containsExactly("xn--q9jyb4c", "foobar");
   }
 
@@ -99,12 +98,12 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   public void testSuccess_addAllowedTlds() throws Exception {
     createTlds("xn--q9jyb4c", "foo", "bar");
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
     runCommand("--add_allowed_tlds=foo,bar", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getAllowedTlds())
+    assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .containsExactly("xn--q9jyb4c", "foo", "bar");
   }
 
@@ -112,20 +111,20 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   public void testSuccess_addAllowedTldsWithDupes() throws Exception {
     createTlds("xn--q9jyb4c", "foo", "bar");
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
     runCommand("--add_allowed_tlds=xn--q9jyb4c,foo,bar", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getAllowedTlds())
+    assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .isEqualTo(ImmutableSet.of("xn--q9jyb4c", "foo", "bar"));
   }
 
   @Test
   public void testSuccess_ipWhitelist() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getIpAddressWhitelist()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getIpAddressWhitelist()).isEmpty();
     runCommand("--ip_whitelist=192.168.1.1,192.168.0.2/16", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getIpAddressWhitelist())
+    assertThat(loadRegistrar("NewRegistrar").getIpAddressWhitelist())
         .containsExactly(
             CidrAddressBlock.create("192.168.1.1"), CidrAddressBlock.create("192.168.0.2/16"))
         .inOrder();
@@ -134,25 +133,25 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   @Test
   public void testSuccess_clearIpWhitelist() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setIpAddressWhitelist(
                 ImmutableList.of(
                     CidrAddressBlock.create("192.168.1.1"),
                     CidrAddressBlock.create("192.168.0.2/16")))
             .build());
-    assertThat(loadByClientId("NewRegistrar").getIpAddressWhitelist()).isNotEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getIpAddressWhitelist()).isNotEmpty();
     runCommand("--ip_whitelist=null", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getIpAddressWhitelist()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getIpAddressWhitelist()).isEmpty();
   }
 
   @Test
   public void testSuccess_certFile() throws Exception {
-    Registrar registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    Registrar registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getClientCertificate()).isNull();
     assertThat(registrar.getClientCertificateHash()).isNull();
     runCommand("--cert_file=" + getCertFilename(), "--force", "NewRegistrar");
-    registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    registrar = loadRegistrar("NewRegistrar");
     // NB: Hash was computed manually using 'openssl x509 -fingerprint -sha256 -in ...' and then
     // converting the result from a hex string to non-padded base64 encoded string.
     assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);
@@ -161,62 +160,62 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_certHash() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getClientCertificateHash()).isNull();
+    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull();
     runCommand("--cert_hash=" + SAMPLE_CERT_HASH, "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getClientCertificateHash())
+    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash())
         .isEqualTo(SAMPLE_CERT_HASH);
   }
 
   @Test
   public void testSuccess_clearCert() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setClientCertificate(SAMPLE_CERT, DateTime.now(UTC))
             .build());
-    assertThat(isNullOrEmpty(loadByClientId("NewRegistrar").getClientCertificate())).isFalse();
+    assertThat(isNullOrEmpty(loadRegistrar("NewRegistrar").getClientCertificate())).isFalse();
     runCommand("--cert_file=/dev/null", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getClientCertificate()).isNull();
+    assertThat(loadRegistrar("NewRegistrar").getClientCertificate()).isNull();
   }
 
   @Test
   public void testSuccess_clearCertHash() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setClientCertificateHash(SAMPLE_CERT_HASH)
             .build());
-    assertThat(isNullOrEmpty(loadByClientId("NewRegistrar").getClientCertificateHash())).isFalse();
+    assertThat(isNullOrEmpty(loadRegistrar("NewRegistrar").getClientCertificateHash())).isFalse();
     runCommand("--cert_hash=null", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getClientCertificateHash()).isNull();
+    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull();
   }
 
   @Test
   public void testSuccess_ianaId() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getIanaIdentifier()).isEqualTo(8);
+    assertThat(loadRegistrar("NewRegistrar").getIanaIdentifier()).isEqualTo(8);
     runCommand("--iana_id=12345", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getIanaIdentifier()).isEqualTo(12345);
+    assertThat(loadRegistrar("NewRegistrar").getIanaIdentifier()).isEqualTo(12345);
   }
 
   @Test
   public void testSuccess_billingId() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getBillingIdentifier()).isNull();
+    assertThat(loadRegistrar("NewRegistrar").getBillingIdentifier()).isNull();
     runCommand("--billing_id=12345", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBillingIdentifier()).isEqualTo(12345);
+    assertThat(loadRegistrar("NewRegistrar").getBillingIdentifier()).isEqualTo(12345);
   }
 
   @Test
   public void testSuccess_billingAccountMap() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand("--billing_account_map=USD=abc123,JPY=789xyz", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap())
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
         .containsExactly(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz");
   }
 
   @Test
   public void testFailure_billingAccountMap_doesNotContainEntryForTldAllowed() throws Exception {
     createTlds("foo");
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     thrown.expect(IllegalArgumentException.class, "USD");
     runCommand(
         "--billing_account_map=JPY=789xyz",
@@ -229,13 +228,13 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   @Test
   public void testSuccess_billingAccountMap_onlyAppliesToRealRegistrar() throws Exception {
     createTlds("foo");
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap()).isEmpty();
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand(
         "--billing_account_map=JPY=789xyz",
         "--allowed_tlds=foo",
         "--force",
         "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap())
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
         .containsExactly(CurrencyUnit.JPY, "789xyz");
   }
 
@@ -243,29 +242,29 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   public void testSuccess_billingAccountMap_partialUpdate() throws Exception {
     createTlds("foo");
     persistResource(
-        loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setBillingAccountMap(
                 ImmutableMap.of(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz"))
             .build());
     runCommand("--billing_account_map=JPY=123xyz", "--allowed_tlds=foo", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBillingAccountMap())
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
         .containsExactly(CurrencyUnit.JPY, "123xyz", CurrencyUnit.USD, "abc123");
   }
 
   @Test
   public void testSuccess_changeBillingMethodToBraintreeWhenBalanceIsZero() throws Exception {
     createTlds("xn--q9jyb4c");
-    assertThat(loadByClientId("NewRegistrar").getBillingMethod()).isEqualTo(BillingMethod.EXTERNAL);
+    assertThat(loadRegistrar("NewRegistrar").getBillingMethod()).isEqualTo(BillingMethod.EXTERNAL);
     runCommand("--billing_method=braintree", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBillingMethod())
+    assertThat(loadRegistrar("NewRegistrar").getBillingMethod())
         .isEqualTo(BillingMethod.BRAINTREE);
   }
 
   @Test
   public void testFailure_changeBillingMethodWhenBalanceIsNonZero() throws Exception {
     createTlds("xn--q9jyb4c");
-    Registrar registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    Registrar registrar = loadRegistrar("NewRegistrar");
     persistResource(
         new RegistrarBillingEntry.Builder()
             .setPrevious(null)
@@ -286,7 +285,7 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
     runCommand("--street=\"1234 Main St\"", "--street \"4th Floor\"", "--street \"Suite 1\"",
         "--city Brooklyn", "--state NY", "--zip 11223", "--cc US", "--force", "NewRegistrar");
 
-    Registrar registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    Registrar registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getLocalizedAddress() != null).isTrue();
     assertThat(registrar.getLocalizedAddress().getStreet()).hasSize(3);
     assertThat(registrar.getLocalizedAddress().getStreet().get(0)).isEqualTo("1234 Main St");
@@ -300,39 +299,39 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_blockPremiumNames() throws Exception {
-    assertThat(loadByClientId("NewRegistrar").getBlockPremiumNames()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getBlockPremiumNames()).isFalse();
     runCommand("--block_premium=true", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBlockPremiumNames()).isTrue();
+    assertThat(loadRegistrar("NewRegistrar").getBlockPremiumNames()).isTrue();
   }
 
   @Test
   public void testSuccess_resetBlockPremiumNames() throws Exception {
-    persistResource(loadByClientId("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
+    persistResource(loadRegistrar("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
     runCommand("--block_premium=false", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getBlockPremiumNames()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getBlockPremiumNames()).isFalse();
   }
 
   @Test
   public void testSuccess_blockPremiumNamesUnspecified() throws Exception {
-    persistResource(loadByClientId("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
+    persistResource(loadRegistrar("NewRegistrar").asBuilder().setBlockPremiumNames(true).build());
     // Make some unrelated change where we don't specify "--block_premium".
     runCommand("--billing_id=12345", "--force", "NewRegistrar");
     // Make sure the field didn't get reset back to false.
-    assertThat(loadByClientId("NewRegistrar").getBlockPremiumNames()).isTrue();
+    assertThat(loadRegistrar("NewRegistrar").getBlockPremiumNames()).isTrue();
   }
 
   @Test
   public void testSuccess_updateMultiple() throws Exception {
-    assertThat(loadByClientId("TheRegistrar").getState()).isEqualTo(State.ACTIVE);
+    assertThat(loadRegistrar("TheRegistrar").getState()).isEqualTo(State.ACTIVE);
-    assertThat(loadByClientId("NewRegistrar").getState()).isEqualTo(State.ACTIVE);
+    assertThat(loadRegistrar("NewRegistrar").getState()).isEqualTo(State.ACTIVE);
     runCommand("--registrar_state=SUSPENDED", "--force", "TheRegistrar", "NewRegistrar");
-    assertThat(loadByClientId("TheRegistrar").getState()).isEqualTo(State.SUSPENDED);
+    assertThat(loadRegistrar("TheRegistrar").getState()).isEqualTo(State.SUSPENDED);
-    assertThat(loadByClientId("NewRegistrar").getState()).isEqualTo(State.SUSPENDED);
+    assertThat(loadRegistrar("NewRegistrar").getState()).isEqualTo(State.SUSPENDED);
   }
 
   @Test
   public void testSuccess_resetOptionalParamsNullString() throws Exception {
-    Registrar registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    Registrar registrar = loadRegistrar("NewRegistrar");
     registrar = persistResource(registrar.asBuilder()
         .setType(Type.PDT) // for non-null IANA ID
         .setIanaIdentifier(9995L)
@@ -364,7 +363,7 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
         "--force",
         "NewRegistrar");
 
-    registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getIanaIdentifier()).isNull();
     assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
@@ -376,7 +375,7 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_resetOptionalParamsEmptyString() throws Exception {
-    Registrar registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    Registrar registrar = loadRegistrar("NewRegistrar");
     registrar = persistResource(registrar.asBuilder()
         .setType(Type.PDT) // for non-null IANA ID
         .setIanaIdentifier(9995L)
@@ -408,7 +407,7 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
         "--force",
         "NewRegistrar");
 
-    registrar = checkNotNull(loadByClientId("NewRegistrar"));
+    registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getIanaIdentifier()).isNull();
     assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
@@ -421,16 +420,16 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
   @Test
   public void testSuccess_setWhoisServer_works() throws Exception {
     runCommand("--whois=whois.goth.black", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getWhoisServer()).isEqualTo("whois.goth.black");
+    assertThat(loadRegistrar("NewRegistrar").getWhoisServer()).isEqualTo("whois.goth.black");
   }
 
   @Test
   public void testSuccess_triggerGroupSyncing_works() throws Exception {
     persistResource(
-        loadByClientId("NewRegistrar").asBuilder().setContactsRequireSyncing(false).build());
+        loadRegistrar("NewRegistrar").asBuilder().setContactsRequireSyncing(false).build());
-    assertThat(loadByClientId("NewRegistrar").getContactsRequireSyncing()).isFalse();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isFalse();
     runCommand("--sync_groups=true", "--force", "NewRegistrar");
-    assertThat(loadByClientId("NewRegistrar").getContactsRequireSyncing()).isTrue();
+    assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isTrue();
   }
 
   @Test
@@ -594,7 +593,7 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testFailure_doesNotExist() throws Exception {
-    thrown.expect(NullPointerException.class);
+    thrown.expect(IllegalArgumentException.class, "Registrar ClientZ not found");
     runCommand("--force", "ClientZ");
   }
 

javatests/google/registry/tools/ValidateLoginCredentialsCommandTest.java

@@ -16,6 +16,7 @@ package google.registry.tools;
 
 import static google.registry.model.registrar.Registrar.State.ACTIVE;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.beust.jcommander.ParameterException;
@@ -23,7 +24,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.flows.EppException;
 import google.registry.flows.TransportCredentials.BadRegistrarPasswordException;
-import google.registry.model.registrar.Registrar;
 import google.registry.testing.CertificateSamples;
 import google.registry.util.CidrAddressBlock;
 import org.junit.Before;
@@ -40,7 +40,9 @@ public class ValidateLoginCredentialsCommandTest
   @Before
   public void init() {
     createTld("tld");
-    persistResource(Registrar.loadByClientId("NewRegistrar").asBuilder()
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
             .setPassword(PASSWORD)
             .setClientCertificateHash(CERT_HASH)
             .setIpAddressWhitelist(ImmutableList.of(new CidrAddressBlock(CLIENT_IP)))

javatests/google/registry/tools/VerifyOteCommandTest.java

@@ -14,6 +14,7 @@
 
 package google.registry.tools;
 
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.mockito.Matchers.anyMapOf;
 import static org.mockito.Matchers.anyString;
@@ -21,7 +22,6 @@ import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import google.registry.model.registrar.Registrar;
@@ -48,7 +48,7 @@ public class VerifyOteCommandTest extends CommandTestCase<VerifyOteCommand> {
   @Test
   public void testSuccess_pass() throws Exception {
     Registrar registrar =
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setClientId("blobio-1")
             .setRegistrarName("blobio-1")
@@ -66,7 +66,7 @@ public class VerifyOteCommandTest extends CommandTestCase<VerifyOteCommand> {
 
   @Test
   public void testFailure_registrarDoesntExist() throws Exception {
-    thrown.expect(VerifyException.class, "Registrar blobio does not exist.");
+    thrown.expect(IllegalArgumentException.class, "Registrar blobio does not exist.");
     runCommand("blobio");
   }
 

javatests/google/registry/tools/server/ListRegistrarsActionTest.java

@@ -15,11 +15,11 @@
 package google.registry.tools.server;
 
 import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
-import google.registry.model.registrar.Registrar;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -40,12 +40,12 @@ public class ListRegistrarsActionTest extends ListActionTestCase {
     // Ensure that NewRegistrar only has access to xn--q9jyb4c and that TheRegistrar only has access
     // to example.
     persistResource(
-        Registrar.loadByClientId("NewRegistrar")
+        loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
     persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("example"))
             .build());

javatests/google/registry/ui/server/registrar/ContactSettingsTest.java

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResource;
 
@@ -48,14 +49,14 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     @SuppressWarnings("unchecked")
     List<Map<String, ?>> results = (List<Map<String, ?>>) response.get("results");
     assertThat(results.get(0).get("contacts"))
-        .isEqualTo(Registrar.loadByClientId(CLIENT_ID).toJsonMap().get("contacts"));
+        .isEqualTo(loadRegistrar(CLIENT_ID).toJsonMap().get("contacts"));
   }
 
   @Test
   public void testPost_loadSaveRegistrar_success() throws Exception {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
-        "args", Registrar.loadByClientId(CLIENT_ID).toJsonMap()));
+        "args", loadRegistrar(CLIENT_ID).toJsonMap()));
     assertThat(response).containsEntry("status", "SUCCESS");
   }
 
@@ -70,12 +71,11 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     // Have to keep ADMIN or else expect FormException for at-least-one.
     adminContact1.put("types", "ADMIN");
 
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     Map<String, Object> regMap = registrar.toJsonMap();
     regMap.put("contacts", ImmutableList.of(adminContact1));
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
+    Map<String, Object> response =
-        "op", "update",
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", regMap));
-        "args", regMap));
     assertThat(response).containsEntry("status", "SUCCESS");
 
     RegistrarContact newContact = new RegistrarContact.Builder()
@@ -85,13 +85,12 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
         .setPhoneNumber((String) adminContact1.get("phoneNumber"))
         .setTypes(ImmutableList.of(RegistrarContact.Type.ADMIN))
         .build();
-    assertThat(Registrar.loadByClientId(CLIENT_ID).getContacts())
+    assertThat(loadRegistrar(CLIENT_ID).getContacts()).containsExactly(newContact);
-        .containsExactlyElementsIn(ImmutableSet.of(newContact));
   }
 
   @Test
   public void testPost_updateContacts_requiredTypes_error() throws Exception {
-    Map<String, Object> reqJson = Registrar.loadByClientId(CLIENT_ID).toJsonMap();
+    Map<String, Object> reqJson = loadRegistrar(CLIENT_ID).toJsonMap();
     reqJson.put("contacts",
         ImmutableList.of(AppEngineRule.makeRegistrarContact2()
             .asBuilder()
@@ -108,7 +107,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   @Test
   public void testPost_updateContacts_requireTechPhone_error() throws Exception {
     // First make the contact a tech contact as well.
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     RegistrarContact rc = AppEngineRule.makeRegistrarContact2()
         .asBuilder()
         .setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN, RegistrarContact.Type.TECH))
@@ -132,7 +131,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   @Test
   public void testPost_updateContacts_cannotRemoveWhoisAbuseContact_error() throws Exception {
     // First make the contact's info visible in whois as abuse contact info.
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     RegistrarContact rc =
         AppEngineRule.makeRegistrarContact2()
             .asBuilder()
@@ -158,7 +157,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   public void testPost_updateContacts_whoisAbuseContactMustHavePhoneNumber_error()
       throws Exception {
     // First make the contact's info visible in whois as abuse contact info.
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     RegistrarContact rc =
         AppEngineRule.makeRegistrarContact2()
             .asBuilder()

javatests/google/registry/ui/server/registrar/RegistrarPaymentActionTest.java

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.ReflectiveFieldExtractor.extractField;
 import static java.util.Arrays.asList;
 import static org.mockito.Matchers.any;
@@ -33,7 +34,6 @@ import com.braintreegateway.ValidationErrorCode;
 import com.braintreegateway.ValidationErrors;
 import com.google.appengine.api.users.User;
 import com.google.common.collect.ImmutableMap;
-import google.registry.model.registrar.Registrar;
 import google.registry.request.auth.AuthLevel;
 import google.registry.request.auth.AuthResult;
 import google.registry.request.auth.UserAuthInfo;
@@ -94,7 +94,7 @@ public class RegistrarPaymentActionTest {
     when(transactionGateway.sale(any(TransactionRequest.class))).thenReturn(result);
     when(sessionUtils.getRegistrarForAuthResult(
             any(HttpServletRequest.class), any(AuthResult.class)))
-        .thenReturn(Registrar.loadByClientId("TheRegistrar"));
+        .thenReturn(loadRegistrar("TheRegistrar"));
   }
 
   @Test

javatests/google/registry/ui/server/registrar/RegistrarPaymentSetupActionTest.java

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static java.util.Arrays.asList;
 import static org.mockito.Matchers.any;
@@ -45,10 +46,7 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class RegistrarPaymentSetupActionTest {
 
-  @Rule
+  @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build();
-  public final AppEngineRule appEngine = AppEngineRule.builder()
-      .withDatastore()
-      .build();
 
   private final BraintreeGateway braintreeGateway = mock(BraintreeGateway.class);
   private final ClientTokenGateway clientTokenGateway = mock(ClientTokenGateway.class);
@@ -65,7 +63,7 @@ public class RegistrarPaymentSetupActionTest {
     action.braintreeGateway = braintreeGateway;
     action.customerSyncer = customerSyncer;
     Registrar registrar = persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setBillingMethod(Registrar.BillingMethod.BRAINTREE)
             .build());
@@ -93,7 +91,7 @@ public class RegistrarPaymentSetupActionTest {
                     "token", blanketsOfSadness,
                     "currencies", asList("USD", "JPY"),
                     "brainframe", "/doodle")));
-    verify(customerSyncer).sync(eq(Registrar.loadByClientId("TheRegistrar")));
+    verify(customerSyncer).sync(eq(loadRegistrar("TheRegistrar")));
   }
 
   @Test
@@ -108,7 +106,7 @@ public class RegistrarPaymentSetupActionTest {
   @Test
   public void testNotOnCreditCardBillingTerms_showsErrorPage() throws Exception {
     Registrar registrar = persistResource(
-        Registrar.loadByClientId("TheRegistrar")
+        loadRegistrar("TheRegistrar")
             .asBuilder()
             .setBillingMethod(Registrar.BillingMethod.EXTERNAL)
             .build());

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
 import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
@@ -28,7 +29,6 @@ import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableMap;
 import google.registry.export.sheet.SyncRegistrarsSheetAction;
-import google.registry.model.registrar.Registrar;
 import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.auth.AuthResult;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
@@ -88,8 +88,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testRead_authorized_returnsRegistrarJson() throws Exception {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.<String, Object>of());
     assertThat(response).containsEntry("status", "SUCCESS");
-    assertThat(response).containsEntry("results", asList(
+    assertThat(response).containsEntry("results", asList(loadRegistrar(CLIENT_ID).toJsonMap()));
-        Registrar.loadByClientId(CLIENT_ID).toJsonMap()));
   }
 
   @Test

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java

@@ -18,6 +18,7 @@ import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddres
 import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName;
 import static google.registry.security.JsonHttpTestUtils.createJsonPayload;
 import static google.registry.security.JsonHttpTestUtils.createJsonResponseSupplier;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -29,7 +30,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import google.registry.export.sheet.SyncRegistrarsSheetAction;
 import google.registry.model.ofy.Ofy;
-import google.registry.model.registrar.Registrar;
 import google.registry.request.JsonActionRunner;
 import google.registry.request.JsonResponse;
 import google.registry.request.ResponseImpl;
@@ -107,7 +107,7 @@ public class RegistrarSettingsActionTestCase {
     when(req.getContentType()).thenReturn("application/json");
     when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read")));
     when(sessionUtils.getRegistrarForAuthResult(req, action.authResult))
-        .thenReturn(Registrar.loadByClientId(CLIENT_ID));
+        .thenReturn(loadRegistrar(CLIENT_ID));
     when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname");
   }
 

javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java

@@ -21,6 +21,7 @@ import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT2;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT2_HASH;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static java.util.Arrays.asList;
@@ -44,7 +45,9 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testPost_updateCert_success() throws Exception {
-    Registrar modified = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Registrar modified =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setClientCertificate(SAMPLE_CERT, clock.nowUtc())
             .build();
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
@@ -59,12 +62,12 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
             .build();
     assertThat(response).containsEntry("status", "SUCCESS");
     assertThat(response).containsEntry("results", asList(modified.toJsonMap()));
-    assertThat(Registrar.loadByClientId(CLIENT_ID)).isEqualTo(modified);
+    assertThat(loadRegistrar(CLIENT_ID)).isEqualTo(modified);
   }
 
   @Test
   public void testPost_updateCert_failure() throws Exception {
-    Map<String, Object> reqJson = Registrar.loadByClientId(CLIENT_ID).toJsonMap();
+    Map<String, Object> reqJson = loadRegistrar(CLIENT_ID).toJsonMap();
     reqJson.put("clientCertificate", "BLAH");
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
@@ -75,13 +78,13 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testChangeCertificates() throws Exception {
-    Map<String, Object> jsonMap = Registrar.loadByClientId(CLIENT_ID).toJsonMap();
+    Map<String, Object> jsonMap = loadRegistrar(CLIENT_ID).toJsonMap();
     jsonMap.put("clientCertificate", SAMPLE_CERT);
     jsonMap.put("failoverClientCertificate", null);
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update", "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);
     assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
     assertThat(registrar.getFailoverClientCertificate()).isNull();
@@ -90,20 +93,22 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testChangeFailoverCertificate() throws Exception {
-    Map<String, Object> jsonMap = Registrar.loadByClientId(CLIENT_ID).toJsonMap();
+    Map<String, Object> jsonMap = loadRegistrar(CLIENT_ID).toJsonMap();
     jsonMap.put("failoverClientCertificate", SAMPLE_CERT2);
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update", "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT2);
     assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT2_HASH);
   }
 
   @Test
   public void testEmptyOrNullCertificate_doesNotClearOutCurrentOne() throws Exception {
-    Registrar initialRegistrar = persistResource(
+    Registrar initialRegistrar =
-        Registrar.loadByClientId(CLIENT_ID).asBuilder()
+        persistResource(
+            loadRegistrar(CLIENT_ID)
+                .asBuilder()
                 .setClientCertificate(SAMPLE_CERT, START_OF_TIME)
                 .setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME)
                 .build());
@@ -115,7 +120,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update", "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
-    Registrar registrar = Registrar.loadByClientId(CLIENT_ID);
+    Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);
     assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
     assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT2);
@@ -124,7 +129,9 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testToJsonMap_containsCertificate() throws Exception {
-    Map<String, Object> jsonMap = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Map<String, Object> jsonMap =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setClientCertificate(SAMPLE_CERT2, START_OF_TIME)
             .build()
             .toJsonMap();
@@ -134,7 +141,9 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testToJsonMap_containsFailoverCertificate() throws Exception {
-    Map<String, Object> jsonMap = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Map<String, Object> jsonMap =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME)
             .build()
             .toJsonMap();

javatests/google/registry/ui/server/registrar/SessionUtilsTest.java

@@ -17,6 +17,7 @@ package google.registry.ui.server.registrar;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.AppEngineRule.THE_REGISTRAR_GAE_USER_ID;
 import static google.registry.testing.DatastoreHelper.deleteResource;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -25,7 +26,6 @@ import static org.mockito.Mockito.when;
 
 import com.google.appengine.api.users.User;
 import com.google.common.testing.NullPointerTester;
-import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -85,8 +85,7 @@ public class SessionUtilsTest {
   @Test
   public void testCheckRegistrarConsoleLogin_sessionRevoked_invalidates() throws Exception {
     RegistrarContact.updateContacts(
-        Registrar.loadByClientId("TheRegistrar"),
+        loadRegistrar("TheRegistrar"), new java.util.HashSet<RegistrarContact>());
-        new java.util.HashSet<RegistrarContact>());
     when(session.getAttribute("clientId")).thenReturn("TheRegistrar");
     assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isFalse();
     verify(session).invalidate();
@@ -94,7 +93,7 @@ public class SessionUtilsTest {
 
   @Test
   public void testCheckRegistrarConsoleLogin_orphanedContactIsDenied() throws Exception {
-    deleteResource(Registrar.loadByClientId("TheRegistrar"));
+    deleteResource(loadRegistrar("TheRegistrar"));
     assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isFalse();
   }
 

javatests/google/registry/ui/server/registrar/WhoisSettingsTest.java

@@ -16,6 +16,7 @@ package google.registry.ui.server.registrar;
 
 import static com.google.common.base.Strings.repeat;
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static java.util.Arrays.asList;
 
 import com.google.common.collect.ImmutableList;
@@ -37,13 +38,16 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testPost_update_success() throws Exception {
-    Registrar modified = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Registrar modified =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setEmailAddress("hello.kitty@example.com")
             .setPhoneNumber("+1.2125650000")
             .setFaxNumber("+1.2125650001")
             .setReferralUrl("http://acme.com/")
             .setWhoisServer("ns1.foo.bar")
-        .setLocalizedAddress(new RegistrarAddress.Builder()
+            .setLocalizedAddress(
+                new RegistrarAddress.Builder()
                     .setStreet(ImmutableList.of("76 Ninth Avenue", "Eleventh Floor"))
                     .setCity("New York")
                     .setState("NY")
@@ -51,21 +55,23 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .setCountryCode("US")
                     .build())
             .build();
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
+    Map<String, Object> response =
-        "op", "update",
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
-        "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("SUCCESS");
     assertThat(response.get("results")).isEqualTo(asList(modified.toJsonMap()));
-    assertThat(Registrar.loadByClientId(CLIENT_ID)).isEqualTo(modified);
+    assertThat(loadRegistrar(CLIENT_ID)).isEqualTo(modified);
   }
 
   @Test
   public void testPost_badUsStateCode_returnsFormFieldError() throws Exception {
-    Registrar modified = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Registrar modified =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setEmailAddress("hello.kitty@example.com")
             .setPhoneNumber("+1.2125650000")
             .setFaxNumber("+1.2125650001")
-        .setLocalizedAddress(new RegistrarAddress.Builder()
+            .setLocalizedAddress(
+                new RegistrarAddress.Builder()
                     .setStreet(ImmutableList.of("76 Ninth Avenue", "Eleventh Floor"))
                     .setCity("New York")
                     .setState("ZZ")
@@ -73,22 +79,24 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .setCountryCode("US")
                     .build())
             .build();
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
+    Map<String, Object> response =
-        "op", "update",
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
-        "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("localizedAddress.state");
     assertThat(response.get("message")).isEqualTo("Unknown US state code.");
-    assertThat(Registrar.loadByClientId(CLIENT_ID)).isNotEqualTo(modified);
+    assertThat(loadRegistrar(CLIENT_ID)).isNotEqualTo(modified);
   }
 
   @Test
   public void testPost_badAddress_returnsFormFieldError() throws Exception {
-    Registrar modified = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Registrar modified =
+        loadRegistrar(CLIENT_ID)
+            .asBuilder()
             .setEmailAddress("hello.kitty@example.com")
             .setPhoneNumber("+1.2125650000")
             .setFaxNumber("+1.2125650001")
-        .setLocalizedAddress(new RegistrarAddress.Builder()
+            .setLocalizedAddress(
+                new RegistrarAddress.Builder()
                     .setStreet(ImmutableList.of("76 Ninth Avenue", repeat("lol", 200)))
                     .setCity("New York")
                     .setState("NY")
@@ -96,27 +104,24 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .setCountryCode("US")
                     .build())
             .build();
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
+    Map<String, Object> response =
-        "op", "update",
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
-        "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("localizedAddress.street[1]");
     assertThat((String) response.get("message"))
         .contains("Number of characters (600) not in range");
-    assertThat(Registrar.loadByClientId(CLIENT_ID)).isNotEqualTo(modified);
+    assertThat(loadRegistrar(CLIENT_ID)).isNotEqualTo(modified);
   }
 
   @Test
   public void testPost_badWhoisServer_returnsFormFieldError() throws Exception {
-    Registrar modified = Registrar.loadByClientId(CLIENT_ID).asBuilder()
+    Registrar modified =
-        .setWhoisServer("tears@dry.tragical.lol")
+        loadRegistrar(CLIENT_ID).asBuilder().setWhoisServer("tears@dry.tragical.lol").build();
-        .build();
+    Map<String, Object> response =
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
-        "op", "update",
-        "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("whoisServer");
     assertThat(response.get("message")).isEqualTo("Not a valid hostname.");
-    assertThat(Registrar.loadByClientId(CLIENT_ID)).isNotEqualTo(modified);
+    assertThat(loadRegistrar(CLIENT_ID)).isNotEqualTo(modified);
   }
 }

javatests/google/registry/whois/DomainWhoisResponseTest.java

@@ -16,6 +16,7 @@ package google.registry.whois;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.whois.WhoisHelper.loadWhoisTestFile;
@@ -69,10 +70,7 @@ public class DomainWhoisResponseTest {
     // Update the registrar to have an IANA ID.
     Registrar registrar =
         persistResource(
-            Registrar.loadByClientId("NewRegistrar")
+            loadRegistrar("NewRegistrar").asBuilder().setIanaIdentifier(5555555L).build());
-                .asBuilder()
-                .setIanaIdentifier(5555555L)
-                .build());
 
     persistResource(
         new RegistrarContact.Builder()