Create methods to administratively (un)lock domains (#494)

* Refactor DomainLockUtils methods to take a time rather than a clock

* Add administratively (un)lock methods

* Responses to CR

- Javadoc changes
- Method renames
- Variable renames

* Refactor lock methods to use JPA transaction time

* Remove clock, use Datastore transaction time

* Properly use Datastore transaction time, batched

* Continue to throw exceptions on invalid domains

* DAO writes should be in a transaction

* Assume in-transaction for all RLDao methods

* clean up test

* Fix more tests

* add comment

core/src/main/java/google/registry/model/registry/RegistryLockDao.java

@@ -31,38 +31,32 @@ public final class RegistryLockDao {
    * creation and one after verification.
    */
   public static Optional<RegistryLock> getByVerificationCode(String verificationCode) {
-    return jpaTm()
-        .transact(
-            () -> {
-              EntityManager em = jpaTm().getEntityManager();
-              Long revisionId =
-                  em.createQuery(
-                          "SELECT MAX(revisionId) FROM RegistryLock WHERE verificationCode ="
-                              + " :verificationCode",
-                          Long.class)
-                      .setParameter("verificationCode", verificationCode)
-                      .getSingleResult();
-              return Optional.ofNullable(revisionId)
-                  .map(revision -> em.find(RegistryLock.class, revision));
-            });
+    jpaTm().assertInTransaction();
+    EntityManager em = jpaTm().getEntityManager();
+    Long revisionId =
+        em.createQuery(
+                "SELECT MAX(revisionId) FROM RegistryLock WHERE verificationCode ="
+                    + " :verificationCode",
+                Long.class)
+            .setParameter("verificationCode", verificationCode)
+            .getSingleResult();
+    return Optional.ofNullable(revisionId).map(revision -> em.find(RegistryLock.class, revision));
   }
 
   /** Returns all lock objects that this registrar has created. */
   public static ImmutableList<RegistryLock> getLockedDomainsByRegistrarId(String registrarId) {
-    return jpaTm()
-        .transact(
-            () ->
-                ImmutableList.copyOf(
-                    jpaTm()
-                        .getEntityManager()
-                        .createQuery(
-                            "SELECT lock FROM RegistryLock lock WHERE"
-                                + " lock.registrarId = :registrarId "
-                                + "AND lock.lockCompletionTimestamp IS NOT NULL "
-                                + "AND lock.unlockCompletionTimestamp IS NULL",
-                            RegistryLock.class)
-                        .setParameter("registrarId", registrarId)
-                        .getResultList()));
+    jpaTm().assertInTransaction();
+    return ImmutableList.copyOf(
+        jpaTm()
+            .getEntityManager()
+            .createQuery(
+                "SELECT lock FROM RegistryLock lock WHERE"
+                    + " lock.registrarId = :registrarId "
+                    + "AND lock.lockCompletionTimestamp IS NOT NULL "
+                    + "AND lock.unlockCompletionTimestamp IS NULL",
+                RegistryLock.class)
+            .setParameter("registrarId", registrarId)
+            .getResultList());
   }
 
   /**
@@ -70,19 +64,17 @@ public final class RegistryLockDao {
    * domain hasn't been locked before.
    */
   public static Optional<RegistryLock> getMostRecentByRepoId(String repoId) {
+    jpaTm().assertInTransaction();
     return jpaTm()
-        .transact(
-            () ->
-                jpaTm()
-                    .getEntityManager()
-                    .createQuery(
-                        "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId"
-                            + " ORDER BY lock.revisionId DESC",
-                        RegistryLock.class)
-                    .setParameter("repoId", repoId)
-                    .setMaxResults(1)
-                    .getResultStream()
-                    .findFirst());
+        .getEntityManager()
+        .createQuery(
+            "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId"
+                + " ORDER BY lock.revisionId DESC",
+            RegistryLock.class)
+        .setParameter("repoId", repoId)
+        .setMaxResults(1)
+        .getResultStream()
+        .findFirst();
   }
 
   /**
@@ -91,24 +83,23 @@ public final class RegistryLockDao {
    * #getMostRecentByRepoId(String)} in that it only returns verified locks.
    */
   public static Optional<RegistryLock> getMostRecentVerifiedLockByRepoId(String repoId) {
+    jpaTm().assertInTransaction();
     return jpaTm()
-        .transact(
-            () ->
-                jpaTm()
-                    .getEntityManager()
-                    .createQuery(
-                        "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId AND"
-                            + " lock.lockCompletionTimestamp IS NOT NULL ORDER BY lock.revisionId"
-                            + " DESC",
-                        RegistryLock.class)
-                    .setParameter("repoId", repoId)
-                    .setMaxResults(1)
-                    .getResultStream()
-                    .findFirst());
+        .getEntityManager()
+        .createQuery(
+            "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId AND"
+                + " lock.lockCompletionTimestamp IS NOT NULL ORDER BY lock.revisionId"
+                + " DESC",
+            RegistryLock.class)
+        .setParameter("repoId", repoId)
+        .setMaxResults(1)
+        .getResultStream()
+        .findFirst();
   }
 
   public static RegistryLock save(RegistryLock registryLock) {
+    jpaTm().assertInTransaction();
     checkNotNull(registryLock, "Null registry lock cannot be saved");
-    return jpaTm().transact(() -> jpaTm().getEntityManager().merge(registryLock));
+    return jpaTm().getEntityManager().merge(registryLock);
   }
 }

core/src/main/java/google/registry/schema/domain/RegistryLock.java

@@ -23,7 +23,6 @@ import google.registry.model.Buildable;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
 import google.registry.model.UpdateAutoTimestamp;
-import google.registry.util.Clock;
 import google.registry.util.DateTimeUtils;
 import java.time.ZonedDateTime;
 import java.util.Optional;
@@ -185,17 +184,17 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
   }
 
   /** Returns true iff the lock was requested >= 1 hour ago and has not been verified. */
-  public boolean isLockRequestExpired(Clock clock) {
+  public boolean isLockRequestExpired(DateTime now) {
     return !getLockCompletionTimestamp().isPresent()
-        && isBeforeOrAt(getLockRequestTimestamp(), clock.nowUtc().minusHours(1));
+        && isBeforeOrAt(getLockRequestTimestamp(), now.minusHours(1));
   }
 
   /** Returns true iff the unlock was requested >= 1 hour ago and has not been verified. */
-  public boolean isUnlockRequestExpired(Clock clock) {
+  public boolean isUnlockRequestExpired(DateTime now) {
     Optional<DateTime> unlockRequestTimestamp = getUnlockRequestTimestamp();
     return unlockRequestTimestamp.isPresent()
         && !getUnlockCompletionTimestamp().isPresent()
-        && isBeforeOrAt(unlockRequestTimestamp.get(), clock.nowUtc().minusHours(1));
+        && isBeforeOrAt(unlockRequestTimestamp.get(), now.minusHours(1));
   }
 
   @Override

core/src/main/java/google/registry/tools/DomainLockUtils.java

@@ -31,12 +31,12 @@ import google.registry.model.registry.Registry;
 import google.registry.model.registry.RegistryLockDao;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.schema.domain.RegistryLock;
-import google.registry.util.Clock;
 import google.registry.util.StringGenerator;
 import java.util.Optional;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Named;
+import org.joda.time.DateTime;
 
 /**
  * Utility functions for validating and applying {@link RegistryLock}s.
@@ -56,13 +56,133 @@ public final class DomainLockUtils {
     this.stringGenerator = stringGenerator;
   }
 
-  public RegistryLock createRegistryLockRequest(
-      String domainName,
-      String registrarId,
-      @Nullable String registrarPocId,
-      boolean isAdmin,
-      Clock clock) {
-    DomainBase domainBase = getDomain(domainName, clock);
+  /**
+   * Creates and persists a lock request when requested by a user.
+   *
+   * <p>The lock will not be applied until {@link #verifyAndApplyLock} is called.
+   */
+  public RegistryLock saveNewRegistryLockRequest(
+      String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () ->
+                RegistryLockDao.save(
+                    createLockBuilder(domainName, registrarId, registrarPocId, isAdmin).build()));
+  }
+
+  /**
+   * Creates and persists an unlock request when requested by a user.
+   *
+   * <p>The unlock will not be applied until {@link #verifyAndApplyUnlock} is called.
+   */
+  public RegistryLock saveNewRegistryUnlockRequest(
+      String domainName, String registrarId, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () ->
+                RegistryLockDao.save(
+                    createUnlockBuilder(domainName, registrarId, isAdmin).build()));
+  }
+
+  /** Verifies and applies the lock request previously requested by a user. */
+  public RegistryLock verifyAndApplyLock(String verificationCode, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () -> {
+              DateTime now = jpaTm().getTransactionTime();
+              RegistryLock lock = getByVerificationCode(verificationCode);
+
+              checkArgument(
+                  !lock.getLockCompletionTimestamp().isPresent(),
+                  "Domain %s is already locked",
+                  lock.getDomainName());
+
+              checkArgument(
+                  !lock.isLockRequestExpired(now),
+                  "The pending lock has expired; please try again");
+
+              checkArgument(
+                  !lock.isSuperuser() || isAdmin, "Non-admin user cannot complete admin lock");
+
+              RegistryLock newLock =
+                  RegistryLockDao.save(lock.asBuilder().setLockCompletionTimestamp(now).build());
+              tm().transact(() -> applyLockStatuses(newLock, now));
+              return newLock;
+            });
+  }
+
+  /** Verifies and applies the unlock request previously requested by a user. */
+  public RegistryLock verifyAndApplyUnlock(String verificationCode, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () -> {
+              DateTime now = jpaTm().getTransactionTime();
+              RegistryLock lock = getByVerificationCode(verificationCode);
+              checkArgument(
+                  !lock.getUnlockCompletionTimestamp().isPresent(),
+                  "Domain %s is already unlocked",
+                  lock.getDomainName());
+
+              checkArgument(
+                  !lock.isUnlockRequestExpired(now),
+                  "The pending unlock has expired; please try again");
+
+              checkArgument(
+                  isAdmin || !lock.isSuperuser(), "Non-admin user cannot complete admin unlock");
+
+              RegistryLock newLock =
+                  RegistryLockDao.save(lock.asBuilder().setUnlockCompletionTimestamp(now).build());
+              tm().transact(() -> removeLockStatuses(newLock, isAdmin, now));
+              return newLock;
+            });
+  }
+
+  /**
+   * Creates and applies a lock in one step -- this should only be used for admin actions, e.g.
+   * Nomulus tool commands or relocks.
+   *
+   * <p>Note: in the case of relocks, isAdmin is determined by the previous lock.
+   */
+  public RegistryLock administrativelyApplyLock(
+      String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () -> {
+              DateTime now = jpaTm().getTransactionTime();
+              RegistryLock result =
+                  RegistryLockDao.save(
+                      createLockBuilder(domainName, registrarId, registrarPocId, isAdmin)
+                          .setLockCompletionTimestamp(now)
+                          .build());
+              tm().transact(() -> applyLockStatuses(result, now));
+              return result;
+            });
+  }
+
+  /**
+   * Creates and applies an unlock in one step -- this should only be used for admin actions, e.g.
+   * Nomulus tool commands.
+   */
+  public RegistryLock administrativelyApplyUnlock(
+      String domainName, String registrarId, boolean isAdmin) {
+    return jpaTm()
+        .transact(
+            () -> {
+              DateTime now = jpaTm().getTransactionTime();
+              RegistryLock result =
+                  RegistryLockDao.save(
+                      createUnlockBuilder(domainName, registrarId, isAdmin)
+                          .setUnlockCompletionTimestamp(now)
+                          .build());
+              tm().transact(() -> removeLockStatuses(result, isAdmin, now));
+              return result;
+            });
+  }
+
+  private RegistryLock.Builder createLockBuilder(
+      String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) {
+    DateTime now = jpaTm().getTransactionTime();
+    DomainBase domainBase = getDomain(domainName, now);
     verifyDomainNotLocked(domainBase);
 
     // Multiple pending actions are not allowed
@@ -70,26 +190,24 @@ public final class DomainLockUtils {
         .ifPresent(
             previousLock ->
                 checkArgument(
-                    previousLock.isLockRequestExpired(clock)
+                    previousLock.isLockRequestExpired(now)
                         || previousLock.getUnlockCompletionTimestamp().isPresent(),
                     "A pending or completed lock action already exists for %s",
                     previousLock.getDomainName()));
 
-    RegistryLock lock =
-        new RegistryLock.Builder()
-            .setVerificationCode(stringGenerator.createString(VERIFICATION_CODE_LENGTH))
-            .setDomainName(domainName)
-            .setRepoId(domainBase.getRepoId())
-            .setRegistrarId(registrarId)
-            .setRegistrarPocId(registrarPocId)
-            .isSuperuser(isAdmin)
-            .build();
-    return RegistryLockDao.save(lock);
+    return new RegistryLock.Builder()
+        .setVerificationCode(stringGenerator.createString(VERIFICATION_CODE_LENGTH))
+        .setDomainName(domainName)
+        .setRepoId(domainBase.getRepoId())
+        .setRegistrarId(registrarId)
+        .setRegistrarPocId(registrarPocId)
+        .isSuperuser(isAdmin);
   }
 
-  public RegistryLock createRegistryUnlockRequest(
-      String domainName, String registrarId, boolean isAdmin, Clock clock) {
-    DomainBase domainBase = getDomain(domainName, clock);
+  private RegistryLock.Builder createUnlockBuilder(
+      String domainName, String registrarId, boolean isAdmin) {
+    DateTime now = jpaTm().getTransactionTime();
+    DomainBase domainBase = getDomain(domainName, now);
     Optional<RegistryLock> lockOptional =
         RegistryLockDao.getMostRecentVerifiedLockByRepoId(domainBase.getRepoId());
 
@@ -105,7 +223,7 @@ public final class DomainLockUtils {
                   new RegistryLock.Builder()
                       .setRepoId(domainBase.getRepoId())
                       .setDomainName(domainName)
-                      .setLockCompletionTimestamp(clock.nowUtc())
+                      .setLockCompletionTimestamp(now)
                       .setRegistrarId(registrarId));
     } else {
       verifyDomainLocked(domainBase);
@@ -117,7 +235,7 @@ public final class DomainLockUtils {
       checkArgument(
           lock.isLocked(), "Lock object for domain %s is not currently locked", domainName);
       checkArgument(
-          !lock.getUnlockRequestTimestamp().isPresent() || lock.isUnlockRequestExpired(clock),
+          !lock.getUnlockRequestTimestamp().isPresent() || lock.isUnlockRequestExpired(now),
           "A pending unlock action already exists for %s",
           domainName);
       checkArgument(
@@ -128,65 +246,11 @@ public final class DomainLockUtils {
           !lock.isSuperuser(), "Non-admin user cannot unlock admin-locked domain %s", domainName);
       newLockBuilder = lock.asBuilder();
     }
-    RegistryLock newLock =
-        newLockBuilder
-            .setVerificationCode(stringGenerator.createString(VERIFICATION_CODE_LENGTH))
-            .isSuperuser(isAdmin)
-            .setUnlockRequestTimestamp(clock.nowUtc())
-            .setRegistrarId(registrarId)
-            .build();
-    return RegistryLockDao.save(newLock);
-  }
-
-  public RegistryLock verifyAndApplyLock(String verificationCode, boolean isAdmin, Clock clock) {
-    return jpaTm()
-        .transact(
-            () -> {
-              RegistryLock lock = getByVerificationCode(verificationCode);
-
-              checkArgument(
-                  !lock.getLockCompletionTimestamp().isPresent(),
-                  "Domain %s is already locked",
-                  lock.getDomainName());
-
-              checkArgument(
-                  !lock.isLockRequestExpired(clock),
-                  "The pending lock has expired; please try again");
-
-              checkArgument(
-                  !lock.isSuperuser() || isAdmin, "Non-admin user cannot complete admin lock");
-
-              RegistryLock newLock =
-                  RegistryLockDao.save(
-                      lock.asBuilder().setLockCompletionTimestamp(clock.nowUtc()).build());
-              tm().transact(() -> applyLockStatuses(newLock, clock));
-              return newLock;
-            });
-  }
-
-  public RegistryLock verifyAndApplyUnlock(String verificationCode, boolean isAdmin, Clock clock) {
-    return jpaTm()
-        .transact(
-            () -> {
-              RegistryLock lock = getByVerificationCode(verificationCode);
-              checkArgument(
-                  !lock.getUnlockCompletionTimestamp().isPresent(),
-                  "Domain %s is already unlocked",
-                  lock.getDomainName());
-
-              checkArgument(
-                  !lock.isUnlockRequestExpired(clock),
-                  "The pending unlock has expired; please try again");
-
-              checkArgument(
-                  isAdmin || !lock.isSuperuser(), "Non-admin user cannot complete admin unlock");
-
-              RegistryLock newLock =
-                  RegistryLockDao.save(
-                      lock.asBuilder().setUnlockCompletionTimestamp(clock.nowUtc()).build());
-              tm().transact(() -> removeLockStatuses(newLock, isAdmin, clock));
-              return newLock;
-            });
+    return newLockBuilder
+        .setVerificationCode(stringGenerator.createString(VERIFICATION_CODE_LENGTH))
+        .isSuperuser(isAdmin)
+        .setUnlockRequestTimestamp(now)
+        .setRegistrarId(registrarId);
   }
 
   private static void verifyDomainNotLocked(DomainBase domainBase) {
@@ -203,8 +267,8 @@ public final class DomainLockUtils {
         domainBase.getFullyQualifiedDomainName());
   }
 
-  private static DomainBase getDomain(String domainName, Clock clock) {
-    return loadByForeignKeyCached(DomainBase.class, domainName, clock.nowUtc())
+  private static DomainBase getDomain(String domainName, DateTime now) {
+    return loadByForeignKeyCached(DomainBase.class, domainName, now)
         .orElseThrow(
             () -> new IllegalArgumentException(String.format("Unknown domain %s", domainName)));
   }
@@ -217,8 +281,8 @@ public final class DomainLockUtils {
                     String.format("Invalid verification code %s", verificationCode)));
   }
 
-  private static void applyLockStatuses(RegistryLock lock, Clock clock) {
-    DomainBase domain = getDomain(lock.getDomainName(), clock);
+  private static void applyLockStatuses(RegistryLock lock, DateTime lockTime) {
+    DomainBase domain = getDomain(lock.getDomainName(), lockTime);
     verifyDomainNotLocked(domain);
 
     DomainBase newDomain =
@@ -227,11 +291,11 @@ public final class DomainLockUtils {
             .setStatusValues(
                 ImmutableSet.copyOf(Sets.union(domain.getStatusValues(), REGISTRY_LOCK_STATUSES)))
             .build();
-    saveEntities(newDomain, lock, clock);
+    saveEntities(newDomain, lock, lockTime, true);
   }
 
-  private static void removeLockStatuses(RegistryLock lock, boolean isAdmin, Clock clock) {
-    DomainBase domain = getDomain(lock.getDomainName(), clock);
+  private static void removeLockStatuses(RegistryLock lock, boolean isAdmin, DateTime unlockTime) {
+    DomainBase domain = getDomain(lock.getDomainName(), unlockTime);
     if (!isAdmin) {
       verifyDomainLocked(domain);
     }
@@ -243,18 +307,21 @@ public final class DomainLockUtils {
                 ImmutableSet.copyOf(
                     Sets.difference(domain.getStatusValues(), REGISTRY_LOCK_STATUSES)))
             .build();
-    saveEntities(newDomain, lock, clock);
+    saveEntities(newDomain, lock, unlockTime, false);
   }
 
-  private static void saveEntities(DomainBase domain, RegistryLock lock, Clock clock) {
-    String reason = "Lock or unlock of a domain through a RegistryLock operation";
+  private static void saveEntities(
+      DomainBase domain, RegistryLock lock, DateTime now, boolean isLock) {
+    String reason =
+        String.format(
+            "%s of a domain through a RegistryLock operation", isLock ? "Lock" : "Unlock");
     HistoryEntry historyEntry =
         new HistoryEntry.Builder()
             .setClientId(domain.getCurrentSponsorClientId())
             .setBySuperuser(lock.isSuperuser())
             .setRequestedByRegistrar(!lock.isSuperuser())
             .setType(HistoryEntry.Type.DOMAIN_UPDATE)
-            .setModificationTime(clock.nowUtc())
+            .setModificationTime(now)
             .setParent(Key.create(domain))
             .setReason(reason)
             .build();
@@ -266,8 +333,8 @@ public final class DomainLockUtils {
               .setTargetId(domain.getForeignKey())
               .setClientId(domain.getCurrentSponsorClientId())
               .setCost(Registry.get(domain.getTld()).getServerStatusChangeCost())
-              .setEventTime(clock.nowUtc())
-              .setBillingTime(clock.nowUtc())
+              .setEventTime(now)
+              .setBillingTime(now)
               .setParent(historyEntry)
               .build();
       ofy().save().entity(oneTime);

core/src/main/java/google/registry/tools/LockDomainCommand.java

@@ -22,7 +22,6 @@ import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.eppcommon.StatusValue;
-import google.registry.schema.domain.RegistryLock;
 import org.joda.time.DateTime;
 
 /**
@@ -36,35 +35,24 @@ public class LockDomainCommand extends LockOrUnlockDomainCommand {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   @Override
-  protected ImmutableSet<String> getRelevantDomains() {
-    // Project all domains as of the same time so that argument order doesn't affect behavior.
-    DateTime now = clock.nowUtc();
-    ImmutableSet.Builder<String> relevantDomains = new ImmutableSet.Builder<>();
-    for (String domain : getDomains()) {
-      DomainBase domainBase =
-          loadByForeignKey(DomainBase.class, domain, now)
-              .orElseThrow(
-                  () ->
-                      new IllegalArgumentException(
-                          String.format("Domain '%s' does not exist or is deleted", domain)));
-      ImmutableSet<StatusValue> statusesToAdd =
-          Sets.difference(REGISTRY_LOCK_STATUSES, domainBase.getStatusValues()).immutableCopy();
-      if (statusesToAdd.isEmpty()) {
-        logger.atInfo().log("Domain '%s' is already locked and needs no updates.", domain);
-        continue;
-      }
-      relevantDomains.add(domain);
+  protected boolean shouldApplyToDomain(String domain, DateTime now) {
+    DomainBase domainBase =
+        loadByForeignKey(DomainBase.class, domain, now)
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        String.format("Domain '%s' does not exist or is deleted", domain)));
+    ImmutableSet<StatusValue> statusesToAdd =
+        Sets.difference(REGISTRY_LOCK_STATUSES, domainBase.getStatusValues()).immutableCopy();
+    if (statusesToAdd.isEmpty()) {
+      logger.atInfo().log("Domain '%s' is already locked and needs no updates.", domain);
+      return false;
     }
-    return relevantDomains.build();
+    return true;
   }
 
   @Override
-  protected RegistryLock createLock(String domain) {
-    return domainLockUtils.createRegistryLockRequest(domain, clientId, null, true, clock);
-  }
-
-  @Override
-  protected void finalizeLockOrUnlockRequest(RegistryLock lock) {
-    domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
+  protected void createAndApplyRequest(String domain) {
+    domainLockUtils.administrativelyApplyLock(domain, clientId, null, true);
   }
 }

core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java

@@ -15,29 +15,33 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.Iterables.partition;
 import static google.registry.model.eppcommon.StatusValue.SERVER_DELETE_PROHIBITED;
 import static google.registry.model.eppcommon.StatusValue.SERVER_TRANSFER_PROHIBITED;
 import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.CollectionUtils.findDuplicates;
 
 import com.beust.jcommander.Parameter;
 import com.google.common.base.Joiner;
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.model.eppcommon.StatusValue;
-import google.registry.schema.domain.RegistryLock;
-import google.registry.util.Clock;
 import java.util.List;
 import javax.inject.Inject;
+import org.joda.time.DateTime;
 
-/** Shared base class for commands to registry lock or unlock a domain via EPP. */
+/**
+ * Shared base class for commands to registry lock or unlock a domain via EPP.
+ */
 public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand
     implements CommandWithRemoteApi {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
+  private static final int BATCH_SIZE = 10;
+
   public static final ImmutableSet<StatusValue> REGISTRY_LOCK_STATUSES =
       ImmutableSet.of(
           SERVER_DELETE_PROHIBITED, SERVER_TRANSFER_PROHIBITED, SERVER_UPDATE_PROHIBITED);
@@ -55,11 +59,8 @@ public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand
   @Config("registryAdminClientId")
   String registryAdminClientId;
 
-  @Inject Clock clock;
-
-  @Inject DomainLockUtils domainLockUtils;
-
-  protected ImmutableSet<String> relevantDomains = ImmutableSet.of();
+  @Inject
+  DomainLockUtils domainLockUtils;
 
   protected ImmutableSet<String> getDomains() {
     return ImmutableSet.copyOf(mainParameters);
@@ -75,34 +76,37 @@ public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand
     checkArgument(duplicates.isEmpty(), "Duplicate domain arguments found: '%s'", duplicates);
     System.out.println(
         "== ENSURE THAT YOU HAVE AUTHENTICATED THE REGISTRAR BEFORE RUNNING THIS COMMAND ==");
-    relevantDomains = getRelevantDomains();
   }
 
   @Override
   protected String execute() {
-    int failures = 0;
-    for (String domain : relevantDomains) {
-      try {
-        RegistryLock lock = createLock(domain);
-        finalizeLockOrUnlockRequest(lock);
-      } catch (Throwable t) {
-        Throwable rootCause = Throwables.getRootCause(t);
-        logger.atSevere().withCause(rootCause).log("Error when (un)locking domain %s.", domain);
-        failures++;
+    ImmutableSet.Builder<String> successfulDomainsBuilder = new ImmutableSet.Builder<>();
+    ImmutableSet.Builder<String> skippedDomainsBuilder = new ImmutableSet.Builder<>();
+    ImmutableSet.Builder<String> failedDomainsBuilder = new ImmutableSet.Builder<>();
+    partition(getDomains(), BATCH_SIZE).forEach(batch -> tm().transact(() -> {
+      for (String domain : batch) {
+        if (shouldApplyToDomain(domain, tm().getTransactionTime())) {
+          try {
+            createAndApplyRequest(domain);
+          } catch (Throwable t) {
+            logger.atSevere().withCause(t).log("Error when (un)locking domain %s.", domain);
+            failedDomainsBuilder.add(domain);
+          }
+          successfulDomainsBuilder.add(domain);
+        } else {
+          skippedDomainsBuilder.add(domain);
+        }
       }
-    }
-    if (failures == 0) {
-      return String.format("Successfully locked/unlocked %d domains.", relevantDomains.size());
-    } else {
-      return String.format(
-          "Successfully locked/unlocked %d domains with %d failures.",
-          relevantDomains.size() - failures, failures);
-    }
+    }));
+    ImmutableSet<String> successfulDomains = successfulDomainsBuilder.build();
+    ImmutableSet<String> skippedDomains = skippedDomainsBuilder.build();
+    ImmutableSet<String> failedDomains = failedDomainsBuilder.build();
+    return String.format(
+        "Successfully locked/unlocked domains:\n%s\nSkipped domains:\n%s\nFailed domains:\n%s",
+        successfulDomains, skippedDomains, failedDomains);
   }
 
-  protected abstract ImmutableSet<String> getRelevantDomains();
+  protected abstract boolean shouldApplyToDomain(String domain, DateTime now);
 
-  protected abstract RegistryLock createLock(String domain);
-
-  protected abstract void finalizeLockOrUnlockRequest(RegistryLock lock);
+  protected abstract void createAndApplyRequest(String domain);
 }

core/src/main/java/google/registry/tools/UnlockDomainCommand.java

@@ -22,7 +22,6 @@ import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.eppcommon.StatusValue;
-import google.registry.schema.domain.RegistryLock;
 import org.joda.time.DateTime;
 
 /**
@@ -36,35 +35,24 @@ public class UnlockDomainCommand extends LockOrUnlockDomainCommand {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   @Override
-  protected ImmutableSet<String> getRelevantDomains() {
-    // Project all domains as of the same time so that argument order doesn't affect behavior.
-    DateTime now = clock.nowUtc();
-    ImmutableSet.Builder<String> relevantDomains = new ImmutableSet.Builder<>();
-    for (String domain : getDomains()) {
-      DomainBase domainBase =
-          loadByForeignKey(DomainBase.class, domain, now)
-              .orElseThrow(
-                  () ->
-                      new IllegalArgumentException(
-                          String.format("Domain '%s' does not exist or is deleted", domain)));
-      ImmutableSet<StatusValue> statusesToRemove =
-          Sets.intersection(domainBase.getStatusValues(), REGISTRY_LOCK_STATUSES).immutableCopy();
-      if (statusesToRemove.isEmpty()) {
-        logger.atInfo().log("Domain '%s' is already unlocked and needs no updates.", domain);
-        continue;
-      }
-      relevantDomains.add(domain);
+  protected boolean shouldApplyToDomain(String domain, DateTime now) {
+    DomainBase domainBase =
+        loadByForeignKey(DomainBase.class, domain, now)
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        String.format("Domain '%s' does not exist or is deleted", domain)));
+    ImmutableSet<StatusValue> statusesToRemove =
+        Sets.intersection(domainBase.getStatusValues(), REGISTRY_LOCK_STATUSES).immutableCopy();
+    if (statusesToRemove.isEmpty()) {
+      logger.atInfo().log("Domain '%s' is already unlocked and needs no updates.", domain);
+      return false;
     }
-    return relevantDomains.build();
+    return true;
   }
 
   @Override
-  protected RegistryLock createLock(String domain) {
-    return domainLockUtils.createRegistryUnlockRequest(domain, clientId, true, clock);
-  }
-
-  @Override
-  protected void finalizeLockOrUnlockRequest(RegistryLock lock) {
-    domainLockUtils.verifyAndApplyUnlock(lock.getVerificationCode(), true, clock);
+  protected void createAndApplyRequest(String domain) {
+    domainLockUtils.administrativelyApplyUnlock(domain, clientId, true);
   }
 }

core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java

@@ -17,6 +17,7 @@ package google.registry.tools.javascrap;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
 
 import com.beust.jcommander.Parameter;
@@ -73,15 +74,14 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand
   @Named("base58StringGenerator")
   StringGenerator stringGenerator;
 
-  private DateTime now;
   private ImmutableList<DomainBase> lockedDomains;
 
   @Override
   protected String prompt() {
     checkArgument(
         roids != null && !roids.isEmpty(), "Must provide non-empty domain_roids argument");
-    now = clock.nowUtc();
-    lockedDomains = getLockedDomainsWithoutLocks();
+    lockedDomains =
+        jpaTm().transact(() -> getLockedDomainsWithoutLocks(jpaTm().getTransactionTime()));
     ImmutableList<String> lockedDomainNames =
         lockedDomains.stream()
             .map(DomainBase::getFullyQualifiedDomainName)
@@ -94,24 +94,30 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand
   @Override
   protected String execute() {
     ImmutableSet.Builder<DomainBase> failedDomainsBuilder = new ImmutableSet.Builder<>();
-    for (DomainBase domainBase : lockedDomains) {
-      try {
-        RegistryLockDao.save(
-            new RegistryLock.Builder()
-                .isSuperuser(true)
-                .setRegistrarId(registryAdminClientId)
-                .setRepoId(domainBase.getRepoId())
-                .setDomainName(domainBase.getFullyQualifiedDomainName())
-                .setLockCompletionTimestamp(getLockCompletionTimestamp(domainBase, now))
-                .setVerificationCode(stringGenerator.createString(VERIFICATION_CODE_LENGTH))
-                .build());
-      } catch (Throwable t) {
-        logger.atSevere().withCause(t).log(
-            "Error when creating lock object for domain %s.",
-            domainBase.getFullyQualifiedDomainName());
-        failedDomainsBuilder.add(domainBase);
-      }
-    }
+    jpaTm()
+        .transact(
+            () -> {
+              for (DomainBase domainBase : lockedDomains) {
+                try {
+                  RegistryLockDao.save(
+                      new RegistryLock.Builder()
+                          .isSuperuser(true)
+                          .setRegistrarId(registryAdminClientId)
+                          .setRepoId(domainBase.getRepoId())
+                          .setDomainName(domainBase.getFullyQualifiedDomainName())
+                          .setLockCompletionTimestamp(
+                              getLockCompletionTimestamp(domainBase, jpaTm().getTransactionTime()))
+                          .setVerificationCode(
+                              stringGenerator.createString(VERIFICATION_CODE_LENGTH))
+                          .build());
+                } catch (Throwable t) {
+                  logger.atSevere().withCause(t).log(
+                      "Error when creating lock object for domain %s.",
+                      domainBase.getFullyQualifiedDomainName());
+                  failedDomainsBuilder.add(domainBase);
+                }
+              }
+            });
     ImmutableSet<DomainBase> failedDomains = failedDomainsBuilder.build();
     if (failedDomains.isEmpty()) {
       return String.format(
@@ -136,7 +142,7 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand
         .orElse(now);
   }
 
-  private ImmutableList<DomainBase> getLockedDomainsWithoutLocks() {
+  private ImmutableList<DomainBase> getLockedDomainsWithoutLocks(DateTime now) {
     return ImmutableList.copyOf(
         ofy().load()
             .keys(

core/src/main/java/google/registry/ui/server/registrar/RegistryLockGetAction.java

@@ -16,6 +16,7 @@ package google.registry.ui.server.registrar;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.security.JsonResponseHelper.Status.SUCCESS;
 import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
@@ -151,9 +152,12 @@ public final class RegistryLockGetAction implements JsonGetAction {
 
   private ImmutableList<ImmutableMap<String, ?>> getLockedDomains(
       String clientId, boolean isAdmin) {
-    return RegistryLockDao.getLockedDomainsByRegistrarId(clientId).stream()
-        .map(lock -> lockToMap(lock, isAdmin))
-        .collect(toImmutableList());
+    return jpaTm()
+        .transact(
+            () ->
+                RegistryLockDao.getLockedDomainsByRegistrarId(clientId).stream()
+                    .map(lock -> lockToMap(lock, isAdmin))
+                    .collect(toImmutableList()));
   }
 
   private ImmutableMap<String, ?> lockToMap(RegistryLock lock, boolean isAdmin) {

core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java

@@ -43,7 +43,6 @@ import google.registry.request.auth.UserAuthInfo;
 import google.registry.schema.domain.RegistryLock;
 import google.registry.security.JsonResponseHelper;
 import google.registry.tools.DomainLockUtils;
-import google.registry.util.Clock;
 import google.registry.util.EmailMessage;
 import google.registry.util.SendEmailService;
 import java.net.URISyntaxException;
@@ -82,7 +81,6 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
   private final AuthResult authResult;
   private final AuthenticatedRegistrarAccessor registrarAccessor;
   private final SendEmailService sendEmailService;
-  private final Clock clock;
   private final DomainLockUtils domainLockUtils;
   private final InternetAddress gSuiteOutgoingEmailAddress;
 
@@ -92,14 +90,12 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
       AuthResult authResult,
       AuthenticatedRegistrarAccessor registrarAccessor,
       SendEmailService sendEmailService,
-      Clock clock,
       DomainLockUtils domainLockUtils,
       @Config("gSuiteOutgoingEmailAddress") InternetAddress gSuiteOutgoingEmailAddress) {
     this.jsonActionRunner = jsonActionRunner;
     this.authResult = authResult;
     this.registrarAccessor = registrarAccessor;
     this.sendEmailService = sendEmailService;
-    this.clock = clock;
     this.domainLockUtils = domainLockUtils;
     this.gSuiteOutgoingEmailAddress = gSuiteOutgoingEmailAddress;
   }
@@ -138,14 +134,13 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
               () -> {
                 RegistryLock registryLock =
                     postInput.isLock
-                        ? domainLockUtils.createRegistryLockRequest(
+                        ? domainLockUtils.saveNewRegistryLockRequest(
                             postInput.fullyQualifiedDomainName,
                             postInput.clientId,
                             userEmail,
-                            isAdmin,
-                            clock)
-                        : domainLockUtils.createRegistryUnlockRequest(
-                            postInput.fullyQualifiedDomainName, postInput.clientId, isAdmin, clock);
+                            isAdmin)
+                        : domainLockUtils.saveNewRegistryUnlockRequest(
+                            postInput.fullyQualifiedDomainName, postInput.clientId, isAdmin);
                 sendVerificationEmail(registryLock, userEmail, postInput.isLock);
               });
       String action = postInput.isLock ? "lock" : "unlock";

core/src/main/java/google/registry/ui/server/registrar/RegistryLockVerifyAction.java

@@ -27,7 +27,6 @@ import google.registry.schema.domain.RegistryLock;
 import google.registry.tools.DomainLockUtils;
 import google.registry.ui.server.SoyTemplateUtils;
 import google.registry.ui.soy.registrar.RegistryLockVerificationSoyInfo;
-import google.registry.util.Clock;
 import java.util.HashMap;
 import javax.inject.Inject;
 
@@ -48,18 +47,15 @@ public final class RegistryLockVerifyAction extends HtmlAction {
           google.registry.ui.soy.AnalyticsSoyInfo.getInstance(),
           google.registry.ui.soy.registrar.RegistryLockVerificationSoyInfo.getInstance());
 
-  private final Clock clock;
   private final DomainLockUtils domainLockUtils;
   private final String lockVerificationCode;
   private final Boolean isLock;
 
   @Inject
   public RegistryLockVerifyAction(
-      Clock clock,
       DomainLockUtils domainLockUtils,
       @Parameter("lockVerificationCode") String lockVerificationCode,
       @Parameter("isLock") Boolean isLock) {
-    this.clock = clock;
     this.domainLockUtils = domainLockUtils;
     this.lockVerificationCode = lockVerificationCode;
     this.isLock = isLock;
@@ -71,9 +67,9 @@ public final class RegistryLockVerifyAction extends HtmlAction {
       boolean isAdmin = authResult.userAuthInfo().get().isUserAdmin();
       final RegistryLock resultLock;
       if (isLock) {
-        resultLock = domainLockUtils.verifyAndApplyLock(lockVerificationCode, isAdmin, clock);
+        resultLock = domainLockUtils.verifyAndApplyLock(lockVerificationCode, isAdmin);
       } else {
-        resultLock = domainLockUtils.verifyAndApplyUnlock(lockVerificationCode, isAdmin, clock);
+        resultLock = domainLockUtils.verifyAndApplyUnlock(lockVerificationCode, isAdmin);
       }
       data.put("isLock", isLock);
       data.put("success", true);