Development: Improve notifications performance (#7072)

src/main/java/de/tum/in/www1/artemis/repository/GroupNotificationRepository.java

@@ -6,13 +6,12 @@
 import org.springframework.stereotype.Repository;
 
 import de.tum.in.www1.artemis.domain.notification.GroupNotification;
-import de.tum.in.www1.artemis.domain.notification.Notification;
 
 /**
  * Spring Data repository for the Notification entity.
  */
 @Repository
-public interface GroupNotificationRepository extends JpaRepository<Notification, Long> {
+public interface GroupNotificationRepository extends JpaRepository<GroupNotification, Long> {
 
     List<GroupNotification> findAllByCourseId(Long courseId);
 }

src/main/java/de/tum/in/www1/artemis/repository/NotificationRepository.java

@@ -3,14 +3,14 @@
 import java.time.ZonedDateTime;
 import java.util.Set;
 
+import javax.validation.constraints.NotNull;
+
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.jpa.repository.JpaRepository;
-import org.springframework.data.jpa.repository.Modifying;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Repository;
-import org.springframework.transaction.annotation.Transactional;
 
 import de.tum.in.www1.artemis.domain.notification.Notification;
 
@@ -26,8 +26,7 @@ public interface NotificationRepository extends JpaRepository<Notification, Long
                 FROM Notification notification
                     LEFT JOIN notification.course
                     LEFT JOIN notification.recipient
-                WHERE notification.notificationDate IS NOT NULL
-                    AND (cast(:hideUntil as timestamp ) IS NULL OR notification.notificationDate > :hideUntil)
+                WHERE notification.notificationDate > :hideUntil
                     AND (
                         (type(notification) = GroupNotification
                             AND ((notification.course.instructorGroupName IN :#{#currentGroups} AND notification.type = 'INSTRUCTOR')
@@ -44,16 +43,15 @@ OR type(notification) = TutorialGroupNotification and notification.tutorialGroup
                     )
             """)
     Page<Notification> findAllNotificationsForRecipientWithLogin(@Param("currentGroups") Set<String> currentUserGroups, @Param("login") String login,
-            @Param("hideUntil") ZonedDateTime hideUntil, @Param("tutorialGroupIds") Set<Long> tutorialGroupIds,
+            @NotNull @Param("hideUntil") ZonedDateTime hideUntil, @Param("tutorialGroupIds") Set<Long> tutorialGroupIds,
             @Param("titlesToNotLoadNotification") Set<String> titlesToNotLoadNotification, Pageable pageable);
 
     @Query("""
                 SELECT notification
                 FROM Notification notification
                     LEFT JOIN notification.course
                     LEFT JOIN notification.recipient
-                WHERE notification.notificationDate IS NOT NULL
-                    AND (:#{#hideUntil} IS NULL OR notification.notificationDate > :#{#hideUntil})
+                WHERE notification.notificationDate > :hideUntil
                     AND (
                          (type(notification) = GroupNotification
                             AND (notification.title NOT IN :#{#deactivatedTitles}
@@ -81,15 +79,6 @@ Page<Notification> findAllNotificationsForRecipientWithLogin(@Param("currentGrou
                 )
             """)
     Page<Notification> findAllNotificationsFilteredBySettingsForRecipientWithLogin(@Param("currentGroups") Set<String> currentUserGroups, @Param("login") String login,
-            @Param("hideUntil") ZonedDateTime hideUntil, @Param("deactivatedTitles") Set<String> deactivatedTitles, @Param("tutorialGroupIds") Set<Long> tutorialGroupIds,
+            @NotNull @Param("hideUntil") ZonedDateTime hideUntil, @Param("deactivatedTitles") Set<String> deactivatedTitles, @Param("tutorialGroupIds") Set<Long> tutorialGroupIds,
             @Param("titlesToNotLoadNotification") Set<String> titlesToNotLoadNotification, Pageable pageable);
-
-    @Transactional // ok because of modifying query
-    @Modifying
-    @Query("""
-            UPDATE Notification n
-            SET n.author = null
-            WHERE n.author.id = :userId
-            """)
-    void removeAuthor(@Param("userId") long userId);
 }

src/main/java/de/tum/in/www1/artemis/repository/SystemNotificationRepository.java

@@ -16,6 +16,12 @@
 @Repository
 public interface SystemNotificationRepository extends JpaRepository<SystemNotification, Long> {
 
-    @Query("SELECT distinct notification FROM SystemNotification notification WHERE (notification.expireDate >= :now OR notification.expireDate IS NULL) ORDER BY notification.notificationDate ASC")
+    @Query("""
+            SELECT DISTINCT notification
+            FROM SystemNotification notification
+            WHERE notification.expireDate >= :now
+                OR notification.expireDate IS NULL
+            ORDER BY notification.notificationDate ASC
+            """)
     List<SystemNotification> findAllActiveAndFutureSystemNotifications(@Param("now") ZonedDateTime now);
 }

src/main/java/de/tum/in/www1/artemis/web/rest/NotificationResource.java

@@ -32,6 +32,7 @@
 import de.tum.in.www1.artemis.service.notifications.NotificationSettingsCommunicationChannel;
 import de.tum.in.www1.artemis.service.notifications.NotificationSettingsService;
 import de.tum.in.www1.artemis.service.tutorialgroups.TutorialGroupService;
+import de.tum.in.www1.artemis.service.util.TimeLogUtil;
 import io.swagger.annotations.ApiParam;
 import tech.jhipster.web.util.PaginationUtil;
 
@@ -74,14 +75,21 @@ public NotificationResource(NotificationRepository notificationRepository, UserR
     @GetMapping("notifications")
     @EnforceAtLeastStudent
     public ResponseEntity<List<Notification>> getAllNotificationsForCurrentUserFilteredBySettings(@ApiParam Pageable pageable) {
+        long start = System.nanoTime();
         User currentUser = userRepository.getUserWithGroupsAndAuthorities();
+        log.info("REST request to get notifications page {} with size {} for current user {} filtered by settings", pageable.getPageNumber(), pageable.getPageSize(),
+                currentUser.getLogin());
         var tutorialGroupIds = tutorialGroupService.findAllForNotifications(currentUser).stream().map(DomainObject::getId).collect(Collectors.toSet());
-        log.debug("REST request to get all Notifications for current user {} filtered by settings", currentUser);
         Set<NotificationSetting> notificationSettings = notificationSettingRepository.findAllNotificationSettingsForRecipientWithId(currentUser.getId());
         Set<NotificationType> deactivatedTypes = notificationSettingsService.findDeactivatedNotificationTypes(NotificationSettingsCommunicationChannel.WEBAPP,
                 notificationSettings);
         Set<String> deactivatedTitles = notificationSettingsService.convertNotificationTypesToTitles(deactivatedTypes);
-        final ZonedDateTime hideNotificationsUntilDate = currentUser.getHideNotificationsUntil();
+        // Note: at the moment, we only support to show notifications from the last month, because without a proper date the query below becomes too slow
+        var hideNotificationsUntilDate = currentUser.getHideNotificationsUntil();
+        var notificationDateLimit = ZonedDateTime.now().minusMonths(3);
+        if (hideNotificationsUntilDate == null || hideNotificationsUntilDate.isBefore(notificationDateLimit)) {
+            hideNotificationsUntilDate = notificationDateLimit;
+        }
         final Page<Notification> page;
         if (deactivatedTitles.isEmpty()) {
             page = notificationRepository.findAllNotificationsForRecipientWithLogin(currentUser.getGroups(), currentUser.getLogin(), hideNotificationsUntilDate, tutorialGroupIds,
@@ -92,6 +100,7 @@ public ResponseEntity<List<Notification>> getAllNotificationsForCurrentUserFilte
                     deactivatedTitles, tutorialGroupIds, TITLES_TO_NOT_LOAD_NOTIFICATION, pageable);
         }
         HttpHeaders headers = PaginationUtil.generatePaginationHttpHeaders(ServletUriComponentsBuilder.fromCurrentRequest(), page);
+        log.info("Load notifications for user {} done in {}", currentUser.getLogin(), TimeLogUtil.formatDurationFrom(start));
         return new ResponseEntity<>(page.getContent(), headers, HttpStatus.OK);
     }
 }

src/main/resources/config/liquibase/changelog/20230812135300_changelog.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
+    <changeSet author="krusche" id="20230812135300">
+        <createIndex tableName="notification" indexName="notification_discriminator">
+            <column name="discriminator"/>
+        </createIndex>
+        <createIndex tableName="notification" indexName="notification_expire_date">
+            <column name="expire_date"/>
+        </createIndex>
+    </changeSet>
+</databaseChangeLog>

src/main/resources/config/liquibase/master.xml

@@ -48,6 +48,7 @@
     <include file="classpath:config/liquibase/changelog/20230626220000_changelog.xml" relativeToChangelogFile="false"/>
     <include file="classpath:config/liquibase/changelog/20230629194400_changelog.xml" relativeToChangelogFile="false"/>
     <include file="classpath:config/liquibase/changelog/20230721135400_changelog.xml" relativeToChangelogFile="false"/>
+    <include file="classpath:config/liquibase/changelog/20230812135300_changelog.xml" relativeToChangelogFile="false"/>
     <!-- NOTE: please use the format "YYYYMMDDhhmmss_changelog.xml", i.e. year month day hour minutes seconds and not something else! -->
     <!-- we should also stay in a chronological order! -->
     <!-- you can use the command 'date '+%Y%m%d%H%M%S'' to get the current date and time in the correct format -->