From a38010dfec487e812583fbc5ed44147fafc8d7bc Mon Sep 17 00:00:00 2001 From: gbrodman Date: Thu, 8 Sep 2022 15:21:56 -0400 Subject: [PATCH] Add a DAO for User objects and fix up the User DB object (#1765) First, we create a sequence of User IDs in Postgres and assign it to the User ID field, meaning that Hibernate can autogenerate IDs. Next, add an update timestamp. Next, add a constraint that we can't have multiple Users with the same email address. Finally, create a DAO since we'll usually want to query by that email address (at least for now). --- .../google/registry/model/console/User.java | 12 ++- .../registry/model/console/UserDao.java | 57 ++++++++++++ .../registry/model/console/UserDaoTest.java | 89 +++++++++++++++++++ .../registry/model/console/UserTest.java | 35 ++------ .../sql/er_diagram/brief_er_diagram.html | 4 +- .../sql/er_diagram/full_er_diagram.html | 4 +- .../sql/schema/db-schema.sql.generated | 3 +- 7 files changed, 165 insertions(+), 39 deletions(-) create mode 100644 core/src/main/java/google/registry/model/console/UserDao.java create mode 100644 core/src/test/java/google/registry/model/console/UserDaoTest.java diff --git a/core/src/main/java/google/registry/model/console/User.java b/core/src/main/java/google/registry/model/console/User.java index 6a1345199..11977307b 100644 --- a/core/src/main/java/google/registry/model/console/User.java +++ b/core/src/main/java/google/registry/model/console/User.java @@ -22,10 +22,12 @@ import static google.registry.util.PasswordUtils.SALT_SUPPLIER; import static google.registry.util.PasswordUtils.hashPassword; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; +import google.registry.model.BackupGroupRoot; import google.registry.model.Buildable; -import google.registry.model.ImmutableObject; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; import javax.persistence.Id; import javax.persistence.Index; import javax.persistence.Table; @@ -37,10 +39,12 @@ import javax.persistence.Table; @Index(columnList = "gaiaId", name = "user_gaia_id_idx"), @Index(columnList = "emailAddress", name = "user_email_address_idx") }) -public class User extends ImmutableObject implements Buildable { +public class User extends BackupGroupRoot implements Buildable { /** Autogenerated unique ID of this user. */ - @Id private long id; + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; /** GAIA ID associated with the user in question. */ @Column(nullable = false) @@ -63,7 +67,7 @@ public class User extends ImmutableObject implements Buildable { /** Randomly generated hash salt. */ String registryLockPasswordSalt; - public long getId() { + public Long getId() { return id; } diff --git a/core/src/main/java/google/registry/model/console/UserDao.java b/core/src/main/java/google/registry/model/console/UserDao.java new file mode 100644 index 000000000..a61960e2c --- /dev/null +++ b/core/src/main/java/google/registry/model/console/UserDao.java @@ -0,0 +1,57 @@ +// Copyright 2022 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.model.console; + +import static com.google.common.base.Preconditions.checkArgument; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import java.util.Optional; + +/** Data access object for {@link User} objects to simplify saving and retrieval. */ +public class UserDao { + + /** Retrieves the one user with this email address if it exists. */ + public static Optional loadUser(String emailAddress) { + return jpaTm() + .transact( + () -> + jpaTm() + .query("FROM User WHERE emailAddress = :emailAddress", User.class) + .setParameter("emailAddress", emailAddress) + .getResultStream() + .findFirst()); + } + + /** Saves the given user, checking that no existing user already exists with this email. */ + public static void saveUser(User user) { + jpaTm() + .transact( + () -> { + // Check for an existing user (the unique constraint protects us, but this gives a + // nicer exception) + Optional maybeSavedUser = loadUser(user.getEmailAddress()); + if (maybeSavedUser.isPresent()) { + User savedUser = maybeSavedUser.get(); + checkArgument( + savedUser.getId().equals(user.getId()), + String.format( + "Attempted save of User with email address %s and ID %s, user with that" + + " email already exists with ID %s", + user.getEmailAddress(), user.getId(), savedUser.getId())); + } + jpaTm().put(user); + }); + } +} diff --git a/core/src/test/java/google/registry/model/console/UserDaoTest.java b/core/src/test/java/google/registry/model/console/UserDaoTest.java new file mode 100644 index 000000000..4e383f07b --- /dev/null +++ b/core/src/test/java/google/registry/model/console/UserDaoTest.java @@ -0,0 +1,89 @@ +// Copyright 2022 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.model.console; + +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.common.truth.Truth8; +import google.registry.model.EntityTestCase; +import org.junit.jupiter.api.Test; + +/** Tests for {@link UserDao}. */ +public class UserDaoTest extends EntityTestCase { + + @Test + void testSuccess_saveAndRetrieve() { + User user1 = + new User.Builder() + .setEmailAddress("email@email.com") + .setGaiaId("gaiaId") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.SUPPORT_AGENT).build()) + .build(); + User user2 = + new User.Builder() + .setEmailAddress("foo@bar.com") + .setGaiaId("otherId") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.SUPPORT_AGENT).build()) + .build(); + UserDao.saveUser(user1); + UserDao.saveUser(user2); + assertAboutImmutableObjects() + .that(user1) + .isEqualExceptFields(UserDao.loadUser("email@email.com").get(), "id", "updateTimestamp"); + assertAboutImmutableObjects() + .that(user2) + .isEqualExceptFields(UserDao.loadUser("foo@bar.com").get(), "id", "updateTimestamp"); + } + + @Test + void testSuccess_absentUser() { + User user = + new User.Builder() + .setEmailAddress("email@email.com") + .setGaiaId("gaiaId") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.SUPPORT_AGENT).build()) + .build(); + UserDao.saveUser(user); + User fromDb = UserDao.loadUser("email@email.com").get(); + // nonexistent one should never exist + Truth8.assertThat(UserDao.loadUser("nonexistent@email.com")).isEmpty(); + // now try deleting the one that does exist + jpaTm().transact(() -> jpaTm().delete(fromDb)); + Truth8.assertThat(UserDao.loadUser("email@email.com")).isEmpty(); + } + + @Test + void testFailure_sameEmail() { + User user1 = + new User.Builder() + .setEmailAddress("email@email.com") + .setGaiaId("gaiaId") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.SUPPORT_AGENT).build()) + .build(); + User user2 = + new User.Builder() + .setEmailAddress("email@email.com") + .setGaiaId("otherId") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.SUPPORT_AGENT).build()) + .build(); + UserDao.saveUser(user1); + assertThrows(IllegalArgumentException.class, () -> UserDao.saveUser(user2)); + assertAboutImmutableObjects() + .that(user1) + .isEqualExceptFields(UserDao.loadUser("email@email.com").get(), "id", "updateTimestamp"); + } +} diff --git a/core/src/test/java/google/registry/model/console/UserTest.java b/core/src/test/java/google/registry/model/console/UserTest.java index fe03f1458..9827b20e8 100644 --- a/core/src/test/java/google/registry/model/console/UserTest.java +++ b/core/src/test/java/google/registry/model/console/UserTest.java @@ -15,8 +15,8 @@ package google.registry.model.console; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.testing.DatabaseHelper.persistResource; import static org.junit.jupiter.api.Assertions.assertThrows; import google.registry.model.EntityTestCase; @@ -29,32 +29,6 @@ public class UserTest extends EntityTestCase { super(JpaEntityCoverageCheck.ENABLED); } - @Test - void testPersistence_lookupByEmail() { - User user = - new User.Builder() - .setGaiaId("gaiaId") - .setEmailAddress("email@email.com") - .setUserRoles( - new UserRoles.Builder().setGlobalRole(GlobalRole.FTE).setIsAdmin(true).build()) - .build(); - persistResource(user); - jpaTm() - .transact( - () -> { - assertThat( - jpaTm() - .query("FROM User WHERE emailAddress = 'email@email.com'", User.class) - .getSingleResult()) - .isEqualTo(user); - assertThat( - jpaTm() - .query("FROM User WHERE emailAddress = 'bad@fake.com'", User.class) - .getResultList()) - .isEmpty(); - }); - } - @Test void testPersistence_lookupByGaiaId() { User user = @@ -64,15 +38,16 @@ public class UserTest extends EntityTestCase { .setUserRoles( new UserRoles.Builder().setGlobalRole(GlobalRole.FTE).setIsAdmin(true).build()) .build(); - persistResource(user); + jpaTm().transact(() -> jpaTm().put(user)); jpaTm() .transact( () -> { - assertThat( + assertAboutImmutableObjects() + .that( jpaTm() .query("FROM User WHERE gaiaId = 'gaiaId'", User.class) .getSingleResult()) - .isEqualTo(user); + .isEqualExceptFields(user, "id", "updateTimestamp"); assertThat( jpaTm() .query("FROM User WHERE gaiaId = 'badGaiaId'", User.class) diff --git a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html index 550d21a9e..fb82435df 100644 --- a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html @@ -261,7 +261,7 @@ td.section { generated on - 2022-08-30 20:29:57.165769 + 2022-09-01 15:31:59.206925 last flyway file @@ -284,7 +284,7 @@ td.section { generated on - 2022-08-30 20:29:57.165769 + 2022-09-01 15:31:59.206925 diff --git a/db/src/main/resources/sql/er_diagram/full_er_diagram.html b/db/src/main/resources/sql/er_diagram/full_er_diagram.html index c06c4d31f..d86508582 100644 --- a/db/src/main/resources/sql/er_diagram/full_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/full_er_diagram.html @@ -261,7 +261,7 @@ td.section { generated on - 2022-08-30 20:29:55.057361 + 2022-09-01 15:31:54.876473 last flyway file @@ -284,7 +284,7 @@ td.section { generated on - 2022-08-30 20:29:55.057361 + 2022-09-01 15:31:54.876473 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 d0065992e..7165e65b9 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -742,7 +742,8 @@ ); create table "User" ( - id int8 not null, + id bigserial not null, + update_timestamp timestamptz, email_address text not null, gaia_id text not null, registry_lock_password_hash text,