[#12779] Fix notifications update logic (#13083)

* Fix notifications update logic * Add required mocks --------- Co-authored-by: Ching Ming Yuan <[email protected]>
TEAMMATES · Jul 8, 2024 · 399a5fa · 399a5fa
1 parent 836f637
commit 399a5fa
Show file tree

Hide file tree

Showing 13 changed files with 187 additions and 32 deletions.
diff --git a/src/it/java/teammates/it/sqllogic/core/NotificationsLogicIT.java b/src/it/java/teammates/it/sqllogic/core/NotificationsLogicIT.java
@@ -10,6 +10,7 @@
 import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.exception.InvalidParametersException;
+import teammates.common.util.HibernateUtil;
 import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess;
 import teammates.sqllogic.core.NotificationsLogic;
 import teammates.storage.sqlentity.Notification;
@@ -21,6 +22,48 @@ public class NotificationsLogicIT extends BaseTestCaseWithSqlDatabaseAccess {
 
     private NotificationsLogic notificationsLogic = NotificationsLogic.inst();
 
+    @Test
+    public void testCreateNotification_endTimeIsBeforeStartTime_throwsInvalidParametersException() {
+        Notification invalidNotification = new Notification(
+                Instant.parse("2011-02-01T00:00:00Z"),
+                Instant.parse("2011-01-01T00:00:00Z"),
+                NotificationStyle.DANGER,
+                NotificationTargetUser.GENERAL,
+                "A deprecation note",
+                "<p>Deprecation happens in three minutes</p>");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        assertNull(notificationsLogic.getNotification(invalidNotification.getId()));
+    }
+
+    @Test
+    public void testCreateNotification_emptyTitle_throwsInvalidParametersException() {
+        Notification invalidNotification = new Notification(
+                Instant.parse("2011-01-01T00:00:00Z"),
+                Instant.parse("2099-01-01T00:00:00Z"),
+                NotificationStyle.DANGER,
+                NotificationTargetUser.GENERAL,
+                "",
+                "<p>Deprecation happens in three minutes</p>");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        assertNull(notificationsLogic.getNotification(invalidNotification.getId()));
+    }
+
+    @Test
+    public void testCreateNotification_emptyMessage_throwsInvalidParametersException() {
+        Notification invalidNotification = new Notification(
+                Instant.parse("2011-01-01T00:00:00Z"),
+                Instant.parse("2099-01-01T00:00:00Z"),
+                NotificationStyle.DANGER,
+                NotificationTargetUser.GENERAL,
+                "A deprecation note",
+                "");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        assertNull(notificationsLogic.getNotification(invalidNotification.getId()));
+    }
+
     @Test
     public void testUpdateNotification()
             throws EntityAlreadyExistsException, InvalidParametersException, EntityDoesNotExistException {
@@ -59,4 +102,38 @@ public void testUpdateNotification()
                 newStartTime, newEndTime, newStyle, newTargetUser, newTitle, newMessage));
     }
 
+    @Test
+    public void testUpdateNotification_invalidParameters_originalUnchanged() throws Exception {
+
+        String originalTitle = "The original title";
+        String originalMessage = "<p>Deprecation happens in three minutes</p>";
+        Notification notif = new Notification(
+                Instant.parse("2011-01-01T00:00:00Z"),
+                Instant.parse("2099-01-01T00:00:00Z"),
+                NotificationStyle.DANGER,
+                NotificationTargetUser.GENERAL,
+                originalTitle,
+                originalMessage);
+        notificationsLogic.createNotification(notif);
+
+        String invalidLongTitle = "1234567890".repeat(10);
+        String validNewMessage = "This should not be reflected.";
+        assertThrows(
+                InvalidParametersException.class,
+                () -> notificationsLogic.updateNotification(
+                        notif.getId(),
+                        Instant.parse("2011-01-01T00:00:00Z"),
+                        Instant.parse("2099-01-01T00:00:00Z"),
+                        NotificationStyle.DANGER,
+                        NotificationTargetUser.GENERAL,
+                        invalidLongTitle,
+                        validNewMessage));
+
+        HibernateUtil.flushSession();
+        HibernateUtil.clearSession();
+        Notification actual = notificationsLogic.getNotification(notif.getId());
+        assertEquals(originalTitle, actual.getTitle());
+        assertEquals(originalMessage, actual.getMessage());
+    }
+
 }
diff --git a/src/main/java/teammates/common/util/HibernateUtil.java b/src/main/java/teammates/common/util/HibernateUtil.java
@@ -294,4 +294,13 @@ public static <T> T getReference(Class<T> entityType, Object id) {
         return getCurrentSession().getReference(entityType, id);
     }
 
+    /**
+     * Flush the current session and evict the given entity from the session.
+     * @see Session#evict(Object)
+     */
+    public static <T> void flushAndEvict(T entity) {
+        flushSession();
+        getCurrentSession().evict(entity);
+    }
+
 }
diff --git a/src/main/java/teammates/sqllogic/core/NotificationsLogic.java b/src/main/java/teammates/sqllogic/core/NotificationsLogic.java
@@ -11,6 +11,7 @@
 import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.exception.InvalidParametersException;
+import teammates.common.util.HibernateUtil;
 import teammates.storage.sqlapi.NotificationsDb;
 import teammates.storage.sqlentity.Notification;
 
@@ -47,6 +48,11 @@ public void initLogicDependencies(NotificationsDb notificationsDb) {
      */
     public Notification createNotification(Notification notification)
             throws InvalidParametersException, EntityAlreadyExistsException {
+        assert notification != null;
+
+        if (!notification.isValid()) {
+            throw new InvalidParametersException(notification.getInvalidityInfo());
+        }
         return notificationsDb.createNotification(notification);
     }
 
@@ -78,6 +84,9 @@ public Notification updateNotification(UUID notificationId, Instant startTime, I
             throw new EntityDoesNotExistException(ERROR_UPDATE_NON_EXISTENT + Notification.class);
         }
 
+        // evict managed entity to avoid auto-persist
+        HibernateUtil.flushAndEvict(notification);
+
         notification.setStartTime(startTime);
         notification.setEndTime(endTime);
         notification.setStyle(style);
@@ -89,6 +98,8 @@ public Notification updateNotification(UUID notificationId, Instant startTime, I
             throw new InvalidParametersException(notification.getInvalidityInfo());
         }
 
+        notificationsDb.updateNotification(notification);
+
         return notification;
     }
 

diff --git a/src/main/java/teammates/storage/sqlapi/EntitiesDb.java b/src/main/java/teammates/storage/sqlapi/EntitiesDb.java
@@ -20,7 +20,7 @@ protected <T extends BaseEntity> T merge(T entity) {
         assert entity != null;
 
         T newEntity = HibernateUtil.merge(entity);
-        log.info("Entity saved: " + entity.toString());
+        log.info("Entity saved: " + newEntity.toString());
         return newEntity;
     }
 

diff --git a/src/main/java/teammates/storage/sqlapi/NotificationsDb.java b/src/main/java/teammates/storage/sqlapi/NotificationsDb.java
@@ -1,5 +1,8 @@
 package teammates.storage.sqlapi;
 
+import static teammates.common.util.Const.ERROR_CREATE_ENTITY_ALREADY_EXISTS;
+import static teammates.common.util.Const.ERROR_UPDATE_NON_EXISTENT;
+
 import java.time.Instant;
 import java.util.List;
 import java.util.UUID;
@@ -11,7 +14,7 @@
 
 import teammates.common.datatransfer.NotificationTargetUser;
 import teammates.common.exception.EntityAlreadyExistsException;
-import teammates.common.exception.InvalidParametersException;
+import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.util.HibernateUtil;
 import teammates.storage.sqlentity.Notification;
 
@@ -34,13 +37,16 @@ public static NotificationsDb inst() {
 
     /**
      * Creates a notification.
+     *
+     * <p>Preconditions:</p>
+     * * Notification fields are valid.
      */
-    public Notification createNotification(Notification notification)
-            throws InvalidParametersException, EntityAlreadyExistsException {
+    public Notification createNotification(Notification notification) throws EntityAlreadyExistsException {
         assert notification != null;
 
-        if (!notification.isValid()) {
-            throw new InvalidParametersException(notification.getInvalidityInfo());
+        if (getNotification(notification.getId()) != null) {
+            throw new EntityAlreadyExistsException(
+                    String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, notification.toString()));
         }
 
         persist(notification);
@@ -98,4 +104,21 @@ public List<Notification> getActiveNotificationsByTargetUser(NotificationTargetU
         TypedQuery<Notification> query = HibernateUtil.createQuery(cq);
         return query.getResultList();
     }
+
+    /**
+     * Updates a notification.
+     *
+     * <p>Preconditions:</p>
+     * * Notification fields are valid.
+     */
+    public Notification updateNotification(Notification notification) throws EntityDoesNotExistException {
+        assert notification != null;
+
+        if (getNotification(notification.getId()) == null) {
+            throw new EntityDoesNotExistException(ERROR_UPDATE_NON_EXISTENT + Notification.class);
+        }
+
+        return merge(notification);
+    }
+
 }
diff --git a/src/test/java/teammates/sqllogic/core/NotificationsLogicTest.java b/src/test/java/teammates/sqllogic/core/NotificationsLogicTest.java
@@ -1,20 +1,26 @@
 package teammates.sqllogic.core;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.time.Instant;
 import java.util.UUID;
 
+import org.mockito.MockedStatic;
+import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import teammates.common.datatransfer.NotificationStyle;
 import teammates.common.datatransfer.NotificationTargetUser;
+import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.exception.InvalidParametersException;
+import teammates.common.util.HibernateUtil;
 import teammates.storage.sqlapi.NotificationsDb;
 import teammates.storage.sqlentity.Notification;
 import teammates.test.BaseTestCase;
@@ -28,10 +34,49 @@ public class NotificationsLogicTest extends BaseTestCase {
 
     private NotificationsDb notificationsDb;
 
+    private MockedStatic<HibernateUtil> mockHibernateUtil;
+
     @BeforeMethod
     public void setUpMethod() {
         notificationsDb = mock(NotificationsDb.class);
         notificationsLogic.initLogicDependencies(notificationsDb);
+        mockHibernateUtil = mockStatic(HibernateUtil.class);
+    }
+
+    @AfterMethod
+    public void tearDownMethod() {
+        mockHibernateUtil.close();
+    }
+
+    @Test
+    public void testCreateNotification_endTimeIsBeforeStartTime_throwsInvalidParametersException()
+            throws EntityAlreadyExistsException {
+        Notification invalidNotification = new Notification(Instant.parse("2011-02-01T00:00:00Z"),
+                Instant.parse("2011-01-01T00:00:00Z"), NotificationStyle.DANGER, NotificationTargetUser.GENERAL,
+                "A deprecation note", "<p>Deprecation happens in three minutes</p>");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        verify(notificationsDb, never()).createNotification(invalidNotification);
+    }
+
+    @Test
+    public void testCreateNotification_emptyTitle_throwsInvalidParametersException() throws EntityAlreadyExistsException {
+        Notification invalidNotification = new Notification(Instant.parse("2011-01-01T00:00:00Z"),
+                Instant.parse("2099-01-01T00:00:00Z"), NotificationStyle.DANGER, NotificationTargetUser.GENERAL,
+                "", "<p>Deprecation happens in three minutes</p>");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        verify(notificationsDb, never()).createNotification(invalidNotification);
+    }
+
+    @Test
+    public void testCreateNotification_emptyMessage_throwsInvalidParametersException() throws EntityAlreadyExistsException {
+        Notification invalidNotification = new Notification(Instant.parse("2011-01-01T00:00:00Z"),
+                Instant.parse("2099-01-01T00:00:00Z"), NotificationStyle.DANGER, NotificationTargetUser.GENERAL,
+                "A deprecation note", "");
+
+        assertThrows(InvalidParametersException.class, () -> notificationsLogic.createNotification(invalidNotification));
+        verify(notificationsDb, never()).createNotification(invalidNotification);
     }
 
     @Test

diff --git a/src/test/java/teammates/storage/sqlapi/AccountRequestsDbTest.java b/src/test/java/teammates/storage/sqlapi/AccountRequestsDbTest.java
@@ -105,6 +105,7 @@ public void testUpdateAccountRequest_success() throws InvalidParametersException
         AccountRequest accountRequest =
                 new AccountRequest("[email protected]", "name", "institute", AccountRequestStatus.PENDING, "comments");
         doReturn(accountRequest).when(accountRequestDb).getAccountRequest(accountRequest.getId());
+        mockHibernateUtil.when(() -> HibernateUtil.merge(accountRequest)).thenReturn(accountRequest);
 
         accountRequestDb.updateAccountRequest(accountRequest);
 

diff --git a/src/test/java/teammates/storage/sqlapi/AccountsDbTest.java b/src/test/java/teammates/storage/sqlapi/AccountsDbTest.java
@@ -114,6 +114,7 @@ public void testUpdateAccount_accountAlreadyExists_success()
         Account account = getTypicalAccount();
         mockHibernateUtil.when(() -> HibernateUtil.get(Account.class, account.getId()))
                 .thenReturn(account);
+        mockHibernateUtil.when(() -> HibernateUtil.merge(account)).thenReturn(account);
         account.setName("new name");
 
         accountsDb.updateAccount(account);

diff --git a/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java b/src/test/java/teammates/storage/sqlapi/CoursesDbTest.java
@@ -117,6 +117,7 @@ public void testUpdateCourse_courseDoesNotExist_throwsEntityDoesNotExistExceptio
     public void testUpdateCourse_success() throws InvalidParametersException, EntityDoesNotExistException {
         Course c = new Course("course-id", "new-course-name", null, "institute");
         doReturn(c).when(coursesDb).getCourse(anyString());
+        mockHibernateUtil.when(() -> HibernateUtil.merge(c)).thenReturn(c);
 
         coursesDb.updateCourse(c);
 

diff --git a/src/test/java/teammates/storage/sqlapi/FeedbackResponseCommentsDbTest.java b/src/test/java/teammates/storage/sqlapi/FeedbackResponseCommentsDbTest.java
@@ -126,6 +126,7 @@ public void testUpdateCourse_success() throws InvalidParametersException, Entity
         comment.setCommentText("Placeholder Text");
 
         doReturn(comment).when(feedbackResponseCommentsDb).getFeedbackResponseComment(anyLong());
+        mockHibernateUtil.when(() -> HibernateUtil.merge(comment)).thenReturn(comment);
         feedbackResponseCommentsDb.updateFeedbackResponseComment(comment);
 
         mockHibernateUtil.verify(() -> HibernateUtil.merge(comment));

diff --git a/src/test/java/teammates/storage/sqlapi/FeedbackSessionsDbTest.java b/src/test/java/teammates/storage/sqlapi/FeedbackSessionsDbTest.java
@@ -106,6 +106,7 @@ public void testGetFeedbackSession_sessionDoesNotExists_returnNull() {
     public void testUpdateFeedbackSession_success() throws InvalidParametersException, EntityDoesNotExistException {
         FeedbackSession feedbackSession = getTypicalFeedbackSessionForCourse(getTypicalCourse());
         doReturn(feedbackSession).when(feedbackSessionsDb).getFeedbackSession(any(UUID.class));
+        mockHibernateUtil.when(() -> HibernateUtil.merge(feedbackSession)).thenReturn(feedbackSession);
 
         feedbackSessionsDb.updateFeedbackSession(feedbackSession);
 
@@ -189,6 +190,7 @@ public void testRestoreDeletedFeedbackSession_success() throws EntityDoesNotExis
         String courseId = feedbackSession.getCourse().getId();
         feedbackSession.setDeletedAt(TimeHelperExtension.getInstantDaysOffsetFromNow(2));
         doReturn(feedbackSession).when(feedbackSessionsDb).getFeedbackSession(sessionName, courseId);
+        mockHibernateUtil.when(() -> HibernateUtil.merge(feedbackSession)).thenReturn(feedbackSession);
 
         feedbackSessionsDb.restoreDeletedFeedbackSession(sessionName, courseId);
 
@@ -215,6 +217,7 @@ public void testSoftDeleteFeedbackSession_success() throws EntityDoesNotExistExc
         String sessionName = feedbackSession.getName();
         String courseId = feedbackSession.getCourse().getId();
         doReturn(feedbackSession).when(feedbackSessionsDb).getFeedbackSession(sessionName, courseId);
+        mockHibernateUtil.when(() -> HibernateUtil.merge(feedbackSession)).thenReturn(feedbackSession);
 
         feedbackSessionsDb.softDeleteFeedbackSession(sessionName, courseId);