From 256e3cfa5d2010b2251aa0ba1348f3756f3ea1b0 Mon Sep 17 00:00:00 2001 From: Samuel Fang <60355570+samuelfangjw@users.noreply.github.com> Date: Fri, 24 Feb 2023 03:44:00 +0800 Subject: [PATCH] [#12048] Migrate accounts db layer (#12114) --- .../it/storage/sqlapi/AccountRequestDbIT.java | 2 +- .../it/storage/sqlapi/AccountsDbIT.java | 61 ++++++++ .../BaseTestCaseWithSqlDatabaseAccess.java | 40 +++++- .../teammates/common/util/HibernateUtil.java | 9 ++ .../teammates/storage/sqlapi/AccountsDb.java | 88 ++++++++++++ .../teammates/storage/sqlapi/EntitiesDb.java | 8 +- .../teammates/storage/sqlentity/Account.java | 3 +- .../storage/sqlapi/AccountsDbTest.java | 135 ++++++++++++++++++ .../storage/sqlapi/CoursesDbTest.java | 2 +- 9 files changed, 340 insertions(+), 8 deletions(-) create mode 100644 src/it/java/teammates/it/storage/sqlapi/AccountsDbIT.java create mode 100644 src/main/java/teammates/storage/sqlapi/AccountsDb.java create mode 100644 src/test/java/teammates/storage/sqlapi/AccountsDbTest.java diff --git a/src/it/java/teammates/it/storage/sqlapi/AccountRequestDbIT.java b/src/it/java/teammates/it/storage/sqlapi/AccountRequestDbIT.java index 0a91002012b..83e128c8e2f 100644 --- a/src/it/java/teammates/it/storage/sqlapi/AccountRequestDbIT.java +++ b/src/it/java/teammates/it/storage/sqlapi/AccountRequestDbIT.java @@ -66,7 +66,7 @@ public void testCreateReadDeleteAccountRequest() throws Exception { AccountRequest actualAccountRequest = accountRequestDb.getAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute()); - verifyEquals(null, actualAccountRequest); + assertNull(actualAccountRequest); } @Test diff --git a/src/it/java/teammates/it/storage/sqlapi/AccountsDbIT.java b/src/it/java/teammates/it/storage/sqlapi/AccountsDbIT.java new file mode 100644 index 00000000000..61c9e3bacc0 --- /dev/null +++ b/src/it/java/teammates/it/storage/sqlapi/AccountsDbIT.java @@ -0,0 +1,61 @@ +package teammates.it.storage.sqlapi; + +import org.testng.annotations.Test; + +import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.InvalidParametersException; +import teammates.common.util.HibernateUtil; +import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess; +import teammates.storage.sqlapi.AccountsDb; +import teammates.storage.sqlentity.Account; + +/** + * SUT: {@link AccountsDb}. + */ +public class AccountsDbIT extends BaseTestCaseWithSqlDatabaseAccess { + + private final AccountsDb accountsDb = AccountsDb.inst(); + + @Test + public void testCreateAccount() throws Exception { + ______TS("Create account, does not exists, succeeds"); + + Account account = new Account("google-id", "name", "email@teammates.com"); + + accountsDb.createAccount(account); + HibernateUtil.flushSession(); + + Account actualAccount = accountsDb.getAccount(account.getId()); + verifyEquals(account, actualAccount); + } + + @Test + public void testUpdateAccount() throws Exception { + Account account = new Account("google-id", "name", "email@teammates.com"); + accountsDb.createAccount(account); + HibernateUtil.flushSession(); + + ______TS("Update existing account, success"); + + account.setName("new account name"); + accountsDb.updateAccount(account); + + Account actual = accountsDb.getAccount(account.getId()); + verifyEquals(account, actual); + } + + @Test + public void testDeleteAccount() throws InvalidParametersException, EntityAlreadyExistsException { + Account account = new Account("google-id", "name", "email@teammates.com"); + accountsDb.createAccount(account); + HibernateUtil.flushSession(); + + ______TS("Delete existing account, success"); + + accountsDb.deleteAccount(account); + + Account actual = accountsDb.getAccount(account.getId()); + assertNull(actual); + } + +} diff --git a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java index 5635efa8570..15871e0458f 100644 --- a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java +++ b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java @@ -13,9 +13,12 @@ import teammates.common.util.JsonUtils; import teammates.sqllogic.api.Logic; import teammates.sqllogic.core.LogicStarter; +import teammates.storage.sqlentity.Account; +import teammates.storage.sqlentity.AccountRequest; import teammates.storage.sqlentity.BaseEntity; import teammates.storage.sqlentity.Course; import teammates.storage.sqlentity.Notification; +import teammates.storage.sqlentity.UsageStatistics; import teammates.test.BaseTestCase; /** @@ -34,7 +37,8 @@ public class BaseTestCaseWithSqlDatabaseAccess extends BaseTestCase { public static void setUpClass() throws Exception { PGSQL.start(); // Temporarily disable migration utility - // DbMigrationUtil.resetDb(PGSQL.getJdbcUrl(), PGSQL.getUsername(), PGSQL.getPassword()); + // DbMigrationUtil.resetDb(PGSQL.getJdbcUrl(), PGSQL.getUsername(), + // PGSQL.getPassword()); HibernateUtil.buildSessionFactory(PGSQL.getJdbcUrl(), PGSQL.getUsername(), PGSQL.getPassword()); LogicStarter.initializeDependencies(); @@ -69,6 +73,23 @@ protected void verifyEquals(BaseEntity expected, BaseEntity actual) { Notification actualNotification = (Notification) actual; equalizeIrrelevantData(expectedNotification, actualNotification); assertEquals(JsonUtils.toJson(expectedNotification), JsonUtils.toJson(actualNotification)); + } else if (expected instanceof Account) { + Account expectedAccount = (Account) expected; + Account actualAccount = (Account) actual; + equalizeIrrelevantData(expectedAccount, actualAccount); + assertEquals(JsonUtils.toJson(expectedAccount), JsonUtils.toJson(actualAccount)); + } else if (expected instanceof AccountRequest) { + AccountRequest expectedAccountRequest = (AccountRequest) expected; + AccountRequest actualAccountRequest = (AccountRequest) actual; + equalizeIrrelevantData(expectedAccountRequest, actualAccountRequest); + assertEquals(JsonUtils.toJson(expectedAccountRequest), JsonUtils.toJson(actualAccountRequest)); + } else if (expected instanceof UsageStatistics) { + UsageStatistics expectedUsageStatistics = (UsageStatistics) expected; + UsageStatistics actualUsageStatistics = (UsageStatistics) actual; + equalizeIrrelevantData(expectedUsageStatistics, actualUsageStatistics); + assertEquals(JsonUtils.toJson(expectedUsageStatistics), JsonUtils.toJson(actualUsageStatistics)); + } else { + fail("Unknown entity"); } } @@ -100,6 +121,23 @@ private void equalizeIrrelevantData(Notification expected, Notification actual) expected.setUpdatedAt(actual.getUpdatedAt()); } + private void equalizeIrrelevantData(Account expected, Account actual) { + // Ignore time field as it is stamped at the time of creation in testing + expected.setCreatedAt(actual.getCreatedAt()); + expected.setUpdatedAt(actual.getUpdatedAt()); + } + + private void equalizeIrrelevantData(AccountRequest expected, AccountRequest actual) { + // Ignore time field as it is stamped at the time of creation in testing + expected.setCreatedAt(actual.getCreatedAt()); + expected.setUpdatedAt(actual.getUpdatedAt()); + } + + private void equalizeIrrelevantData(UsageStatistics expected, UsageStatistics actual) { + // Ignore time field as it is stamped at the time of creation in testing + expected.setCreatedAt(actual.getCreatedAt()); + } + /** * Generates a UUID that is different from the given {@code uuid}. */ diff --git a/src/main/java/teammates/common/util/HibernateUtil.java b/src/main/java/teammates/common/util/HibernateUtil.java index d431c1c960d..34e02c99a58 100644 --- a/src/main/java/teammates/common/util/HibernateUtil.java +++ b/src/main/java/teammates/common/util/HibernateUtil.java @@ -145,6 +145,15 @@ public static T get(Class entityType, Object id) { return HibernateUtil.getCurrentSession().get(entityType, id); } + /** + * Return the persistent instance of the given entity class with the given natural id, + * or null if there is no such persistent instance. + * @see Session#get(Class, Object) + */ + public static T getBySimpleNaturalId(Class entityType, Object id) { + return HibernateUtil.getCurrentSession().bySimpleNaturalId(entityType).load(id); + } + /** * Copy the state of the given object onto the persistent object with the same identifier. * @see Session#merge(E) diff --git a/src/main/java/teammates/storage/sqlapi/AccountsDb.java b/src/main/java/teammates/storage/sqlapi/AccountsDb.java new file mode 100644 index 00000000000..2795b4d34a6 --- /dev/null +++ b/src/main/java/teammates/storage/sqlapi/AccountsDb.java @@ -0,0 +1,88 @@ +package teammates.storage.sqlapi; + +import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; +import teammates.common.exception.InvalidParametersException; +import teammates.common.util.HibernateUtil; +import teammates.storage.sqlentity.Account; + +/** + * Handles CRUD operations for accounts. + * + * @see Account + */ +public final class AccountsDb extends EntitiesDb { + + private static final AccountsDb instance = new AccountsDb(); + + private AccountsDb() { + // prevent initialization + } + + public static AccountsDb inst() { + return instance; + } + + /** + * Returns an Account with the {@code id} or null if it does not exist. + */ + public Account getAccount(Integer id) { + assert id != null; + + return HibernateUtil.get(Account.class, id); + } + + /** + * Returns an Account with the {@code googleId} or null if it does not exist. + */ + public Account getAccountByGoogleId(String googleId) { + assert googleId != null; + + return HibernateUtil.getBySimpleNaturalId(Account.class, googleId); + } + + /** + * Creates an Account. + */ + public Account createAccount(Account account) throws InvalidParametersException, EntityAlreadyExistsException { + assert account != null; + + if (!account.isValid()) { + throw new InvalidParametersException(account.getInvalidityInfo()); + } + + if (getAccountByGoogleId(account.getGoogleId()) != null) { + throw new EntityAlreadyExistsException(String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, account.toString())); + } + + persist(account); + return account; + } + + /** + * Saves an updated {@code Account} to the db. + */ + public Account updateAccount(Account account) throws InvalidParametersException, EntityDoesNotExistException { + assert account != null; + + if (!account.isValid()) { + throw new InvalidParametersException(account.getInvalidityInfo()); + } + + if (getAccount(account.getId()) == null) { + throw new EntityDoesNotExistException(ERROR_UPDATE_NON_EXISTENT + account.toString()); + } + + return merge(account); + } + + /** + * Deletes an Account. + */ + public void deleteAccount(Account account) { + if (account != null) { + delete(account); + } + } + +} diff --git a/src/main/java/teammates/storage/sqlapi/EntitiesDb.java b/src/main/java/teammates/storage/sqlapi/EntitiesDb.java index 6b100a861cf..9640e36a013 100644 --- a/src/main/java/teammates/storage/sqlapi/EntitiesDb.java +++ b/src/main/java/teammates/storage/sqlapi/EntitiesDb.java @@ -22,18 +22,18 @@ class EntitiesDb { * Copy the state of the given object onto the persistent object with the same identifier. * If there is no persistent instance currently associated with the session, it will be loaded. */ - E merge(E entity) { + protected E merge(E entity) { assert entity != null; E newEntity = HibernateUtil.merge(entity); - log.info("Entity saves: " + JsonUtils.toJson(entity)); + log.info("Entity updated: " + JsonUtils.toJson(entity)); return newEntity; } /** * Associate {@code entity} with the persistence context. */ - void persist(E entity) { + protected void persist(E entity) { assert entity != null; HibernateUtil.persist(entity); @@ -43,7 +43,7 @@ void persist(E entity) { /** * Deletes {@code entity} from persistence context. */ - void delete(E entity) { + protected void delete(E entity) { assert entity != null; HibernateUtil.remove(entity); diff --git a/src/main/java/teammates/storage/sqlentity/Account.java b/src/main/java/teammates/storage/sqlentity/Account.java index 3a590028097..4189c1af73b 100644 --- a/src/main/java/teammates/storage/sqlentity/Account.java +++ b/src/main/java/teammates/storage/sqlentity/Account.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Objects; +import org.hibernate.annotations.NaturalId; import org.hibernate.annotations.UpdateTimestamp; import teammates.common.util.FieldValidator; @@ -27,7 +28,7 @@ public class Account extends BaseEntity { @GeneratedValue private Integer id; - @Column(nullable = false) + @NaturalId private String googleId; @Column(nullable = false) diff --git a/src/test/java/teammates/storage/sqlapi/AccountsDbTest.java b/src/test/java/teammates/storage/sqlapi/AccountsDbTest.java new file mode 100644 index 00000000000..4682744127e --- /dev/null +++ b/src/test/java/teammates/storage/sqlapi/AccountsDbTest.java @@ -0,0 +1,135 @@ +package teammates.storage.sqlapi; + +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; + +import org.mockito.MockedStatic; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; +import teammates.common.exception.InvalidParametersException; +import teammates.common.util.HibernateUtil; +import teammates.storage.sqlentity.Account; +import teammates.test.BaseTestCase; + +/** + * SUT: {@code AccountsDb}. + */ +public class AccountsDbTest extends BaseTestCase { + + private AccountsDb accountsDb = AccountsDb.inst(); + + private MockedStatic mockHibernateUtil; + + @BeforeMethod + public void setUpMethod() { + mockHibernateUtil = mockStatic(HibernateUtil.class); + } + + @AfterMethod + public void teardownMethod() { + mockHibernateUtil.close(); + } + + @Test + public void testCreateAccount_accountDoesNotExist_success() + throws InvalidParametersException, EntityAlreadyExistsException { + Account account = new Account("google-id", "name", "email@teammates.com"); + + accountsDb.createAccount(account); + + mockHibernateUtil.verify(() -> HibernateUtil.persist(account)); + } + + @Test + public void testCreateAccount_accountAlreadyExists_throwsEntityAlreadyExistsException() { + Account existingAccount = getAccountWithId(); + mockHibernateUtil.when(() -> HibernateUtil.getBySimpleNaturalId(Account.class, "google-id")) + .thenReturn(existingAccount); + Account account = new Account("google-id", "different name", "email@teammates.com"); + + EntityAlreadyExistsException ex = assertThrows(EntityAlreadyExistsException.class, + () -> accountsDb.createAccount(account)); + + assertEquals("Trying to create an entity that exists: " + account.toString(), ex.getMessage()); + mockHibernateUtil.verify(() -> HibernateUtil.persist(account), never()); + } + + @Test + public void testCreateAccount_invalidEmail_throwsInvalidParametersException() { + Account account = new Account("google-id", "name", "invalid"); + + InvalidParametersException ex = assertThrows(InvalidParametersException.class, + () -> accountsDb.createAccount(account)); + + assertEquals( + "\"invalid\" is not acceptable to TEAMMATES as a/an email because it is not in the correct format. " + + "An email address contains some text followed by one '@' sign followed by some more text, " + + "and should end with a top level domain address like .com. " + + "It cannot be longer than 254 characters, " + + "cannot be empty and cannot contain spaces.", + ex.getMessage()); + mockHibernateUtil.verify(() -> HibernateUtil.persist(account), never()); + } + + @Test + public void testUpdateAccount_accountAlreadyExists_success() + throws InvalidParametersException, EntityDoesNotExistException { + Account account = getAccountWithId(); + mockHibernateUtil.when(() -> HibernateUtil.get(Account.class, account.getId())) + .thenReturn(account); + account.setName("new name"); + + accountsDb.updateAccount(account); + + mockHibernateUtil.verify(() -> HibernateUtil.merge(account)); + } + + @Test + public void testUpdateAccount_accountDoesNotExist_throwsEntityDoesNotExistException() + throws InvalidParametersException, EntityAlreadyExistsException { + Account account = getAccountWithId(); + + EntityDoesNotExistException ex = assertThrows(EntityDoesNotExistException.class, + () -> accountsDb.updateAccount(account)); + + assertEquals("Trying to update non-existent Entity: " + account.toString(), ex.getMessage()); + mockHibernateUtil.verify(() -> HibernateUtil.persist(account), never()); + } + + @Test + public void testUpdateAccount_invalidEmail_throwsInvalidParametersException() { + Account account = getAccountWithId(); + account.setEmail("invalid"); + + InvalidParametersException ex = assertThrows(InvalidParametersException.class, + () -> accountsDb.updateAccount(account)); + + assertEquals( + "\"invalid\" is not acceptable to TEAMMATES as a/an email because it is not in the correct format. " + + "An email address contains some text followed by one '@' sign followed by some more text, " + + "and should end with a top level domain address like .com. " + + "It cannot be longer than 254 characters, " + + "cannot be empty and cannot contain spaces.", + ex.getMessage()); + mockHibernateUtil.verify(() -> HibernateUtil.persist(account), never()); + } + + @Test + public void testDeleteAccount_success() { + Account account = new Account("google-id", "name", "email@teammates.com"); + + accountsDb.deleteAccount(account); + + mockHibernateUtil.verify(() -> HibernateUtil.remove(account)); + } + + private Account getAccountWithId() { + Account account = new Account("google-id", "name", "email@teammates.com"); + account.setId(1); + return account; + } +} diff --git a/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java b/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java index 00aaebadb1d..25754bc340d 100644 --- a/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java +++ b/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java @@ -51,7 +51,7 @@ public void testCreateCourse_courseAlreadyExists_throwsEntityAlreadyExistsExcept EntityAlreadyExistsException ex = assertThrows(EntityAlreadyExistsException.class, () -> coursesDb.createCourse(c)); - assertEquals(ex.getMessage(), "Trying to create an entity that exists: " + c.toString()); + assertEquals("Trying to create an entity that exists: " + c.toString(), ex.getMessage()); mockHibernateUtil.verify(() -> HibernateUtil.persist(c), never()); } }