diff --git a/src/it/java/teammates/it/storage/sqlapi/UsersDbIT.java b/src/it/java/teammates/it/storage/sqlapi/UsersDbIT.java new file mode 100644 index 00000000000..525e9e6e1b3 --- /dev/null +++ b/src/it/java/teammates/it/storage/sqlapi/UsersDbIT.java @@ -0,0 +1,85 @@ +package teammates.it.storage.sqlapi; + +import java.util.UUID; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.datatransfer.InstructorPermissionRole; +import teammates.common.datatransfer.InstructorPrivileges; +import teammates.common.util.Const; +import teammates.common.util.HibernateUtil; +import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess; +import teammates.storage.sqlapi.CoursesDb; +import teammates.storage.sqlapi.UsersDb; +import teammates.storage.sqlentity.Course; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; + +/** + * SUT: {@link UsersDb}. + */ +public class UsersDbIT extends BaseTestCaseWithSqlDatabaseAccess { + + private final UsersDb usersDb = UsersDb.inst(); + private final CoursesDb coursesDb = CoursesDb.inst(); + + private Course course; + private Instructor instructor; + private Student student; + + @BeforeMethod + @Override + public void setUp() throws Exception { + super.setUp(); + + course = new Course("course-id", "course-name", Const.DEFAULT_TIME_ZONE, "institute"); + coursesDb.createCourse(course); + + instructor = getTypicalInstructor(); + usersDb.createInstructor(instructor); + + student = getTypicalStudent(); + usersDb.createStudent(student); + + HibernateUtil.flushSession(); + } + + @Test + public void testGetInstructor() { + ______TS("success: gets an instructor that already exists"); + Instructor actualInstructor = usersDb.getInstructor(instructor.getId()); + verifyEquals(instructor, actualInstructor); + + ______TS("success: gets an instructor that does not exist"); + UUID nonExistentId = UUID.fromString("00000000-0000-1000-0000-000000000000"); + Instructor nonExistentInstructor = usersDb.getInstructor(nonExistentId); + assertNull(nonExistentInstructor); + } + + @Test + public void testGetStudent() { + ______TS("success: gets a student that already exists"); + Student actualstudent = usersDb.getStudent(student.getId()); + verifyEquals(student, actualstudent); + + ______TS("success: gets a student that does not exist"); + UUID nonExistentId = UUID.fromString("00000000-0000-1000-0000-000000000000"); + Student nonExistentstudent = usersDb.getStudent(nonExistentId); + assertNull(nonExistentstudent); + } + + private Student getTypicalStudent() { + return new Student(course, "student-name", "valid@email.tmt", "comments"); + } + + private Instructor getTypicalInstructor() { + InstructorPrivileges instructorPrivileges = + new InstructorPrivileges(Const.InstructorPermissionRoleNames.INSTRUCTOR_PERMISSION_ROLE_COOWNER); + InstructorPermissionRole role = InstructorPermissionRole + .getEnum(Const.InstructorPermissionRoleNames.INSTRUCTOR_PERMISSION_ROLE_COOWNER); + + return new Instructor(course, "instructor-name", "valid@email.tmt", + false, Const.DEFAULT_DISPLAY_NAME_FOR_INSTRUCTOR, role, instructorPrivileges); + } +} diff --git a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java index 15871e0458f..507784a17dd 100644 --- a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java +++ b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java @@ -17,7 +17,9 @@ import teammates.storage.sqlentity.AccountRequest; import teammates.storage.sqlentity.BaseEntity; import teammates.storage.sqlentity.Course; +import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Notification; +import teammates.storage.sqlentity.Student; import teammates.storage.sqlentity.UsageStatistics; import teammates.test.BaseTestCase; @@ -88,6 +90,16 @@ protected void verifyEquals(BaseEntity expected, BaseEntity actual) { UsageStatistics actualUsageStatistics = (UsageStatistics) actual; equalizeIrrelevantData(expectedUsageStatistics, actualUsageStatistics); assertEquals(JsonUtils.toJson(expectedUsageStatistics), JsonUtils.toJson(actualUsageStatistics)); + } else if (expected instanceof Instructor) { + Instructor expectedInstructor = (Instructor) expected; + Instructor actualInstructor = (Instructor) actual; + equalizeIrrelevantData(expectedInstructor, actualInstructor); + assertEquals(JsonUtils.toJson(expectedInstructor), JsonUtils.toJson(actualInstructor)); + } else if (expected instanceof Student) { + Student expectedStudent = (Student) expected; + Student actualStudent = (Student) actual; + equalizeIrrelevantData(expectedStudent, actualStudent); + assertEquals(JsonUtils.toJson(expectedStudent), JsonUtils.toJson(actualStudent)); } else { fail("Unknown entity"); } @@ -138,6 +150,18 @@ private void equalizeIrrelevantData(UsageStatistics expected, UsageStatistics ac expected.setCreatedAt(actual.getCreatedAt()); } + private void equalizeIrrelevantData(Instructor expected, Instructor 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(Student expected, Student actual) { + // Ignore time field as it is stamped at the time of creation in testing + expected.setCreatedAt(actual.getCreatedAt()); + expected.setUpdatedAt(actual.getUpdatedAt()); + } + /** * Generates a UUID that is different from the given {@code uuid}. */ diff --git a/src/main/java/teammates/sqllogic/core/UsersLogic.java b/src/main/java/teammates/sqllogic/core/UsersLogic.java new file mode 100644 index 00000000000..93366d44445 --- /dev/null +++ b/src/main/java/teammates/sqllogic/core/UsersLogic.java @@ -0,0 +1,56 @@ +package teammates.sqllogic.core; + +import java.util.UUID; + +import teammates.storage.sqlapi.UsersDb; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; + +/** + * Handles operations related to user (instructor & student). + * + * @see User + * @see UsersDb + */ +public final class UsersLogic { + + private static final UsersLogic instance = new UsersLogic(); + + private UsersDb usersDb; + + private UsersLogic() { + // prevent initialization + } + + public static UsersLogic inst() { + return instance; + } + + void initLogicDependencies(UsersDb usersDb) { + this.usersDb = usersDb; + } + + /** + * Gets instructor associated with {@code id}. + * + * @param id Id of Instructor. + * @return Returns Instructor if found else null. + */ + public Instructor getInstructor(UUID id) { + assert id != null; + + return usersDb.getInstructor(id); + } + + /** + * Gets student associated with {@code id}. + * + * @param id Id of Student. + * @return Returns Student if found else null. + */ + public Student getStudent(UUID id) { + assert id != null; + + return usersDb.getStudent(id); + } +} diff --git a/src/main/java/teammates/storage/sqlapi/UsersDb.java b/src/main/java/teammates/storage/sqlapi/UsersDb.java index 726aaf8a10a..b1c02caf2ca 100644 --- a/src/main/java/teammates/storage/sqlapi/UsersDb.java +++ b/src/main/java/teammates/storage/sqlapi/UsersDb.java @@ -26,6 +26,7 @@ * @see User */ public final class UsersDb extends EntitiesDb { + private static final UsersDb instance = new UsersDb(); private UsersDb() { @@ -51,7 +52,8 @@ public Instructor createInstructor(Instructor instructor) String email = instructor.getEmail(); if (hasExistingInstructor(courseId, email)) { - throw new EntityAlreadyExistsException(String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, instructor.toString())); + throw new EntityAlreadyExistsException( + String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, instructor.toString())); } persist(instructor); @@ -73,7 +75,8 @@ public Student createStudent(Student student) String email = student.getEmail(); if (hasExistingStudent(courseId, email)) { - throw new EntityAlreadyExistsException(String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, student.toString())); + throw new EntityAlreadyExistsException( + String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, student.toString())); } persist(student); @@ -83,19 +86,19 @@ public Student createStudent(Student student) /** * Gets an instructor by its {@code id}. */ - public Instructor getInstructor(Integer id) { + public Instructor getInstructor(UUID id) { assert id != null; - return HibernateUtil.getCurrentSession().get(Instructor.class, id); + return HibernateUtil.get(Instructor.class, id); } /** * Gets a student by its {@code id}. */ - public Student getStudent(Integer id) { + public Student getStudent(UUID id) { assert id != null; - return HibernateUtil.getCurrentSession().get(Student.class, id); + return HibernateUtil.get(Student.class, id); } /** @@ -160,16 +163,15 @@ public long getNumStudentsByTimeRange(Instant startTime, Instant endTime) { /** * Checks if an instructor exists by its {@code courseId} and {@code email}. */ - private boolean hasExistingInstructor(String courseId, String email) { + private boolean hasExistingInstructor(String courseId, String email) { Session session = HibernateUtil.getCurrentSession(); CriteriaBuilder cb = session.getCriteriaBuilder(); CriteriaQuery cr = cb.createQuery(Instructor.class); Root instructorRoot = cr.from(Instructor.class); - cr.select(instructorRoot.get("id")) - .where(cb.and( - cb.equal(instructorRoot.get("courseId"), courseId), - cb.equal(instructorRoot.get("email"), email))); + cr.select(instructorRoot).where(cb.and( + cb.equal(instructorRoot.get("courseId"), courseId), + cb.equal(instructorRoot.get("email"), email))); return session.createQuery(cr).getSingleResultOrNull() != null; } @@ -177,16 +179,15 @@ private boolean hasExistingInstructor(String courseId, String e /** * Checks if a student exists by its {@code courseId} and {@code email}. */ - private boolean hasExistingStudent(String courseId, String email) { + private boolean hasExistingStudent(String courseId, String email) { Session session = HibernateUtil.getCurrentSession(); CriteriaBuilder cb = session.getCriteriaBuilder(); CriteriaQuery cr = cb.createQuery(Student.class); Root studentRoot = cr.from(Student.class); - cr.select(studentRoot.get("id")) - .where(cb.and( - cb.equal(studentRoot.get("courseId"), courseId), - cb.equal(studentRoot.get("email"), email))); + cr.select(studentRoot).where(cb.and( + cb.equal(studentRoot.get("courseId"), courseId), + cb.equal(studentRoot.get("email"), email))); return session.createQuery(cr).getSingleResultOrNull() != null; } @@ -197,6 +198,6 @@ private boolean hasExistingStudent(String courseId, String emai private boolean hasExistingUser(UUID id) { assert id != null; - return HibernateUtil.getCurrentSession().get(User.class, id) != null; + return HibernateUtil.get(User.class, id) != null; } } diff --git a/src/main/java/teammates/storage/sqlentity/BaseEntity.java b/src/main/java/teammates/storage/sqlentity/BaseEntity.java index 770f689ace5..d3fbf038548 100644 --- a/src/main/java/teammates/storage/sqlentity/BaseEntity.java +++ b/src/main/java/teammates/storage/sqlentity/BaseEntity.java @@ -92,8 +92,8 @@ public Duration convertToEntityAttribute(Long minutes) { @Converter public static class JsonConverter implements AttributeConverter { @Override - public String convertToDatabaseColumn(T questionDetails) { - return JsonUtils.toJson(questionDetails); + public String convertToDatabaseColumn(T entity) { + return JsonUtils.toJson(entity); } @Override diff --git a/src/main/java/teammates/storage/sqlentity/Instructor.java b/src/main/java/teammates/storage/sqlentity/Instructor.java index 6cdc773a1a4..bf811156410 100644 --- a/src/main/java/teammates/storage/sqlentity/Instructor.java +++ b/src/main/java/teammates/storage/sqlentity/Instructor.java @@ -5,7 +5,9 @@ import teammates.common.datatransfer.InstructorPermissionRole; import teammates.common.datatransfer.InstructorPrivileges; +import teammates.common.datatransfer.InstructorPrivilegesLegacy; import teammates.common.util.FieldValidator; +import teammates.common.util.JsonUtils; import teammates.common.util.SanitizationHelper; import jakarta.persistence.Column; @@ -32,7 +34,7 @@ public class Instructor extends User { @Enumerated(EnumType.STRING) private InstructorPermissionRole role; - @Column(nullable = false) + @Column(nullable = false, columnDefinition = "TEXT") @Convert(converter = InstructorPrivilegesConverter.class) private InstructorPrivileges instructorPrivileges; @@ -40,9 +42,9 @@ protected Instructor() { // required by Hibernate } - public Instructor(Course course, Team team, String name, String email, boolean isDisplayedToStudents, + public Instructor(Course course, String name, String email, boolean isDisplayedToStudents, String displayName, InstructorPermissionRole role, InstructorPrivileges instructorPrivileges) { - super(course, team, name, email); + super(course, name, email); this.setDisplayedToStudents(isDisplayedToStudents); this.setDisplayName(displayName); this.setRole(role); @@ -96,7 +98,7 @@ public List getInvalidityInfo() { addNonEmptyError(FieldValidator.getInvalidityInfoForPersonName(super.getName()), errors); addNonEmptyError(FieldValidator.getInvalidityInfoForEmail(super.getEmail()), errors); addNonEmptyError(FieldValidator.getInvalidityInfoForPersonName(displayName), errors); - addNonEmptyError(FieldValidator.getInvalidityInfoForRole(role.name()), errors); + addNonEmptyError(FieldValidator.getInvalidityInfoForRole(role.getRoleName()), errors); return errors; } @@ -107,5 +109,17 @@ public List getInvalidityInfo() { @Converter public static class InstructorPrivilegesConverter extends JsonConverter { + + @Override + public String convertToDatabaseColumn(InstructorPrivileges instructorPrivileges) { + return JsonUtils.toJson(instructorPrivileges.toLegacyFormat(), InstructorPrivilegesLegacy.class); + } + + @Override + public InstructorPrivileges convertToEntityAttribute(String instructorPriviledgesAsString) { + InstructorPrivilegesLegacy privilegesLegacy = + JsonUtils.fromJson(instructorPriviledgesAsString, InstructorPrivilegesLegacy.class); + return new InstructorPrivileges(privilegesLegacy); + } } } diff --git a/src/main/java/teammates/storage/sqlentity/Student.java b/src/main/java/teammates/storage/sqlentity/Student.java index 2f67a9f2e1f..e81f2581ee5 100644 --- a/src/main/java/teammates/storage/sqlentity/Student.java +++ b/src/main/java/teammates/storage/sqlentity/Student.java @@ -23,8 +23,8 @@ protected Student() { // required by Hibernate } - public Student(Course course, Team team, String name, String email, String comments) { - super(course, team, name, email); + public Student(Course course, String name, String email, String comments) { + super(course, name, email); this.setComments(comments); } diff --git a/src/main/java/teammates/storage/sqlentity/User.java b/src/main/java/teammates/storage/sqlentity/User.java index d665e9312aa..8e9817ae36d 100644 --- a/src/main/java/teammates/storage/sqlentity/User.java +++ b/src/main/java/teammates/storage/sqlentity/User.java @@ -33,8 +33,11 @@ public abstract class User extends BaseEntity { @JoinColumn(name = "accountId") private Account account; + @Column(nullable = false, insertable = false, updatable = false) + private String courseId; + @ManyToOne - @JoinColumn(name = "courseId") + @JoinColumn(name = "courseId", nullable = false) private Course course; @ManyToOne @@ -57,10 +60,9 @@ protected User() { // required by Hibernate } - public User(Course course, Team team, String name, String email) { + public User(Course course, String name, String email) { this.setId(UUID.randomUUID()); this.setCourse(course); - this.setTeam(team); this.setName(name); this.setEmail(email); this.setRegKey(generateRegistrationKey()); @@ -82,12 +84,20 @@ public void setAccount(Account account) { this.account = account; } + public String getCourseId() { + return courseId; + } + public Course getCourse() { return course; } + /** + * Sets a course as well as the courseId. + */ public void setCourse(Course course) { this.course = course; + this.courseId = course.getId(); } public Team getTeam() { diff --git a/src/test/java/teammates/storage/sqlapi/UsersDbTest.java b/src/test/java/teammates/storage/sqlapi/UsersDbTest.java new file mode 100644 index 00000000000..44d2be49292 --- /dev/null +++ b/src/test/java/teammates/storage/sqlapi/UsersDbTest.java @@ -0,0 +1,68 @@ +package teammates.storage.sqlapi; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; + +import org.mockito.MockedStatic; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.datatransfer.InstructorPermissionRole; +import teammates.common.datatransfer.InstructorPrivileges; +import teammates.common.util.HibernateUtil; +import teammates.storage.sqlentity.Course; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; +import teammates.test.BaseTestCase; + +/** + * SUT: {@code UsersDb}. + */ +public class UsersDbTest extends BaseTestCase { + + private UsersDb usersDb = UsersDb.inst(); + + private MockedStatic mockHibernateUtil; + + @BeforeMethod + public void setUpMethod() { + mockHibernateUtil = mockStatic(HibernateUtil.class); + } + + @AfterMethod + public void teardownMethod() { + mockHibernateUtil.close(); + } + + @Test + public void testGetInstructor_instructorIdPresent_success() { + Course course = mock(Course.class); + InstructorPrivileges instructorPrivileges = mock(InstructorPrivileges.class); + Instructor instructor = new Instructor(course, "instructor-name", "instructor-email", + false, "instructor-display-name", + InstructorPermissionRole.INSTRUCTOR_PERMISSION_ROLE_COOWNER, instructorPrivileges); + + mockHibernateUtil + .when(() -> HibernateUtil.get(Instructor.class, instructor.getId())) + .thenReturn(instructor); + + Instructor actualInstructor = usersDb.getInstructor(instructor.getId()); + + assertEquals(instructor, actualInstructor); + } + + @Test + public void testGetStudent_studentIdPresent_success() { + Course course = mock(Course.class); + Student student = new Student(course, "student-name", "student-email", "comments"); + + mockHibernateUtil + .when(() -> HibernateUtil.get(Student.class, student.getId())) + .thenReturn(student); + + Student actualStudent = usersDb.getStudent(student.getId()); + + assertEquals(student, actualStudent); + } +}