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 5852b858c..b9c447b56 100644 --- a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java +++ b/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 getByRegistrarId(String registrarId) { + public static ImmutableList 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())); 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 84e2090f4..2ce84a139 100644 --- a/core/src/main/java/google/registry/schema/domain/RegistryLock.java +++ b/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 getUnlockRequestTimestamp() { + return Optional.ofNullable(unlockRequestTimestamp).map(DateTimeUtils::toJodaDateTime); } /** Returns the completion timestamp, or empty if this lock has not been completed yet. */ - public Optional getCompletionTimestamp() { - return Optional.ofNullable(completionTimestamp).map(DateTimeUtils::toJodaDateTime); + public Optional 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 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,8 +200,7 @@ 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 codecannot be null"); + checkArgumentNotNull(getInstance().verificationCode, "Verification code cannot be null"); checkArgument( getInstance().registrarPocId != null || getInstance().isSuperuser, "Registrar POC ID must be provided if superuser is false"); @@ -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; } 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 7ab0ba959..545dd1930 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 @@ -149,11 +149,9 @@ public final class RegistryLockGetAction implements JsonGetAction { } private ImmutableList> getLockedDomains(String clientId) { - ImmutableList locks = - RegistryLockDao.getByRegistrarId(clientId).stream() - .filter(RegistryLock::isVerified) - .collect(toImmutableList()); - return locks.stream().map(this::lockToMap).collect(toImmutableList()); + return RegistryLockDao.getLockedDomainsByRegistrarId(clientId).stream() + .map(this::lockToMap) + .collect(toImmutableList()); } private ImmutableMap 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()); } 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 f1110b00a..8b41e9455 100644 --- a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java +++ b/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 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(); 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 cba95c920..6685b2976 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 @@ -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( diff --git a/db/src/main/resources/sql/flyway/V13__refactor_registry_lock.sql b/db/src/main/resources/sql/flyway/V13__refactor_registry_lock.sql new file mode 100644 index 000000000..b21a2780b --- /dev/null +++ b/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; diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 7f990757d..34c880300 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/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) ); diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 36fb4bf8c..3cd12a54c 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/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 );