Add unlock fields to RegistryLocks (#408)

* Add unlock fields to RegistryLocks

This will make it easier to reason around inter-connected registry lock
objects (like when we add dependent roids). It will make it easier to
answer the question of "Have all locks associated with this host/contact
roid been unlocked?", as well as the question of "Was the last lock
object associated with this domain unlocked?"

* Responses to CR

* Make the DAO API more specific

* whoops, undo rename

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

@@ -49,7 +49,7 @@ public final class RegistryLockDao {
   }
 
   /** Returns all lock objects that this registrar has created. */
-  public static ImmutableList<RegistryLock> getByRegistrarId(String registrarId) {
+  public static ImmutableList<RegistryLock> getLockedDomainsByRegistrarId(String registrarId) {
     return jpaTm()
         .transact(
             () ->
@@ -58,7 +58,9 @@ public final class RegistryLockDao {
                         .getEntityManager()
                         .createQuery(
                             "SELECT lock FROM RegistryLock lock WHERE"
-                                + " lock.registrarId = :registrarId",
+                                + " lock.registrarId = :registrarId "
+                                + "AND lock.lockCompletionTimestamp IS NOT NULL "
+                                + "AND lock.unlockCompletionTimestamp IS NULL",
                             RegistryLock.class)
                         .setParameter("registrarId", registrarId)
                         .getResultList()));

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

@@ -21,13 +21,12 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import google.registry.model.Buildable;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
+import google.registry.model.UpdateAutoTimestamp;
 import google.registry.util.DateTimeUtils;
 import java.time.ZonedDateTime;
 import java.util.Optional;
 import javax.persistence.Column;
 import javax.persistence.Entity;
-import javax.persistence.EnumType;
-import javax.persistence.Enumerated;
 import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
@@ -70,12 +69,6 @@ import org.joda.time.DateTime;
     })
 public final class RegistryLock extends ImmutableObject implements Buildable {
 
-  /** Describes the action taken by the user. */
-  public enum Action {
-    LOCK,
-    UNLOCK
-  }
-
   @Id
   @GeneratedValue(strategy = GenerationType.IDENTITY)
   @Column(nullable = false)
@@ -99,28 +92,26 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
   /** The POC that performed the action, or null if it was a superuser. */
   private String registrarPocId;
 
-  /**
-   * Lock action is immutable and describes whether the action performed was a lock or an unlock.
-   */
-  @Enumerated(EnumType.STRING)
+  /** When the lock is first requested. */
   @Column(nullable = false)
-  private Action action;
+  private CreateAutoTimestamp lockRequestTimestamp = CreateAutoTimestamp.create(null);
 
-  /** Creation timestamp is when the lock/unlock is first requested. */
-  @Column(nullable = false)
-  private CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
+  /** When the unlock is first requested. */
+  private ZonedDateTime unlockRequestTimestamp;
 
   /**
-   * Completion timestamp is when the user has verified the lock/unlock, when this object de facto
-   * becomes immutable. If this field is null, it means that the lock has not been verified yet (and
-   * thus not been put into effect).
+   * When the user has verified the lock. If this field is null, it means the lock has not been
+   * verified yet (and thus not been put into effect).
    */
-  private ZonedDateTime completionTimestamp;
+  private ZonedDateTime lockCompletionTimestamp;
 
   /**
-   * The user must provide the random verification code in order to complete the lock and move the
-   * status from PENDING to COMPLETED.
+   * When the user has verified the unlock of this lock. If this field is null, it means the unlock
+   * action has not been verified yet (and has not been put into effect).
    */
+  private ZonedDateTime unlockCompletionTimestamp;
+
+  /** The user must provide the random verification code in order to complete the action. */
   @Column(nullable = false)
   private String verificationCode;
 
@@ -131,6 +122,9 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
   @Column(nullable = false)
   private boolean isSuperuser;
 
+  /** Time that this entity was last updated. */
+  private UpdateAutoTimestamp lastUpdateTimestamp;
+
   public String getRepoId() {
     return repoId;
   }
@@ -147,17 +141,25 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
     return registrarPocId;
   }
 
-  public Action getAction() {
-    return action;
+  public DateTime getLockRequestTimestamp() {
+    return lockRequestTimestamp.getTimestamp();
   }
 
-  public DateTime getCreationTimestamp() {
-    return creationTimestamp.getTimestamp();
+  /** Returns the unlock request timestamp or null if an unlock has not been requested yet. */
+  public Optional<DateTime> getUnlockRequestTimestamp() {
+    return Optional.ofNullable(unlockRequestTimestamp).map(DateTimeUtils::toJodaDateTime);
   }
 
   /** Returns the completion timestamp, or empty if this lock has not been completed yet. */
-  public Optional<DateTime> getCompletionTimestamp() {
-    return Optional.ofNullable(completionTimestamp).map(DateTimeUtils::toJodaDateTime);
+  public Optional<DateTime> getLockCompletionTimestamp() {
+    return Optional.ofNullable(lockCompletionTimestamp).map(DateTimeUtils::toJodaDateTime);
+  }
+
+  /**
+   * Returns the unlock completion timestamp, or empty if this unlock has not been completed yet.
+   */
+  public Optional<DateTime> getUnlockCompletionTimestamp() {
+    return Optional.ofNullable(unlockCompletionTimestamp).map(DateTimeUtils::toJodaDateTime);
   }
 
   public String getVerificationCode() {
@@ -168,16 +170,16 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
     return isSuperuser;
   }
 
+  public DateTime getLastUpdateTimestamp() {
+    return lastUpdateTimestamp.getTimestamp();
+  }
+
   public Long getRevisionId() {
     return revisionId;
   }
 
-  public void setCompletionTimestamp(DateTime dateTime) {
-    this.completionTimestamp = toZonedDateTime(dateTime);
-  }
-
-  public boolean isVerified() {
-    return completionTimestamp != null;
+  public boolean isLocked() {
+    return lockCompletionTimestamp != null && unlockCompletionTimestamp == null;
   }
 
   @Override
@@ -198,7 +200,6 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
       checkArgumentNotNull(getInstance().repoId, "Repo ID cannot be null");
       checkArgumentNotNull(getInstance().domainName, "Domain name cannot be null");
       checkArgumentNotNull(getInstance().registrarId, "Registrar ID cannot be null");
-      checkArgumentNotNull(getInstance().action, "Action cannot be null");
       checkArgumentNotNull(getInstance().verificationCode, "Verification code cannot be null");
       checkArgument(
           getInstance().registrarPocId != null || getInstance().isSuperuser,
@@ -226,18 +227,18 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
       return this;
     }
 
-    public Builder setAction(Action action) {
-      getInstance().action = action;
+    public Builder setUnlockRequestTimestamp(DateTime unlockRequestTimestamp) {
+      getInstance().unlockRequestTimestamp = toZonedDateTime(unlockRequestTimestamp);
       return this;
     }
 
-    public Builder setCreationTimestamp(CreateAutoTimestamp creationTimestamp) {
-      getInstance().creationTimestamp = creationTimestamp;
+    public Builder setLockCompletionTimestamp(DateTime lockCompletionTimestamp) {
+      getInstance().lockCompletionTimestamp = toZonedDateTime(lockCompletionTimestamp);
       return this;
     }
 
-    public Builder setCompletionTimestamp(DateTime lockTimestamp) {
-      getInstance().completionTimestamp = toZonedDateTime(lockTimestamp);
+    public Builder setUnlockCompletionTimestamp(DateTime unlockCompletionTimestamp) {
+      getInstance().unlockCompletionTimestamp = toZonedDateTime(unlockCompletionTimestamp);
       return this;
     }
 

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

@@ -149,11 +149,9 @@ public final class RegistryLockGetAction implements JsonGetAction {
   }
 
   private ImmutableList<ImmutableMap<String, ?>> getLockedDomains(String clientId) {
-    ImmutableList<RegistryLock> locks =
-        RegistryLockDao.getByRegistrarId(clientId).stream()
-            .filter(RegistryLock::isVerified)
+    return RegistryLockDao.getLockedDomainsByRegistrarId(clientId).stream()
+        .map(this::lockToMap)
         .collect(toImmutableList());
-    return locks.stream().map(this::lockToMap).collect(toImmutableList());
   }
 
   private ImmutableMap<String, ?> lockToMap(RegistryLock lock) {
@@ -161,7 +159,7 @@ public final class RegistryLockGetAction implements JsonGetAction {
         FULLY_QUALIFIED_DOMAIN_NAME_PARAM,
         lock.getDomainName(),
         LOCKED_TIME_PARAM,
-        lock.getCompletionTimestamp().map(DateTime::toString).orElse(""),
+        lock.getLockCompletionTimestamp().map(DateTime::toString).orElse(""),
         LOCKED_BY_PARAM,
         lock.isSuperuser() ? "admin" : lock.getRegistrarPocId());
   }

core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java

@@ -22,7 +22,6 @@ import static google.registry.testing.JUnitBackports.assertThrows;
 import google.registry.model.transaction.JpaTestRules;
 import google.registry.model.transaction.JpaTestRules.JpaIntegrationTestRule;
 import google.registry.schema.domain.RegistryLock;
-import google.registry.schema.domain.RegistryLock.Action;
 import google.registry.testing.AppEngineRule;
 import java.util.Optional;
 import java.util.UUID;
@@ -48,6 +47,7 @@ public final class RegistryLockDaoTest {
     RegistryLock fromDatabase = RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
     assertThat(fromDatabase.getDomainName()).isEqualTo(lock.getDomainName());
     assertThat(fromDatabase.getVerificationCode()).isEqualTo(lock.getVerificationCode());
+    assertThat(fromDatabase.getLastUpdateTimestamp()).isEqualTo(jpaRule.getTxnClock().nowUtc());
   }
 
   @Test
@@ -71,32 +71,56 @@ public final class RegistryLockDaoTest {
             () -> {
               RegistryLock updatedLock =
                   RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
-              updatedLock.setCompletionTimestamp(jpaRule.getTxnClock().nowUtc());
-              RegistryLockDao.save(updatedLock);
+              RegistryLockDao.save(
+                  updatedLock
+                      .asBuilder()
+                      .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+                      .build());
             });
     jpaTm()
         .transact(
             () -> {
               RegistryLock fromDatabase =
                   RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
-              assertThat(fromDatabase.getCompletionTimestamp().get())
+              assertThat(fromDatabase.getLockCompletionTimestamp().get())
+                  .isEqualTo(jpaRule.getTxnClock().nowUtc());
+              assertThat(fromDatabase.getLastUpdateTimestamp())
                   .isEqualTo(jpaRule.getTxnClock().nowUtc());
             });
   }
 
+  @Test
+  public void testSave_load_withUnlock() {
+    RegistryLock lock =
+        RegistryLockDao.save(
+            createLock()
+                .asBuilder()
+                .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+                .setUnlockRequestTimestamp(jpaRule.getTxnClock().nowUtc())
+                .setUnlockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+                .build());
+    RegistryLockDao.save(lock);
+    RegistryLock fromDatabase = RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
+    assertThat(fromDatabase.getUnlockRequestTimestamp())
+        .isEqualTo(Optional.of(jpaRule.getTxnClock().nowUtc()));
+    assertThat(fromDatabase.getUnlockCompletionTimestamp())
+        .isEqualTo(Optional.of(jpaRule.getTxnClock().nowUtc()));
+    assertThat(fromDatabase.isLocked()).isFalse();
+  }
+
   @Test
   public void testUpdateLock_usingSamePrimaryKey() {
     RegistryLock lock = RegistryLockDao.save(createLock());
     jpaRule.getTxnClock().advanceOneMilli();
     RegistryLock updatedLock =
-        lock.asBuilder().setCompletionTimestamp(jpaRule.getTxnClock().nowUtc()).build();
+        lock.asBuilder().setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc()).build();
     jpaTm().transact(() -> RegistryLockDao.save(updatedLock));
     jpaTm()
         .transact(
             () -> {
               RegistryLock fromDatabase =
                   RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
-              assertThat(fromDatabase.getCompletionTimestamp())
+              assertThat(fromDatabase.getLockCompletionTimestamp())
                   .isEqualTo(Optional.of(jpaRule.getTxnClock().nowUtc()));
             });
   }
@@ -107,24 +131,39 @@ public final class RegistryLockDaoTest {
   }
 
   @Test
-  public void testLoad_byRegistrarId() {
-    RegistryLock lock = createLock();
-    RegistryLock secondLock = createLock().asBuilder().setDomainName("otherexample.test").build();
+  public void testLoad_lockedDomains_byRegistrarId() {
+    RegistryLock lock =
+        createLock().asBuilder().setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc()).build();
+    RegistryLock secondLock =
+        createLock()
+            .asBuilder()
+            .setDomainName("otherexample.test")
+            .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .build();
+    RegistryLock unlockedLock =
+        createLock()
+            .asBuilder()
+            .setDomainName("unlocked.test")
+            .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setUnlockRequestTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setUnlockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .build();
     RegistryLockDao.save(lock);
     RegistryLockDao.save(secondLock);
+    RegistryLockDao.save(unlockedLock);
 
     assertThat(
-            RegistryLockDao.getByRegistrarId("TheRegistrar").stream()
+            RegistryLockDao.getLockedDomainsByRegistrarId("TheRegistrar").stream()
                 .map(RegistryLock::getDomainName)
                 .collect(toImmutableSet()))
         .containsExactly("example.test", "otherexample.test");
-    assertThat(RegistryLockDao.getByRegistrarId("nonexistent")).isEmpty();
+    assertThat(RegistryLockDao.getLockedDomainsByRegistrarId("nonexistent")).isEmpty();
   }
 
   @Test
   public void testLoad_byRepoId() {
     RegistryLock completedLock =
-        createLock().asBuilder().setCompletionTimestamp(jpaRule.getTxnClock().nowUtc()).build();
+        createLock().asBuilder().setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc()).build();
     RegistryLockDao.save(completedLock);
 
     jpaRule.getTxnClock().advanceOneMilli();
@@ -133,7 +172,7 @@ public final class RegistryLockDaoTest {
 
     Optional<RegistryLock> mostRecent = RegistryLockDao.getMostRecentByRepoId("repoId");
     assertThat(mostRecent.isPresent()).isTrue();
-    assertThat(mostRecent.get().isVerified()).isFalse();
+    assertThat(mostRecent.get().isLocked()).isFalse();
   }
 
   @Test
@@ -146,7 +185,6 @@ public final class RegistryLockDaoTest {
         .setRepoId("repoId")
         .setDomainName("example.test")
         .setRegistrarId("TheRegistrar")
-        .setAction(Action.LOCK)
         .setVerificationCode(UUID.randomUUID().toString())
         .isSuperuser(true)
         .build();

core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java

@@ -38,7 +38,6 @@ import google.registry.request.auth.AuthResult;
 import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
 import google.registry.schema.domain.RegistryLock;
-import google.registry.schema.domain.RegistryLock.Action;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeResponse;
 import java.util.Map;
@@ -95,10 +94,9 @@ public final class RegistryLockGetActionTest {
             .setRepoId("repoId")
             .setDomainName("example.test")
             .setRegistrarId("TheRegistrar")
-            .setAction(Action.LOCK)
             .setVerificationCode(UUID.randomUUID().toString())
             .setRegistrarPocId("johndoe@theregistrar.com")
-            .setCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
             .build();
     jpaRule.getTxnClock().advanceOneMilli();
     RegistryLock adminLock =
@@ -106,24 +104,35 @@ public final class RegistryLockGetActionTest {
             .setRepoId("repoId")
             .setDomainName("adminexample.test")
             .setRegistrarId("TheRegistrar")
-            .setAction(Action.LOCK)
             .setVerificationCode(UUID.randomUUID().toString())
             .isSuperuser(true)
-            .setCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
             .build();
     RegistryLock incompleteLock =
         new RegistryLock.Builder()
             .setRepoId("repoId")
             .setDomainName("incomplete.test")
             .setRegistrarId("TheRegistrar")
-            .setAction(Action.LOCK)
             .setVerificationCode(UUID.randomUUID().toString())
             .setRegistrarPocId("johndoe@theregistrar.com")
             .build();
 
+    RegistryLock unlockedLock =
+        new RegistryLock.Builder()
+            .setRepoId("repoId")
+            .setDomainName("unlocked.test")
+            .setRegistrarId("TheRegistrar")
+            .setRegistrarPocId("johndoe@theregistrar.com")
+            .setVerificationCode(UUID.randomUUID().toString())
+            .setLockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setUnlockRequestTimestamp(jpaRule.getTxnClock().nowUtc())
+            .setUnlockCompletionTimestamp(jpaRule.getTxnClock().nowUtc())
+            .build();
+
     RegistryLockDao.save(regularLock);
     RegistryLockDao.save(adminLock);
     RegistryLockDao.save(incompleteLock);
+    RegistryLockDao.save(unlockedLock);
 
     action.run();
     assertThat(response.getStatus()).isEqualTo(HttpStatusCodes.STATUS_CODE_OK);
@@ -134,9 +143,12 @@ public final class RegistryLockGetActionTest {
             "results",
                 ImmutableList.of(
                     ImmutableMap.of(
-                        "lockEnabledForContact", true,
-                        "email", "Marla.Singer@crr.com",
-                        "clientId", "TheRegistrar",
+                        "lockEnabledForContact",
+                        true,
+                        "email",
+                        "Marla.Singer@crr.com",
+                        "clientId",
+                        "TheRegistrar",
                         "locks",
                         ImmutableList.of(
                             ImmutableMap.of(

db/src/main/resources/sql/flyway/V13__refactor_registry_lock.sql

@@ -0,0 +1,22 @@
+-- Copyright 2019 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.
+
+ALTER TABLE "RegistryLock" DROP COLUMN action;
+
+ALTER TABLE "RegistryLock" ADD COLUMN unlock_request_timestamp timestamptz;
+ALTER TABLE "RegistryLock" ADD COLUMN unlock_completion_timestamp timestamptz;
+ALTER TABLE "RegistryLock" ADD COLUMN last_update_timestamp timestamptz;
+
+ALTER TABLE "RegistryLock" RENAME creation_timestamp TO lock_request_timestamp;
+ALTER TABLE "RegistryLock" RENAME completion_timestamp TO lock_completion_timestamp;

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -148,14 +148,16 @@
 
     create table "RegistryLock" (
        revision_id  bigserial not null,
-        action text not null,
-        completion_timestamp timestamptz,
-        creation_timestamp timestamptz not null,
         domain_name text not null,
         is_superuser boolean not null,
+        last_update_timestamp timestamptz,
+        lock_completion_timestamp timestamptz,
+        lock_request_timestamp timestamptz not null,
         registrar_id text not null,
         registrar_poc_id text,
         repo_id text not null,
+        unlock_completion_timestamp timestamptz,
+        unlock_request_timestamp timestamptz,
         verification_code text not null,
         primary key (revision_id)
     );

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -122,15 +122,17 @@ ALTER SEQUENCE public."PremiumList_revision_id_seq" OWNED BY public."PremiumList
 
 CREATE TABLE public."RegistryLock" (
     revision_id bigint NOT NULL,
-    action text NOT NULL,
-    completion_timestamp timestamp with time zone,
-    creation_timestamp timestamp with time zone NOT NULL,
+    lock_completion_timestamp timestamp with time zone,
+    lock_request_timestamp timestamp with time zone NOT NULL,
     domain_name text NOT NULL,
     is_superuser boolean NOT NULL,
     registrar_id text NOT NULL,
     registrar_poc_id text,
     repo_id text NOT NULL,
-    verification_code text NOT NULL
+    verification_code text NOT NULL,
+    unlock_request_timestamp timestamp with time zone,
+    unlock_completion_timestamp timestamp with time zone,
+    last_update_timestamp timestamp with time zone
 );