diff --git a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java b/core/src/main/java/google/registry/model/registry/RegistryLockDao.java index f30d9e2bd..5d094c692 100644 --- a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java +++ b/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 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 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 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 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); } } diff --git a/core/src/main/java/google/registry/schema/domain/RegistryLock.java b/core/src/main/java/google/registry/schema/domain/RegistryLock.java index dd2ccb963..3acdf5dcf 100644 --- a/core/src/main/java/google/registry/schema/domain/RegistryLock.java +++ b/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 unlockRequestTimestamp = getUnlockRequestTimestamp(); return unlockRequestTimestamp.isPresent() && !getUnlockCompletionTimestamp().isPresent() - && isBeforeOrAt(unlockRequestTimestamp.get(), clock.nowUtc().minusHours(1)); + && isBeforeOrAt(unlockRequestTimestamp.get(), now.minusHours(1)); } @Override diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index a351b6e11..558a1c8da 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/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. + * + *

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. + * + *

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. + * + *

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 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); diff --git a/core/src/main/java/google/registry/tools/LockDomainCommand.java b/core/src/main/java/google/registry/tools/LockDomainCommand.java index 795b2ecaf..0e94cbd3e 100644 --- a/core/src/main/java/google/registry/tools/LockDomainCommand.java +++ b/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 getRelevantDomains() { - // Project all domains as of the same time so that argument order doesn't affect behavior. - DateTime now = clock.nowUtc(); - ImmutableSet.Builder 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 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 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); } } diff --git a/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java b/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java index 940077e50..80bb1af7e 100644 --- a/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java +++ b/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 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 relevantDomains = ImmutableSet.of(); + @Inject + DomainLockUtils domainLockUtils; protected ImmutableSet 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 successfulDomainsBuilder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder skippedDomainsBuilder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder 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 successfulDomains = successfulDomainsBuilder.build(); + ImmutableSet skippedDomains = skippedDomainsBuilder.build(); + ImmutableSet 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 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); } diff --git a/core/src/main/java/google/registry/tools/UnlockDomainCommand.java b/core/src/main/java/google/registry/tools/UnlockDomainCommand.java index a0415b819..fa07df0cc 100644 --- a/core/src/main/java/google/registry/tools/UnlockDomainCommand.java +++ b/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 getRelevantDomains() { - // Project all domains as of the same time so that argument order doesn't affect behavior. - DateTime now = clock.nowUtc(); - ImmutableSet.Builder 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 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 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); } } diff --git a/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java b/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java index 7457af7ad..df826a059 100644 --- a/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java +++ b/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 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 lockedDomainNames = lockedDomains.stream() .map(DomainBase::getFullyQualifiedDomainName) @@ -94,24 +94,30 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand @Override protected String execute() { ImmutableSet.Builder 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 failedDomains = failedDomainsBuilder.build(); if (failedDomains.isEmpty()) { return String.format( @@ -136,7 +142,7 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand .orElse(now); } - private ImmutableList getLockedDomainsWithoutLocks() { + private ImmutableList getLockedDomainsWithoutLocks(DateTime now) { return ImmutableList.copyOf( ofy().load() .keys( diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockGetAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockGetAction.java index 8a15ba01d..644a7315b 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockGetAction.java +++ b/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> 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 lockToMap(RegistryLock lock, boolean isAdmin) { diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java index 087a882c7..c8c5ee3b8 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java +++ b/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"; diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockVerifyAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockVerifyAction.java index 70b006961..f9342a225 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockVerifyAction.java +++ b/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); diff --git a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java index 35ddc2bc9..34d1e5ec4 100644 --- a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java +++ b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java @@ -17,6 +17,11 @@ package google.registry.model.registry; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.testing.SqlHelper.getMostRecentRegistryLockByRepoId; +import static google.registry.testing.SqlHelper.getMostRecentVerifiedRegistryLockByRepoId; +import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; +import static google.registry.testing.SqlHelper.getRegistryLocksByRegistrarId; +import static google.registry.testing.SqlHelper.saveRegistryLock; import static org.junit.Assert.assertThrows; import google.registry.persistence.transaction.JpaTestRules; @@ -45,9 +50,8 @@ public final class RegistryLockDaoTest { @Test public void testSaveAndLoad_success() { RegistryLock lock = createLock(); - RegistryLockDao.save(lock); - RegistryLock fromDatabase = - RegistryLockDao.getByVerificationCode(lock.getVerificationCode()).get(); + saveRegistryLock(lock); + RegistryLock fromDatabase = getRegistryLockByVerificationCode(lock.getVerificationCode()).get(); assertThat(fromDatabase.getDomainName()).isEqualTo(lock.getDomainName()); assertThat(fromDatabase.getVerificationCode()).isEqualTo(lock.getVerificationCode()); assertThat(fromDatabase.getLastUpdateTimestamp()).isEqualTo(fakeClock.nowUtc()); @@ -56,7 +60,7 @@ public final class RegistryLockDaoTest { @Test public void testSaveTwiceAndLoad_returnsLatest() { RegistryLock lock = createLock(); - jpaTm().transact(() -> RegistryLockDao.save(lock)); + saveRegistryLock(lock); fakeClock.advanceOneMilli(); jpaTm() .transact( @@ -80,16 +84,14 @@ public final class RegistryLockDaoTest { @Test public void testSave_load_withUnlock() { RegistryLock lock = - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .setLockCompletionTimestamp(fakeClock.nowUtc()) .setUnlockRequestTimestamp(fakeClock.nowUtc()) .setUnlockCompletionTimestamp(fakeClock.nowUtc()) .build()); - RegistryLockDao.save(lock); - RegistryLock fromDatabase = - RegistryLockDao.getByVerificationCode(lock.getVerificationCode()).get(); + RegistryLock fromDatabase = getRegistryLockByVerificationCode(lock.getVerificationCode()).get(); assertThat(fromDatabase.getUnlockRequestTimestamp()).isEqualTo(Optional.of(fakeClock.nowUtc())); assertThat(fromDatabase.getUnlockCompletionTimestamp()) .isEqualTo(Optional.of(fakeClock.nowUtc())); @@ -98,11 +100,11 @@ public final class RegistryLockDaoTest { @Test public void testUpdateLock_usingSamePrimaryKey() { - RegistryLock lock = RegistryLockDao.save(createLock()); + RegistryLock lock = saveRegistryLock(createLock()); fakeClock.advanceOneMilli(); RegistryLock updatedLock = lock.asBuilder().setLockCompletionTimestamp(fakeClock.nowUtc()).build(); - jpaTm().transact(() -> RegistryLockDao.save(updatedLock)); + saveRegistryLock(updatedLock); jpaTm() .transact( () -> { @@ -115,12 +117,12 @@ public final class RegistryLockDaoTest { @Test public void testFailure_saveNull() { - assertThrows(NullPointerException.class, () -> RegistryLockDao.save(null)); + assertThrows(NullPointerException.class, () -> saveRegistryLock(null)); } @Test public void getLock_unknownCode() { - assertThat(RegistryLockDao.getByVerificationCode("hi").isPresent()).isFalse(); + assertThat(getRegistryLockByVerificationCode("hi").isPresent()).isFalse(); } @Test @@ -141,57 +143,57 @@ public final class RegistryLockDaoTest { .setUnlockRequestTimestamp(fakeClock.nowUtc()) .setUnlockCompletionTimestamp(fakeClock.nowUtc()) .build(); - RegistryLockDao.save(lock); - RegistryLockDao.save(secondLock); - RegistryLockDao.save(unlockedLock); + saveRegistryLock(lock); + saveRegistryLock(secondLock); + saveRegistryLock(unlockedLock); assertThat( - RegistryLockDao.getLockedDomainsByRegistrarId("TheRegistrar").stream() + getRegistryLocksByRegistrarId("TheRegistrar").stream() .map(RegistryLock::getDomainName) .collect(toImmutableSet())) .containsExactly("example.test", "otherexample.test"); - assertThat(RegistryLockDao.getLockedDomainsByRegistrarId("nonexistent")).isEmpty(); + assertThat(getRegistryLocksByRegistrarId("nonexistent")).isEmpty(); } @Test public void testLoad_byRepoId() { RegistryLock completedLock = createLock().asBuilder().setLockCompletionTimestamp(fakeClock.nowUtc()).build(); - RegistryLockDao.save(completedLock); + saveRegistryLock(completedLock); fakeClock.advanceOneMilli(); RegistryLock inProgressLock = createLock(); - RegistryLockDao.save(inProgressLock); + saveRegistryLock(inProgressLock); - Optional mostRecent = RegistryLockDao.getMostRecentByRepoId("repoId"); + Optional mostRecent = getMostRecentRegistryLockByRepoId("repoId"); assertThat(mostRecent.isPresent()).isTrue(); assertThat(mostRecent.get().isLocked()).isFalse(); } @Test public void testLoad_byRepoId_empty() { - assertThat(RegistryLockDao.getMostRecentByRepoId("nonexistent").isPresent()).isFalse(); + assertThat(getMostRecentRegistryLockByRepoId("nonexistent").isPresent()).isFalse(); } @Test public void testLoad_verified_byRepoId() { RegistryLock completedLock = createLock().asBuilder().setLockCompletionTimestamp(fakeClock.nowUtc()).build(); - RegistryLockDao.save(completedLock); + saveRegistryLock(completedLock); fakeClock.advanceOneMilli(); RegistryLock inProgressLock = createLock(); - RegistryLockDao.save(inProgressLock); + saveRegistryLock(inProgressLock); - Optional mostRecent = RegistryLockDao.getMostRecentVerifiedLockByRepoId("repoId"); + Optional mostRecent = getMostRecentVerifiedRegistryLockByRepoId("repoId"); assertThat(mostRecent.isPresent()).isTrue(); assertThat(mostRecent.get().isLocked()).isTrue(); } @Test public void testLoad_verified_byRepoId_empty() { - RegistryLockDao.save(createLock()); - Optional mostRecent = RegistryLockDao.getMostRecentVerifiedLockByRepoId("repoId"); + saveRegistryLock(createLock()); + Optional mostRecent = getMostRecentVerifiedRegistryLockByRepoId("repoId"); assertThat(mostRecent.isPresent()).isFalse(); } diff --git a/core/src/test/java/google/registry/testing/SqlHelper.java b/core/src/test/java/google/registry/testing/SqlHelper.java new file mode 100644 index 000000000..5b69265a9 --- /dev/null +++ b/core/src/test/java/google/registry/testing/SqlHelper.java @@ -0,0 +1,48 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.testing; + +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.google.common.collect.ImmutableList; +import google.registry.model.registry.RegistryLockDao; +import google.registry.schema.domain.RegistryLock; +import java.util.Optional; + +/** Static utils for setting up and retrieving test resources from the SQL database. */ +public class SqlHelper { + + public static RegistryLock saveRegistryLock(RegistryLock lock) { + return jpaTm().transact(() -> RegistryLockDao.save(lock)); + } + + public static Optional getRegistryLockByVerificationCode(String verificationCode) { + return jpaTm().transact(() -> RegistryLockDao.getByVerificationCode(verificationCode)); + } + + public static Optional getMostRecentRegistryLockByRepoId(String repoId) { + return jpaTm().transact(() -> RegistryLockDao.getMostRecentByRepoId(repoId)); + } + + public static Optional getMostRecentVerifiedRegistryLockByRepoId(String repoId) { + return jpaTm().transact(() -> RegistryLockDao.getMostRecentVerifiedLockByRepoId(repoId)); + } + + public static ImmutableList getRegistryLocksByRegistrarId(String registrarId) { + return jpaTm().transact(() -> RegistryLockDao.getLockedDomainsByRegistrarId(registrarId)); + } + + private SqlHelper() {} +} diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index 84586b049..e4d2daaae 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -16,12 +16,14 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.testing.DatastoreHelper.assertNoBillingEvents; import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.getHistoryEntriesOfType; import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static org.junit.Assert.assertThrows; @@ -31,7 +33,6 @@ import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.model.registry.Registry; -import google.registry.model.registry.RegistryLockDao; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule; @@ -84,98 +85,114 @@ public final class DomainLockUtilsTest { @Test public void testSuccess_createLock() { - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); } @Test public void testSuccess_createUnlock() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); - domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); } @Test public void testSuccess_createUnlock_adminUnlockingAdmin() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock); - domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true); } @Test public void testSuccess_createLock_previousLockExpired() { - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); clock.advanceBy(Duration.standardDays(1)); - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); } @Test public void testSuccess_applyLockDomain() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); - assertThat(reloadDomain().getStatusValues()).containsExactlyElementsIn(REGISTRY_LOCK_STATUSES); - HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); - assertThat(historyEntry.getRequestedByRegistrar()).isTrue(); - assertThat(historyEntry.getBySuperuser()).isFalse(); - assertThat(historyEntry.getReason()) - .isEqualTo("Lock or unlock of a domain through a RegistryLock operation"); - assertBillingEvent(historyEntry); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); + verifyProperlyLockedDomain(false); } @Test public void testSuccess_applyUnlockDomain() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); RegistryLock unlock = - domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock); - domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock); - - assertThat(reloadDomain().getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); - ImmutableList historyEntries = - getHistoryEntriesOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); - assertThat(historyEntries.size()).isEqualTo(2); - historyEntries.forEach( - entry -> { - assertThat(entry.getRequestedByRegistrar()).isTrue(); - assertThat(entry.getBySuperuser()).isFalse(); - assertThat(entry.getReason()) - .isEqualTo("Lock or unlock of a domain through a RegistryLock operation"); - }); - assertBillingEvents(historyEntries); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); + domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false); + verifyProperlyUnlockedDomain(false); } @Test public void testSuccess_applyAdminLock_onlyHistoryEntry() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); + verifyProperlyLockedDomain(true); + } - HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); - assertThat(historyEntry.getRequestedByRegistrar()).isFalse(); - assertThat(historyEntry.getBySuperuser()).isTrue(); - DatastoreHelper.assertNoBillingEvents(); + @Test + public void testSuccess_applyAdminUnlock_onlyHistoryEntry() { + RegistryLock lock = + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); + RegistryLock unlock = + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true); + domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), true); + verifyProperlyUnlockedDomain(true); + } + + @Test + public void testSuccess_administrativelyLock_nonAdmin() { + domainLockUtils.administrativelyApplyLock( + DOMAIN_NAME, "TheRegistrar", "Marla.Singer@crr.com", false); + verifyProperlyLockedDomain(false); + } + + @Test + public void testSuccess_administrativelyLock_admin() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true); + verifyProperlyLockedDomain(true); + } + + @Test + public void testSuccess_administrativelyUnlock_nonAdmin() { + RegistryLock lock = + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); + domainLockUtils.administrativelyApplyUnlock(DOMAIN_NAME, "TheRegistrar", false); + verifyProperlyUnlockedDomain(false); + } + + @Test + public void testSuccess_administrativelyUnlock_admin() { + RegistryLock lock = + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); + domainLockUtils.administrativelyApplyUnlock(DOMAIN_NAME, "TheRegistrar", true); + verifyProperlyUnlockedDomain(true); } @Test public void testFailure_createUnlock_alreadyPendingUnlock() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); - domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); assertThat( assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryUnlockRequest( - DOMAIN_NAME, "TheRegistrar", false, clock))) + domainLockUtils.saveNewRegistryUnlockRequest( + DOMAIN_NAME, "TheRegistrar", false))) .hasMessageThat() .isEqualTo("A pending unlock action already exists for example.tld"); } @@ -183,14 +200,14 @@ public final class DomainLockUtilsTest { @Test public void testFailure_createUnlock_nonAdminUnlockingAdmin() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); assertThat( assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryUnlockRequest( - DOMAIN_NAME, "TheRegistrar", false, clock))) + domainLockUtils.saveNewRegistryUnlockRequest( + DOMAIN_NAME, "TheRegistrar", false))) .hasMessageThat() .isEqualTo("Non-admin user cannot unlock admin-locked domain example.tld"); } @@ -201,21 +218,21 @@ public final class DomainLockUtilsTest { assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryLockRequest( - "asdf.tld", "TheRegistrar", POC_ID, false, clock))) + domainLockUtils.saveNewRegistryLockRequest( + "asdf.tld", "TheRegistrar", POC_ID, false))) .hasMessageThat() .isEqualTo("Unknown domain asdf.tld"); } @Test public void testFailure_createLock_alreadyPendingLock() { - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); assertThat( assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock))) + domainLockUtils.saveNewRegistryLockRequest( + DOMAIN_NAME, "TheRegistrar", POC_ID, false))) .hasMessageThat() .isEqualTo("A pending or completed lock action already exists for example.tld"); } @@ -227,8 +244,8 @@ public final class DomainLockUtilsTest { assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock))) + domainLockUtils.saveNewRegistryLockRequest( + DOMAIN_NAME, "TheRegistrar", POC_ID, false))) .hasMessageThat() .isEqualTo("Domain example.tld is already locked"); } @@ -239,8 +256,8 @@ public final class DomainLockUtilsTest { assertThrows( IllegalArgumentException.class, () -> - domainLockUtils.createRegistryUnlockRequest( - DOMAIN_NAME, "TheRegistrar", false, clock))) + domainLockUtils.saveNewRegistryUnlockRequest( + DOMAIN_NAME, "TheRegistrar", false))) .hasMessageThat() .isEqualTo("Domain example.tld is already unlocked"); } @@ -248,14 +265,13 @@ public final class DomainLockUtilsTest { @Test public void testFailure_applyLock_alreadyApplied() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); domain = reloadDomain(); assertThat( assertThrows( IllegalArgumentException.class, - () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock))) + () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false))) .hasMessageThat() .isEqualTo("Domain example.tld is already locked"); assertNoDomainChanges(); @@ -264,13 +280,12 @@ public final class DomainLockUtilsTest { @Test public void testFailure_applyLock_expired() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); clock.advanceBy(Duration.standardDays(1)); assertThat( assertThrows( IllegalArgumentException.class, - () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock))) + () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true))) .hasMessageThat() .isEqualTo("The pending lock has expired; please try again"); assertNoDomainChanges(); @@ -279,11 +294,11 @@ public final class DomainLockUtilsTest { @Test public void testFailure_applyLock_nonAdmin_applyAdminLock() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); assertThat( assertThrows( IllegalArgumentException.class, - () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock))) + () -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false))) .hasMessageThat() .isEqualTo("Non-admin user cannot complete admin lock"); assertNoDomainChanges(); @@ -292,19 +307,16 @@ public final class DomainLockUtilsTest { @Test public void testFailure_applyUnlock_alreadyUnlocked() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); RegistryLock unlock = - domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock); - domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); + domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false); assertThat( assertThrows( IllegalArgumentException.class, - () -> - domainLockUtils.verifyAndApplyUnlock( - unlock.getVerificationCode(), false, clock))) + () -> domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false))) .hasMessageThat() .isEqualTo("Domain example.tld is already unlocked"); assertNoDomainChanges(); @@ -313,26 +325,57 @@ public final class DomainLockUtilsTest { @Test public void testFailure_applyLock_alreadyLocked() { RegistryLock lock = - domainLockUtils.createRegistryLockRequest( - DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock); + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); String verificationCode = lock.getVerificationCode(); // reload to pick up modification times, etc - lock = RegistryLockDao.getByVerificationCode(verificationCode).get(); + lock = getRegistryLockByVerificationCode(verificationCode).get(); domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); assertThat( assertThrows( IllegalArgumentException.class, - () -> domainLockUtils.verifyAndApplyLock(verificationCode, false, clock))) + () -> domainLockUtils.verifyAndApplyLock(verificationCode, false))) .hasMessageThat() .isEqualTo("Domain example.tld is already locked"); // Failure during Datastore portion shouldn't affect the SQL object - RegistryLock afterAction = - RegistryLockDao.getByVerificationCode(lock.getVerificationCode()).get(); + RegistryLock afterAction = getRegistryLockByVerificationCode(lock.getVerificationCode()).get(); assertThat(afterAction).isEqualTo(lock); assertNoDomainChanges(); } + private void verifyProperlyLockedDomain(boolean isAdmin) { + assertThat(reloadDomain().getStatusValues()).containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); + HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); + assertThat(historyEntry.getRequestedByRegistrar()).isEqualTo(!isAdmin); + assertThat(historyEntry.getBySuperuser()).isEqualTo(isAdmin); + assertThat(historyEntry.getReason()) + .isEqualTo("Lock of a domain through a RegistryLock operation"); + if (isAdmin) { + assertNoBillingEvents(); + } else { + assertBillingEvent(historyEntry); + } + } + + private void verifyProperlyUnlockedDomain(boolean isAdmin) { + assertThat(reloadDomain().getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + ImmutableList historyEntries = + getHistoryEntriesOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); + assertThat(historyEntries.size()).isEqualTo(2); + historyEntries.forEach( + entry -> { + assertThat(entry.getRequestedByRegistrar()).isEqualTo(!isAdmin); + assertThat(entry.getBySuperuser()).isEqualTo(isAdmin); + assertThat(entry.getReason()) + .contains("ock of a domain through a RegistryLock operation"); + }); + if (isAdmin) { + assertNoBillingEvents(); + } else { + assertBillingEvents(historyEntries); + } + } + private DomainBase reloadDomain() { return ofy().load().entity(domain).now(); } diff --git a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java index b087487ec..efff165a1 100644 --- a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java @@ -21,17 +21,16 @@ import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveDomain; import static google.registry.testing.DatastoreHelper.persistNewRegistrar; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getMostRecentRegistryLockByRepoId; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import google.registry.model.domain.DomainBase; import google.registry.model.registrar.Registrar.Type; -import google.registry.model.registry.RegistryLockDao; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule; import google.registry.testing.DeterministicStringGenerator; -import google.registry.testing.FakeClock; import google.registry.util.StringGenerator.Alphabets; import java.util.ArrayList; import java.util.List; @@ -52,7 +51,6 @@ public class LockDomainCommandTest extends CommandTestCase { persistNewRegistrar("adminreg", "Admin Registrar", Type.REAL, 693L); createTld("tld"); command.registryAdminClientId = "adminreg"; - command.clock = new FakeClock(); command.domainLockUtils = new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58)); } @@ -126,7 +124,7 @@ public class LockDomainCommandTest extends CommandTestCase { public void testSuccess_defaultsToAdminRegistrar_ifUnspecified() throws Exception { DomainBase domain = persistActiveDomain("example.tld"); runCommandForced("example.tld"); - assertThat(RegistryLockDao.getMostRecentByRepoId(domain.getRepoId()).get().getRegistrarId()) + assertThat(getMostRecentRegistryLockByRepoId(domain.getRepoId()).get().getRegistrarId()) .isEqualTo("adminreg"); } diff --git a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java index 191a3a044..db44e1127 100644 --- a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java @@ -22,6 +22,7 @@ import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveDomain; import static google.registry.testing.DatastoreHelper.persistNewRegistrar; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getMostRecentRegistryLockByRepoId; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static org.junit.Assert.assertThrows; @@ -29,12 +30,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.model.domain.DomainBase; import google.registry.model.registrar.Registrar.Type; -import google.registry.model.registry.RegistryLockDao; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule; import google.registry.schema.domain.RegistryLock; import google.registry.testing.DeterministicStringGenerator; -import google.registry.testing.FakeClock; import google.registry.util.StringGenerator.Alphabets; import java.util.ArrayList; import java.util.List; @@ -55,7 +54,6 @@ public class UnlockDomainCommandTest extends CommandTestCase lockOptional = RegistryLockDao.getMostRecentByRepoId(domain.getRepoId()); + Optional lockOptional = getMostRecentRegistryLockByRepoId(domain.getRepoId()); Truth8.assertThat(lockOptional).isPresent(); Truth8.assertThat(lockOptional.get().getLockCompletionTimestamp()).isPresent(); } @@ -94,7 +97,7 @@ public class BackfillRegistryLocksCommandTest previouslyLockedDomain.getRepoId(), lockedDomain.getRepoId())); - ImmutableList locks = RegistryLockDao.getLockedDomainsByRegistrarId("adminreg"); + ImmutableList locks = getRegistryLocksByRegistrarId("adminreg"); assertThat(locks).hasSize(1); assertThat(Iterables.getOnlyElement(locks).getDomainName()).isEqualTo("locked.tld"); } @@ -105,7 +108,7 @@ public class BackfillRegistryLocksCommandTest persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); fakeClock.advanceBy(Duration.standardSeconds(1)); runCommandForced("--domain_roids", domain.getRepoId()); - Truth8.assertThat(RegistryLockDao.getMostRecentByRepoId(domain.getRepoId())).isEmpty(); + Truth8.assertThat(getMostRecentRegistryLockByRepoId(domain.getRepoId())).isEmpty(); } @Test @@ -113,7 +116,7 @@ public class BackfillRegistryLocksCommandTest DomainBase domain = persistLockedDomain("example.tld"); RegistryLock previousLock = - RegistryLockDao.save( + saveRegistryLock( new RegistryLock.Builder() .isSuperuser(true) .setRegistrarId("adminreg") @@ -127,7 +130,7 @@ public class BackfillRegistryLocksCommandTest runCommandForced("--domain_roids", domain.getRepoId()); assertThat( - RegistryLockDao.getMostRecentByRepoId(domain.getRepoId()) + getMostRecentRegistryLockByRepoId(domain.getRepoId()) .get() .getLockCompletionTimestamp()) .isEqualTo(previousLock.getLockCompletionTimestamp()); @@ -154,11 +157,10 @@ public class BackfillRegistryLocksCommandTest runCommandForced( "--domain_roids", String.format("%s,%s", ursDomain.getRepoId(), nonUrsDomain.getRepoId())); - RegistryLock ursLock = - RegistryLockDao.getMostRecentVerifiedLockByRepoId(ursDomain.getRepoId()).get(); + RegistryLock ursLock = getMostRecentVerifiedRegistryLockByRepoId(ursDomain.getRepoId()).get(); assertThat(ursLock.getLockCompletionTimestamp().get()).isEqualTo(ursTime); RegistryLock nonUrsLock = - RegistryLockDao.getMostRecentVerifiedLockByRepoId(nonUrsDomain.getRepoId()).get(); + getMostRecentVerifiedRegistryLockByRepoId(nonUrsDomain.getRepoId()).get(); assertThat(nonUrsLock.getLockCompletionTimestamp().get()).isEqualTo(fakeClock.nowUtc()); } diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java index 96a1445b6..2523ea25c 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java @@ -20,6 +20,7 @@ import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.O import static google.registry.testing.AppEngineRule.makeRegistrar2; import static google.registry.testing.AppEngineRule.makeRegistrarContact3; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.saveRegistryLock; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static org.junit.Assert.assertThrows; @@ -30,7 +31,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSetMultimap; import com.google.gson.Gson; -import google.registry.model.registry.RegistryLockDao; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule; import google.registry.request.Action.Method; @@ -132,10 +132,10 @@ public final class RegistryLockGetActionTest { .setUnlockCompletionTimestamp(fakeClock.nowUtc()) .build(); - RegistryLockDao.save(regularLock); - RegistryLockDao.save(adminLock); - RegistryLockDao.save(incompleteLock); - RegistryLockDao.save(unlockedLock); + saveRegistryLock(regularLock); + saveRegistryLock(adminLock); + saveRegistryLock(incompleteLock); + saveRegistryLock(unlockedLock); action.run(); assertThat(response.getStatus()).isEqualTo(HttpStatusCodes.STATUS_CODE_OK); diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java index c0db4a95b..7336a2118 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java @@ -20,6 +20,8 @@ import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.loadRegistrar; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; +import static google.registry.testing.SqlHelper.saveRegistryLock; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -29,7 +31,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSetMultimap; import google.registry.model.domain.DomainBase; -import google.registry.model.registry.RegistryLockDao; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestRule; import google.registry.request.JsonActionRunner; @@ -71,11 +72,13 @@ public final class RegistryLockPostActionTest { + "https:\\/\\/localhost\\/registry-lock-verify\\?lockVerificationCode=" + "[0-9a-zA-Z_\\-]+&isLock=(true|false)"; + private final FakeClock clock = new FakeClock(); + @Rule public final AppEngineRule appEngineRule = AppEngineRule.builder().withDatastore().build(); @Rule public final JpaIntegrationTestRule jpaRule = - new JpaTestRules.Builder().buildIntegrationTestRule(); + new JpaTestRules.Builder().withClock(clock).buildIntegrationTestRule(); @Rule public final MockitoRule mocks = MockitoJUnit.rule(); @@ -84,7 +87,6 @@ public final class RegistryLockPostActionTest { // Marla Singer has registry lock auth permissions private final User userWithLockPermission = new User("Marla.Singer@crr.com", "gmail.com", "31337"); - private final FakeClock clock = new FakeClock(); private InternetAddress outgoingAddress; private DomainBase domain; @@ -112,8 +114,7 @@ public final class RegistryLockPostActionTest { @Test public void testSuccess_unlock() throws Exception { - RegistryLockDao.save( - createLock().asBuilder().setLockCompletionTimestamp(clock.nowUtc()).build()); + saveRegistryLock(createLock().asBuilder().setLockCompletionTimestamp(clock.nowUtc()).build()); persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); Map response = action.handleJsonRequest(unlockRequest()); assertSuccess(response, "unlock", "Marla.Singer@crr.com"); @@ -121,7 +122,7 @@ public final class RegistryLockPostActionTest { @Test public void testSuccess_unlock_adminUnlockingAdmin() throws Exception { - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .isSuperuser(true) @@ -146,7 +147,7 @@ public final class RegistryLockPostActionTest { @Test public void testFailure_unlock_alreadyUnlocked() { persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .setLockCompletionTimestamp(clock.nowUtc()) @@ -158,7 +159,7 @@ public final class RegistryLockPostActionTest { @Test public void testFailure_unlock_nonAdminUnlockingAdmin() { - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .isSuperuser(true) @@ -291,7 +292,7 @@ public final class RegistryLockPostActionTest { @Test public void testSuccess_previousLockUnlocked() throws Exception { - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .setLockCompletionTimestamp(clock.nowUtc().minusMinutes(1)) @@ -305,8 +306,9 @@ public final class RegistryLockPostActionTest { @Test public void testSuccess_previousLockExpired() throws Exception { - RegistryLock previousLock = RegistryLockDao.save(createLock()); - previousLock = RegistryLockDao.getByVerificationCode(previousLock.getVerificationCode()).get(); + RegistryLock previousLock = saveRegistryLock(createLock()); + String verificationCode = previousLock.getVerificationCode(); + previousLock = getRegistryLockByVerificationCode(verificationCode).get(); clock.setTo(previousLock.getLockRequestTimestamp().plusHours(2)); Map response = action.handleJsonRequest(lockRequest()); assertSuccess(response, "lock", "Marla.Singer@crr.com"); @@ -314,7 +316,7 @@ public final class RegistryLockPostActionTest { @Test public void testFailure_alreadyPendingLock() { - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); Map response = action.handleJsonRequest(lockRequest()); assertFailureWithMessage( response, "A pending or completed lock action already exists for example.tld"); @@ -400,7 +402,6 @@ public final class RegistryLockPostActionTest { authResult, registrarAccessor, emailService, - clock, domainLockUtils, outgoingAddress); } diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java index 827fe3b90..40b991b21 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java @@ -21,6 +21,8 @@ import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; +import static google.registry.testing.SqlHelper.saveRegistryLock; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -36,7 +38,6 @@ import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.model.registry.Registry; -import google.registry.model.registry.RegistryLockDao; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule; @@ -104,7 +105,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testSuccess_lockDomain() { - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); assertThat(reloadDomain().getStatusValues()).containsExactlyElementsIn(REGISTRY_LOCK_STATUSES); @@ -113,7 +114,7 @@ public final class RegistryLockVerifyActionTest { assertThat(historyEntry.getRequestedByRegistrar()).isTrue(); assertThat(historyEntry.getBySuperuser()).isFalse(); assertThat(historyEntry.getReason()) - .isEqualTo("Lock or unlock of a domain through a RegistryLock operation"); + .isEqualTo("Lock of a domain through a RegistryLock operation"); assertBillingEvent(historyEntry); } @@ -121,7 +122,7 @@ public final class RegistryLockVerifyActionTest { public void testSuccess_unlockDomain() { action = createAction(lockId, false); domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); - RegistryLockDao.save( + saveRegistryLock( createLock().asBuilder().setUnlockRequestTimestamp(fakeClock.nowUtc()).build()); action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); @@ -131,14 +132,14 @@ public final class RegistryLockVerifyActionTest { assertThat(historyEntry.getRequestedByRegistrar()).isTrue(); assertThat(historyEntry.getBySuperuser()).isFalse(); assertThat(historyEntry.getReason()) - .isEqualTo("Lock or unlock of a domain through a RegistryLock operation"); + .isEqualTo("Unlock of a domain through a RegistryLock operation"); assertBillingEvent(historyEntry); } @Test public void testSuccess_adminLock_createsOnlyHistoryEntry() { action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, true)); - RegistryLockDao.save(createLock().asBuilder().isSuperuser(true).build()); + saveRegistryLock(createLock().asBuilder().isSuperuser(true).build()); action.run(); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); @@ -149,7 +150,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_badVerificationCode() { - RegistryLockDao.save( + saveRegistryLock( createLock().asBuilder().setVerificationCode("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA").build()); action.run(); assertThat(response.getPayload()).contains("Failed: Invalid verification code"); @@ -158,7 +159,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_alreadyVerified() { - RegistryLockDao.save( + saveRegistryLock( createLock().asBuilder().setLockCompletionTimestamp(fakeClock.nowUtc()).build()); action.run(); assertThat(response.getPayload()).contains("Failed: Domain example.tld is already locked"); @@ -167,7 +168,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_expired() { - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); fakeClock.advanceBy(Duration.standardHours(2)); action.run(); assertThat(response.getPayload()) @@ -177,7 +178,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_nonAdmin_verifyingAdminLock() { - RegistryLockDao.save(createLock().asBuilder().isSuperuser(true).build()); + saveRegistryLock(createLock().asBuilder().isSuperuser(true).build()); action.run(); assertThat(response.getPayload()).contains("Failed: Non-admin user cannot complete admin lock"); assertNoDomainChanges(); @@ -186,7 +187,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_alreadyUnlocked() { action = createAction(lockId, false); - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .setLockCompletionTimestamp(fakeClock.nowUtc()) @@ -200,7 +201,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_alreadyLocked() { - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); action.run(); assertThat(response.getPayload()).contains("Failed: Domain example.tld is already locked"); @@ -221,9 +222,9 @@ public final class RegistryLockVerifyActionTest { // A failure when performing Datastore actions means that no actions should be taken in the // Cloud SQL RegistryLock object RegistryLock lock = createLock(); - RegistryLockDao.save(lock); + saveRegistryLock(lock); // reload the lock to pick up creation time - lock = RegistryLockDao.getByVerificationCode(lock.getVerificationCode()).get(); + lock = getRegistryLockByVerificationCode(lock.getVerificationCode()).get(); fakeClock.advanceOneMilli(); domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); action.run(); @@ -231,15 +232,14 @@ public final class RegistryLockVerifyActionTest { assertThat(response.getPayload()).contains("Failed: Domain example.tld is already locked"); // verify that the changes to the SQL object were rolled back - RegistryLock afterAction = - RegistryLockDao.getByVerificationCode(lock.getVerificationCode()).get(); + RegistryLock afterAction = getRegistryLockByVerificationCode(lock.getVerificationCode()).get(); assertThat(afterAction).isEqualTo(lock); } @Test public void testFailure_isLockTrue_shouldBeFalse() { domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); - RegistryLockDao.save( + saveRegistryLock( createLock() .asBuilder() .setLockCompletionTimestamp(fakeClock.nowUtc()) @@ -252,18 +252,18 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_isLockFalse_shouldBeTrue() { action = createAction(lockId, false); - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); action.run(); assertThat(response.getPayload()).contains("Failed: Domain example.tld is already unlocked"); } @Test public void testFailure_lock_unlock_lockAgain() { - RegistryLock lock = RegistryLockDao.save(createLock()); + RegistryLock lock = saveRegistryLock(createLock()); action.run(); assertThat(response.getPayload()).contains("Success: lock has been applied to example.tld"); String unlockVerificationCode = "some-unlock-code"; - RegistryLockDao.save( + saveRegistryLock( lock.asBuilder() .setVerificationCode(unlockVerificationCode) .setUnlockRequestTimestamp(fakeClock.nowUtc()) @@ -278,7 +278,7 @@ public final class RegistryLockVerifyActionTest { @Test public void testFailure_lock_lockAgain() { - RegistryLockDao.save(createLock()); + saveRegistryLock(createLock()); action.run(); assertThat(response.getPayload()).contains("Success: lock has been applied to example.tld"); action = createAction(lockId, true); @@ -290,7 +290,7 @@ public final class RegistryLockVerifyActionTest { public void testFailure_unlock_unlockAgain() { action = createAction(lockId, false); domain = persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); - RegistryLockDao.save( + saveRegistryLock( createLock().asBuilder().setUnlockRequestTimestamp(fakeClock.nowUtc()).build()); action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); @@ -336,7 +336,7 @@ public final class RegistryLockVerifyActionTest { response = new FakeResponse(); RegistryLockVerifyAction action = new RegistryLockVerifyAction( - fakeClock, new DomainLockUtils(stringGenerator), lockVerificationCode, isLock); + new DomainLockUtils(stringGenerator), lockVerificationCode, isLock); authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); action.req = request; action.response = response; diff --git a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java index e10fc7366..1a3f1ea5f 100644 --- a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java +++ b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java @@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.loadRegistrar; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveDomain; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.saveRegistryLock; import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.collect.ImmutableMap; @@ -30,7 +31,6 @@ import com.googlecode.objectify.ObjectifyFilter; import google.registry.model.domain.DomainBase; import google.registry.model.ofy.OfyFilter; import google.registry.model.registrar.Registrar.State; -import google.registry.model.registry.RegistryLockDao; import google.registry.module.frontend.FrontendServlet; import google.registry.schema.domain.RegistryLock; import google.registry.server.RegistryTestServer; @@ -388,7 +388,7 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase { () -> { createTld("tld"); persistResource(newDomainBase("example.tld")); - RegistryLockDao.save( + saveRegistryLock( new RegistryLock.Builder() .setRegistrarPocId("johndoe@theregistrar.com") .setRepoId("repoId") @@ -436,7 +436,7 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase { public void registryLock_nonEmpty() throws Throwable { server.runInAppEngineEnvironment( () -> { - saveRegistryLock(); + createDomainAndSaveLock(); return null; }); driver.get(server.getUrl("/registrar#registry-lock")); @@ -450,9 +450,9 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase { () -> { createTld("tld"); DomainBase domain = persistActiveDomain("example.tld"); - RegistryLockDao.save(createRegistryLock(domain).asBuilder().isSuperuser(true).build()); + saveRegistryLock(createRegistryLock(domain).asBuilder().isSuperuser(true).build()); DomainBase otherDomain = persistActiveDomain("otherexample.tld"); - RegistryLockDao.save(createRegistryLock(otherDomain)); + saveRegistryLock(createRegistryLock(otherDomain)); return null; }); driver.get(server.getUrl("/registrar#registry-lock")); @@ -465,7 +465,7 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase { server.setIsAdmin(true); server.runInAppEngineEnvironment( () -> { - saveRegistryLock(); + createDomainAndSaveLock(); return null; }); driver.get(server.getUrl("/registrar#registry-lock")); @@ -510,10 +510,10 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase { driver.diffPage("page"); } - private void saveRegistryLock() { + private void createDomainAndSaveLock() { createTld("tld"); DomainBase domainBase = persistActiveDomain("example.tld"); - RegistryLockDao.save(createRegistryLock(domainBase)); + saveRegistryLock(createRegistryLock(domainBase)); } private RegistryLock createRegistryLock(DomainBase domainBase) {