Browse Source

Delete old messages with SID when new messages arrive

binwiederhier 1 month ago
parent
commit
75abf2e245
4 changed files with 43 additions and 35 deletions
  1. 5 0
      web/public/sw.js
  2. 22 20
      web/src/app/Poller.js
  3. 11 15
      web/src/app/SubscriptionManager.js
  4. 5 0
      web/src/components/hooks.js

+ 5 - 0
web/public/sw.js

@@ -27,6 +27,11 @@ const addNotification = async ({ subscriptionId, message }) => {
 
   // Note: SubscriptionManager duplicates this logic, so if you change it here, change it there too
 
+  // Delete existing notification with same SID (if any)
+  if (message.sid) {
+    await db.notifications.where({ subscriptionId, sid: message.sid }).delete();
+  }
+
   // Add notification to database
   await db.notifications.add({
     ...messageWithSID(message),

+ 22 - 20
web/src/app/Poller.js

@@ -42,19 +42,22 @@ class Poller {
 
     const since = subscription.last;
     const notifications = await api.poll(subscription.baseUrl, subscription.topic, since);
-    const deletedSids = this.deletedSids(notifications);
-    const newOrUpdatedNotifications = this.newOrUpdatedNotifications(notifications, deletedSids);
+    const latestBySid = this.latestNotificationsBySid(notifications);
 
     // Delete all existing notifications with a deleted sequence ID
+    const deletedSids = Object.entries(latestBySid)
+      .filter(([, notification]) => notification.deleted)
+      .map(([sid]) => sid);
     if (deletedSids.length > 0) {
       console.log(`[Poller] Deleting notifications with deleted sequence IDs for ${subscription.id}`);
       await Promise.all(deletedSids.map((sid) => subscriptionManager.deleteNotificationBySid(subscription.id, sid)));
     }
 
-    // Add new or updated notifications
-    if (newOrUpdatedNotifications.length > 0) {
-      console.log(`[Poller] Adding ${notifications.length} notification(s) for ${subscription.id}`);
-      await subscriptionManager.addNotifications(subscription.id, notifications);
+    // Add only the latest notification for each non-deleted sequence
+    const notificationsToAdd = Object.values(latestBySid).filter((n) => !n.deleted);
+    if (notificationsToAdd.length > 0) {
+      console.log(`[Poller] Adding ${notificationsToAdd.length} notification(s) for ${subscription.id}`);
+      await subscriptionManager.addNotifications(subscription.id, notificationsToAdd);
     } else {
       console.log(`[Poller] No new notifications found for ${subscription.id}`);
     }
@@ -70,20 +73,19 @@ class Poller {
     })();
   }
 
-  deletedSids(notifications) {
-    return new Set(
-      notifications
-        .filter(n => n.sid && n.deleted)
-        .map(n => n.sid)
-    );
-  }
-
-  newOrUpdatedNotifications(notifications, deletedSids) {
-    return notifications
-      .filter((notification) => {
-        const sid = notification.sid || notification.id;
-        return !deletedSids.has(notification.id) && !deletedSids.has(sid) && !notification.deleted;
-      });
+  /**
+   * Groups notifications by sid and returns only the latest (highest time) for each sequence.
+   * Returns an object mapping sid -> latest notification.
+   */
+  latestNotificationsBySid(notifications) {
+    const latestBySid = {};
+    notifications.forEach((notification) => {
+      const sid = notification.sid || notification.id;
+      if (!(sid in latestBySid) || notification.time >= latestBySid[sid].time) {
+        latestBySid[sid] = notification;
+      }
+    });
+    return latestBySid;
   }
 }
 

+ 11 - 15
web/src/app/SubscriptionManager.js

@@ -15,7 +15,7 @@ class SubscriptionManager {
     return Promise.all(
       subscriptions.map(async (s) => ({
         ...s,
-        new: await this.db.notifications.where({ subscriptionId: s.id, new: 1 }).count()
+        new: await this.db.notifications.where({ subscriptionId: s.id, new: 1 }).count(),
       }))
     );
   }
@@ -83,7 +83,7 @@ class SubscriptionManager {
       baseUrl,
       topic,
       mutedUntil: 0,
-      last: null
+      last: null,
     };
 
     await this.db.subscriptions.put(subscription);
@@ -101,7 +101,7 @@ class SubscriptionManager {
 
         const local = await this.add(remote.base_url, remote.topic, {
           displayName: remote.display_name, // May be undefined
-          reservation // May be null!
+          reservation, // May be null!
         });
 
         return local.id;
@@ -157,22 +157,18 @@ class SubscriptionManager {
     // It's actually fine, because the reading and filtering is quite fast. The rendering is what's
     // killing performance. See  https://dexie.org/docs/Collection/Collection.offset()#a-better-paging-approach
 
-    const notifications = await this.db.notifications
+    return this.db.notifications
       .orderBy("time") // Sort by time
       .filter((n) => n.subscriptionId === subscriptionId)
       .reverse()
       .toArray();
-
-    return this.groupNotificationsBySID(notifications);
   }
 
   async getAllNotifications() {
-    const notifications = await this.db.notifications
+    return this.db.notifications
       .orderBy("time") // Efficient, see docs
       .reverse()
       .toArray();
-
-    return this.groupNotificationsBySID(notifications);
   }
 
   // Collapse notification updates based on sids, keeping only the latest version
@@ -204,13 +200,13 @@ class SubscriptionManager {
       await this.db.notifications.add({
         ...messageWithSID(notification),
         subscriptionId,
-        new: 1 // New marker (used for bubble indicator); cannot be boolean; Dexie index limitation
+        new: 1, // New marker (used for bubble indicator); cannot be boolean; Dexie index limitation
       });
 
       // FIXME consider put() for double tab
       // Update subscription last message id (for ?since=... queries)
       await this.db.subscriptions.update(subscriptionId, {
-        last: notification.id
+        last: notification.id,
       });
     } catch (e) {
       console.error(`[SubscriptionManager] Error adding notification`, e);
@@ -226,7 +222,7 @@ class SubscriptionManager {
     const lastNotificationId = notifications.at(-1).id;
     await this.db.notifications.bulkPut(notificationsWithSubscriptionId);
     await this.db.subscriptions.update(subscriptionId, {
-      last: lastNotificationId
+      last: lastNotificationId,
     });
   }
 
@@ -269,19 +265,19 @@ class SubscriptionManager {
 
   async setMutedUntil(subscriptionId, mutedUntil) {
     await this.db.subscriptions.update(subscriptionId, {
-      mutedUntil
+      mutedUntil,
     });
   }
 
   async setDisplayName(subscriptionId, displayName) {
     await this.db.subscriptions.update(subscriptionId, {
-      displayName
+      displayName,
     });
   }
 
   async setReservation(subscriptionId, reservation) {
     await this.db.subscriptions.update(subscriptionId, {
-      reservation
+      reservation,
     });
   }
 

+ 5 - 0
web/src/components/hooks.js

@@ -57,6 +57,11 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop
       };
 
       const handleNewOrUpdatedNotification = async (subscriptionId, notification) => {
+        // Delete existing notification with same sid, if any
+        if (notification.sid) {
+          await subscriptionManager.deleteNotificationBySid(subscriptionId, notification.sid);
+        }
+        // Add notification to database
         const added = await subscriptionManager.addNotification(subscriptionId, notification);
         if (added) {
           await subscriptionManager.notify(subscriptionId, notification);