Make loadByForeignKey() and related methods return Optional

This is safer and addresses a common source of confusion in the codebase because it's always explicit that the resource returned may not be present, whether because it's soft-deleted when projected to the given time or because it never existed in the first place.

In production code, the presence of the returned value is always checked. In test code, its presence is assumed using .get() where that is expected and convenient, as it not being present will throw an NPE that will cause the test to fail anyway.

Note that the roughly equivalent reloadResourceByForeignKey(), which is widely used in test code, is not having this same treatment applied to it. That is out of the scope of this CL, and has much smaller returns anyway because it's only used in tests (where the unexpected absence of a given resource would just cause the test to fail).

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

java/google/registry/tools/GetApplicationCommand.java

@@ -31,9 +31,7 @@ final class GetApplicationCommand extends GetEppResourceCommand {
 
   @Override
   public void runAndPrint() {
-    for (String applicationId : mainParameters) {
-      printResource(
-          "Application", applicationId, loadDomainApplication(applicationId, readTimestamp));
-    }
+    mainParameters.forEach(
+        appId -> printResource("Application", appId, loadDomainApplication(appId, readTimestamp)));
   }
 }

java/google/registry/tools/GetEppResourceCommand.java

@@ -21,7 +21,7 @@ import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.googlecode.objectify.Key;
 import google.registry.model.EppResource;
-import javax.annotation.Nullable;
+import java.util.Optional;
 import org.joda.time.DateTime;
 
 /** Abstract command to print one or more resources to stdout. */
@@ -44,17 +44,20 @@ abstract class GetEppResourceCommand implements CommandWithRemoteApi {
   abstract void runAndPrint();
 
   /**
-   * Prints a possibly-null resource to stdout, using resourceType and uniqueId to construct a
+   * Prints a possibly-absent resource to stdout, using resourceType and uniqueId to construct a
    * nice error message if the resource was null (i.e. doesn't exist).
    *
    * <p>The websafe key is appended to the output for use in e.g. manual mapreduce calls.
    */
-  void printResource(String resourceType, String uniqueId, @Nullable EppResource resource) {
-    System.out.println(resource != null
-        ? String.format("%s\n\nWebsafe key: %s",
-            expand ? resource.toHydratedString() : resource,
-            Key.create(resource).getString())
-        : String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId));
+  void printResource(
+      String resourceType, String uniqueId, Optional<? extends EppResource> resource) {
+    System.out.println(
+        resource.isPresent()
+            ? String.format(
+                "%s\n\nWebsafe key: %s",
+                expand ? resource.get().toHydratedString() : resource.get(),
+                Key.create(resource.get()).getString())
+            : String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId));
   }
 
   @Override

java/google/registry/tools/GetHostCommand.java

@@ -32,9 +32,7 @@ final class GetHostCommand extends GetEppResourceCommand {
 
   @Override
   public void runAndPrint() {
-    for (String hostName : mainParameters) {
-      printResource(
-          "Host", hostName, loadByForeignKey(HostResource.class, hostName, readTimestamp));
-    }
+    mainParameters.forEach(
+        h -> printResource("Host", h, loadByForeignKey(HostResource.class, h, readTimestamp)));
   }
 }

java/google/registry/tools/LockDomainCommand.java

@@ -14,9 +14,9 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameters;
@@ -28,6 +28,7 @@ import com.google.template.soy.data.SoyMapData;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.tools.soy.DomainUpdateSoyInfo;
+import java.util.Optional;
 import org.joda.time.DateTime;
 
 /**
@@ -45,10 +46,11 @@ public class LockDomainCommand extends LockOrUnlockDomainCommand {
     // Project all domains as of the same time so that argument order doesn't affect behavior.
     DateTime now = DateTime.now(UTC);
     for (String domain : getDomains()) {
-      DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now);
-      checkArgument(domainResource != null, "Domain '%s' does not exist", domain);
+      Optional<DomainResource> domainResource = loadByForeignKey(DomainResource.class, domain, now);
+      checkArgumentPresent(domainResource, "Domain '%s' does not exist or is deleted", domain);
       ImmutableSet<StatusValue> statusesToAdd =
-          Sets.difference(REGISTRY_LOCK_STATUSES, domainResource.getStatusValues()).immutableCopy();
+          Sets.difference(REGISTRY_LOCK_STATUSES, domainResource.get().getStatusValues())
+              .immutableCopy();
       if (statusesToAdd.isEmpty()) {
         logger.atInfo().log("Domain '%s' is already locked and needs no updates.", domain);
         continue;

java/google/registry/tools/RenewDomainCommand.java

@@ -17,7 +17,7 @@ package google.registry.tools;
 import static com.google.common.base.Preconditions.checkArgument;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.util.CollectionUtils.findDuplicates;
-import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -27,6 +27,7 @@ import google.registry.model.domain.DomainResource;
 import google.registry.tools.soy.RenewDomainSoyInfo;
 import google.registry.util.Clock;
 import java.util.List;
+import java.util.Optional;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
 import org.joda.time.format.DateTimeFormat;
@@ -56,9 +57,11 @@ final class RenewDomainCommand extends MutatingEppToolCommand {
     checkArgument(period < 10, "Cannot renew domains for 10 or more years");
     DateTime now = clock.nowUtc();
     for (String domainName : mainParameters) {
-      DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now);
-      checkArgumentNotNull(domain, "Domain '%s' does not exist or is deleted", domainName);
+      Optional<DomainResource> domainOptional =
+          loadByForeignKey(DomainResource.class, domainName, now);
+      checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domainName);
       setSoyTemplate(RenewDomainSoyInfo.getInstance(), RenewDomainSoyInfo.RENEWDOMAIN);
+      DomainResource domain = domainOptional.get();
       addSoyRecord(
           domain.getCurrentSponsorClientId(),
           new SoyMapData(

java/google/registry/tools/UniformRapidSuspensionCommand.java

@@ -20,6 +20,7 @@ import static com.google.common.collect.Sets.difference;
 import static google.registry.model.EppResourceUtils.checkResourcesExist;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
@@ -37,6 +38,7 @@ import google.registry.tools.soy.UniformRapidSuspensionSoyInfo;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
 import org.joda.time.DateTime;
@@ -119,17 +121,17 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
     } catch (ClassCastException | ParseException e) {
       throw new IllegalArgumentException("Invalid --dsdata JSON", e);
     }
-    DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now);
-    checkArgument(domain != null, "Domain '%s' does not exist", domainName);
+    Optional<DomainResource> domain = loadByForeignKey(DomainResource.class, domainName, now);
+    checkArgumentPresent(domain, "Domain '%s' does not exist or is deleted", domainName);
     Set<String> missingHosts =
         difference(newHostsSet, checkResourcesExist(HostResource.class, newHosts, now));
     checkArgument(missingHosts.isEmpty(), "Hosts do not exist: %s", missingHosts);
     checkArgument(
         locksToPreserve.isEmpty() || undo,
         "Locks can only be preserved when running with --undo");
-    existingNameservers = getExistingNameservers(domain);
-    existingLocks = getExistingLocks(domain);
-    existingDsData = getExistingDsData(domain);
+    existingNameservers = getExistingNameservers(domain.get());
+    existingLocks = getExistingLocks(domain.get());
+    existingDsData = getExistingDsData(domain.get());
     setSoyTemplate(
         UniformRapidSuspensionSoyInfo.getInstance(),
         UniformRapidSuspensionSoyInfo.UNIFORMRAPIDSUSPENSION);

java/google/registry/tools/UnlockDomainCommand.java

@@ -14,9 +14,9 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameters;
@@ -28,6 +28,7 @@ import com.google.template.soy.data.SoyMapData;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.tools.soy.DomainUpdateSoyInfo;
+import java.util.Optional;
 import org.joda.time.DateTime;
 
 /**
@@ -45,10 +46,10 @@ public class UnlockDomainCommand extends LockOrUnlockDomainCommand {
     // Project all domains as of the same time so that argument order doesn't affect behavior.
     DateTime now = DateTime.now(UTC);
     for (String domain : getDomains()) {
-      DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now);
-      checkArgument(domainResource != null, "Domain '%s' does not exist", domain);
+      Optional<DomainResource> domainResource = loadByForeignKey(DomainResource.class, domain, now);
+      checkArgumentPresent(domainResource, "Domain '%s' does not exist or is deleted", domain);
       ImmutableSet<StatusValue> statusesToRemove =
-          Sets.intersection(domainResource.getStatusValues(), REGISTRY_LOCK_STATUSES)
+          Sets.intersection(domainResource.get().getStatusValues(), REGISTRY_LOCK_STATUSES)
               .immutableCopy();
       if (statusesToRemove.isEmpty()) {
         logger.atInfo().log("Domain '%s' is already unlocked and needs no updates.", domain);

java/google/registry/tools/UnrenewDomainCommand.java

@@ -43,6 +43,7 @@ import google.registry.model.reporting.HistoryEntry.Type;
 import google.registry.util.Clock;
 import google.registry.util.NonFinalForTesting;
 import java.util.List;
+import java.util.Optional;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
 
@@ -90,16 +91,17 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot
         domainsNonexistentBuilder.add(domainName);
         continue;
       }
-      DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now);
-      if (domain == null || domain.getStatusValues().contains(StatusValue.PENDING_DELETE)) {
+      Optional<DomainResource> domain = loadByForeignKey(DomainResource.class, domainName, now);
+      if (!domain.isPresent()
+          || domain.get().getStatusValues().contains(StatusValue.PENDING_DELETE)) {
         domainsDeletingBuilder.add(domainName);
         continue;
       }
       domainsWithDisallowedStatusesBuilder.putAll(
-          domainName, Sets.intersection(domain.getStatusValues(), DISALLOWED_STATUSES));
+          domainName, Sets.intersection(domain.get().getStatusValues(), DISALLOWED_STATUSES));
       if (isBeforeOrAt(
-          leapSafeSubtractYears(domain.getRegistrationExpirationTime(), period), now)) {
-        domainsExpiringTooSoonBuilder.put(domainName, domain.getRegistrationExpirationTime());
+          leapSafeSubtractYears(domain.get().getRegistrationExpirationTime(), period), now)) {
+        domainsExpiringTooSoonBuilder.put(domainName, domain.get().getRegistrationExpirationTime());
       }
     }
 
@@ -149,13 +151,16 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot
   private void unrenewDomain(String domainName) {
     ofy().assertInTransaction();
     DateTime now = ofy().getTransactionTime();
-    DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now);
+    Optional<DomainResource> domainOptional =
+        loadByForeignKey(DomainResource.class, domainName, now);
     // Transactional sanity checks on the off chance that something changed between init() running
     // and here.
     checkState(
-        domain != null && !domain.getStatusValues().contains(StatusValue.PENDING_DELETE),
+        domainOptional.isPresent()
+            && !domainOptional.get().getStatusValues().contains(StatusValue.PENDING_DELETE),
         "Domain %s was deleted or is pending deletion",
         domainName);
+    DomainResource domain = domainOptional.get();
     checkState(
         Sets.intersection(domain.getStatusValues(), DISALLOWED_STATUSES).isEmpty(),
         "Domain %s has prohibited status values",

java/google/registry/tools/UpdateApplicationStatusCommand.java

@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
 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;
@@ -83,9 +82,12 @@ final class UpdateApplicationStatusCommand extends MutatingCommand {
     ofy().assertInTransaction();
     DateTime now = ofy().getTransactionTime();
 
-    // Load the domain application.
-    DomainApplication domainApplication = loadDomainApplication(applicationId, now);
-    checkArgumentNotNull(domainApplication, "Domain application does not exist");
+    DomainApplication domainApplication =
+        loadDomainApplication(applicationId, now)
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        "Domain application does not exist or is deleted"));
 
     // It's not an error if the application already has the intended status. We want the method
     // to be idempotent.

java/google/registry/tools/UpdateClaimsNoticeCommand.java

@@ -82,16 +82,20 @@ final class UpdateClaimsNoticeCommand implements CommandWithRemoteApi {
     DateTime now = ofy().getTransactionTime();
 
     // Load the domain application.
-    DomainApplication domainApplication = loadDomainApplication(applicationId, now);
-    checkArgument(domainApplication != null, "Domain application does not exist");
+    DomainApplication domainApplication =
+        loadDomainApplication(applicationId, now)
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        "Domain application does not exist or is deleted"));
 
     // Make sure this isn't a sunrise application.
     checkArgument(domainApplication.getEncodedSignedMarks().isEmpty(),
         "Can't update claims notice on sunrise applications.");
 
     // Validate the new launch notice checksum.
-    String domainLabel = InternetDomainName.from(domainApplication.getFullyQualifiedDomainName())
-        .parts().get(0);
+    String domainLabel =
+        InternetDomainName.from(domainApplication.getFullyQualifiedDomainName()).parts().get(0);
     launchNotice.validate(domainLabel);
 
     DomainApplication updatedApplication = domainApplication.asBuilder()

java/google/registry/tools/UpdateDomainCommand.java

@@ -19,7 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
 import static google.registry.model.ofy.ObjectifyService.ofy;
-import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
@@ -37,6 +37,7 @@ import google.registry.tools.soy.DomainUpdateSoyInfo;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import org.joda.time.DateTime;
@@ -172,8 +173,10 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
 
       if (!nameservers.isEmpty() || !admins.isEmpty() || !techs.isEmpty() || !statuses.isEmpty()) {
         DateTime now = DateTime.now(UTC);
-        DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now);
-        checkArgumentNotNull(domainResource, "Domain '%s' does not exist", domain);
+        Optional<DomainResource> domainOptional =
+            loadByForeignKey(DomainResource.class, domain, now);
+        checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domain);
+        DomainResource domainResource = domainOptional.get();
         checkArgument(
             !domainResource.getStatusValues().contains(SERVER_UPDATE_PROHIBITED),
             "The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed "

java/google/registry/tools/UpdateSmdCommand.java

@@ -84,11 +84,16 @@ final class UpdateSmdCommand implements CommandWithRemoteApi {
     DateTime now = ofy().getTransactionTime();
 
     // Load the domain application.
-    DomainApplication domainApplication = loadDomainApplication(applicationId, now);
-    checkArgument(domainApplication != null, "Domain application does not exist");
+    DomainApplication domainApplication =
+        loadDomainApplication(applicationId, now)
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        "Domain application does not exist or is deleted"));
 
     // Make sure this is a sunrise application.
-    checkArgument(!domainApplication.getEncodedSignedMarks().isEmpty(),
+    checkArgument(
+        !domainApplication.getEncodedSignedMarks().isEmpty(),
         "Can't update SMD on a landrush application.");
 
     // Verify the new SMD.