From 02be719ce2247c6969101931e4ebcc1ba2af7444 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Thu, 29 Oct 2026 11:31:45 +0000 Subject: [PATCH 1/4] Web app: don't add deleted notifications again when a poll replays them The server replays the whole second of a since= marker, so a poll returns messages the user deleted. Remember deleted notifications and skip them (and older versions of their sequence), and don't move the marker back when a poll only replayed known messages, which made each poll replay a different part of that second. Fixes #2098 --- docs/releases.md | 6 +-- web/src/app/SubscriptionManager.js | 54 ++++++++++++++++--- web/src/app/SubscriptionManager.test.js | 72 +++++++++++++++++++++++++ web/src/app/db.js | 5 ++ 4 files changed, 126 insertions(+), 11 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index ce56a608..0b880b6c 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2274,15 +2274,13 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release * A user without a tier shares its rate limits with anonymous requests from the same IP, and a login from that IP could switch off the auth failure and account action limits for a concurrent request ([#2119](https://github.com/binwiederhier/ntfy/pull/2119)) * A tier with a daily email limit of zero sends no emails, a tier edited in place no longer loses pending user stats, and a clock that goes back no longer takes tokens away +* Accept terabyte units (e.g. `1T`) in size settings such as attachment and tier limits, matching how sizes are displayed ([#2078](https://github.com/binwiederhier/ntfy/pull/2078), thanks to [@cangen185-sudo](https://github.com/cangen185-sudo) for the contribution) +* Web app: Deleted notifications no longer come back when a poll replays messages published in the same second as the last one ([#2098](https://github.com/binwiederhier/ntfy/issues/2098), thanks to [@mattford63](https://github.com/mattford63) and [@arminus](https://github.com/arminus) for reporting) **Security:** * Reject Matrix push gateway requests whose push key does not name a valid topic. Publishing strips NUL bytes and invalid UTF-8 from the topic after authorization, so an anonymous push key like `/sec%00ret` was authorized against an unmatched topic and then stored under the protected topic `secret` (SQLite only; on PostgreSQL the request was denied) -**Bug fixes + maintenance:** - -* Accept terabyte units (e.g. `1T`) in size settings such as attachment and tier limits, matching how sizes are displayed ([#2078](https://github.com/binwiederhier/ntfy/pull/2078), thanks to [@cangen185-sudo](https://github.com/cangen185-sudo) for the contribution) - ### ntfy iOS app v1.8.0 (UNRELEASED) **Features:** diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index e1776833..7e28824f 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -163,6 +163,7 @@ export class SubscriptionManager { async remove(subscription) { await this.db.subscriptions.delete(subscription.id); await this.db.notifications.where({ subscriptionId: subscription.id }).delete(); + await this.db.deleted.where({ subscriptionId: subscription.id }).delete(); } async first() { @@ -199,8 +200,8 @@ export class SubscriptionManager { // Add notification to database; check and add in one transaction, so a poll storing the // same message concurrently (see addNotifications) can't slip in between - const added = await this.db.transaction("rw", this.db.notifications, async () => { - if (await this.db.notifications.get(notification.id)) { + const added = await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { + if ((await this.db.notifications.get(notification.id)) || (await this.wasDeleted(subscriptionId, notification))) { return false; } await this.db.notifications.add({ @@ -230,12 +231,13 @@ export class SubscriptionManager { * older versions of the same sequence; will not throw if they exist */ async addNotifications(subscriptionId, notifications) { - await this.db.transaction("rw", this.db.notifications, async () => { + const added = await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { // Skip notifications that are already stored (e.g. delivered via WebSocket while this poll - // was in flight), so overwriting them doesn't drop their "new" marker + // was in flight), so overwriting them doesn't drop their "new" marker, and ones the user deleted const existing = await this.db.notifications.bulkGet(notifications.map((n) => n.id)); + const deleted = await Promise.all(notifications.map((n) => this.wasDeleted(subscriptionId, n))); const notificationsWithSubscriptionId = notifications - .filter((_, i) => !existing[i]) + .filter((_, i) => !existing[i] && !deleted[i]) .map((notification) => ({ ...messageWithSequenceId(notification), subscriptionId, @@ -252,7 +254,14 @@ export class SubscriptionManager { const replaced = versions.flat().filter((v) => updates.some((n) => n.sequenceId === v.sequenceId)); await this.db.notifications.bulkDelete(replaced.map((v) => v.id)); await this.db.notifications.bulkPut(updates); + return notificationsWithSubscriptionId.length > 0; }); + + // Don't move the marker if the poll only replayed known messages: a replay of the marker's second + // ends with a message older than the marker, and moving back to it replays the marker again + if (!added) { + return; + } const lastNotificationId = notifications.at(-1).id; await this.db.subscriptions.update(subscriptionId, { last: lastNotificationId, @@ -273,7 +282,13 @@ export class SubscriptionManager { } async deleteNotification(notificationId) { - await this.db.notifications.delete(notificationId); + await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { + const notification = await this.db.notifications.get(notificationId); + if (notification) { + await this.db.deleted.put(deletedNotification(notification)); + await this.db.notifications.delete(notificationId); + } + }); } async deleteNotificationBySequenceId(subscriptionId, sequenceId) { @@ -281,7 +296,24 @@ export class SubscriptionManager { } async deleteNotifications(subscriptionId) { - await this.db.notifications.where({ subscriptionId }).delete(); + await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { + const notifications = await this.db.notifications.where({ subscriptionId }).toArray(); + await this.db.deleted.bulkPut(notifications.map(deletedNotification)); + await this.db.notifications.where({ subscriptionId }).delete(); + }); + } + + /** + * Whether the user deleted this message, or a newer version of its sequence. The server replays + * the whole second of a since= marker, so a poll can return messages the user deleted. + */ + async wasDeleted(subscriptionId, notification) { + if (await this.db.deleted.get(notification.id)) { + return true; + } + const sequenceId = notification.sequence_id || notification.id; + const versions = await this.db.deleted.where({ subscriptionId, sequenceId }).toArray(); + return versions.some((v) => notification.time < v.time); } async markNotificationRead(notificationId) { @@ -320,7 +352,15 @@ export class SubscriptionManager { async pruneNotifications(thresholdTimestamp) { await this.db.notifications.where("time").below(thresholdTimestamp).delete(); + await this.db.deleted.where("time").below(thresholdTimestamp).delete(); } } +const deletedNotification = (notification) => ({ + id: notification.id, + subscriptionId: notification.subscriptionId, + sequenceId: notification.sequenceId, + time: notification.time, +}); + export default new SubscriptionManager(db()); diff --git a/web/src/app/SubscriptionManager.test.js b/web/src/app/SubscriptionManager.test.js index 15098463..8311093c 100644 --- a/web/src/app/SubscriptionManager.test.js +++ b/web/src/app/SubscriptionManager.test.js @@ -233,3 +233,75 @@ describe("SubscriptionManager.addNotifications", () => { expect((await db.notifications.toArray()).map((n) => n.message)).toEqual(["Download complete"]); }); }); + +// The server replays the whole second of a since= marker, so a poll returns messages the +// client already has, or had and deleted (#2098) +describe("SubscriptionManager deleted notifications", () => { + it("does not add a deleted message again when a poll replays it", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("m1"), message("m2"), message("m3")]); + await manager.deleteNotification("m1"); + await manager.deleteNotification("m2"); + + await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); + + expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m3"]); + }); + + it("does not add a deleted message again when the WebSocket replays it", async () => { + const { db, manager } = await setup(); + await manager.addNotification(subscriptionId, message("m1")); + await manager.deleteNotification("m1"); + + expect(await manager.addNotification(subscriptionId, message("m1"))).toBe(false); + + expect(await db.notifications.count()).toBe(0); + }); + + it("does not add older versions of a deleted sequence again, but newer ones", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("m2", { sequence_id: "s1", message: "Download complete", time: 1700000001 })]); + await manager.deleteNotification("m2"); + + await manager.addNotifications(subscriptionId, [message("m1", { sequence_id: "s1", message: "Downloading" })]); + expect(await db.notifications.count()).toBe(0); + + await manager.addNotification(subscriptionId, message("m3", { sequence_id: "s1", message: "Download failed", time: 1700000002 })); + expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m3"]); + }); + + it("does not add messages again after deleting all of them", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); + await manager.deleteNotifications(subscriptionId); + + await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); + + expect(await db.notifications.count()).toBe(0); + }); + + it("keeps the last message when a poll only replays known or deleted messages", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("m1"), message("m2"), message("m3")]); + await manager.deleteNotification("m1"); + + await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); + + expect((await db.subscriptions.get(subscriptionId)).last).toBe("m3"); + }); + + it("forgets deleted messages when the subscription is removed or they are pruned", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("old", { time: 1600000000 }), message("m1")]); + await manager.deleteNotifications(subscriptionId); + + await manager.pruneNotifications(1650000000); + await manager.addNotifications(subscriptionId, [message("old", { time: 1600000000 })]); + expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["old"]); + + await manager.remove({ id: subscriptionId }); + await manager.upsert(baseUrl, "mytopic"); + await manager.addNotifications(subscriptionId, [message("m1")]); + expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m1"]); + }); +}); diff --git a/web/src/app/db.js b/web/src/app/db.js index e088a267..132936c6 100644 --- a/web/src/app/db.js +++ b/web/src/app/db.js @@ -18,6 +18,11 @@ const createDatabase = (username) => { prefs: "&key", }); + // Deleted notifications, so that a replay of the marker's second (since=) doesn't add them again + db.version(4).stores({ + deleted: "&id,subscriptionId,time,[subscriptionId+sequenceId]", + }); + // When another connection (e.g., service worker or another tab) wants to upgrade, // close this connection gracefully to allow the upgrade to proceed db.on("versionchange", () => { From a96b35e242660de37a2896275e99b3b1c08a97e5 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 30 Oct 2026 20:45:30 +0000 Subject: [PATCH 2/4] Web app: keep read state of a replayed WebSocket message The WebSocket handler deleted every version of a message's sequence before adding it, so a replayed message that was already stored came back as unread and notified again, and a replayed older version replaced a newer one. addNotification now replaces older versions of the sequence itself, and skips a message if a newer version is stored. Fixes #2090 --- docs/releases.md | 1 + web/src/app/SubscriptionManager.js | 9 +++++++++ web/src/app/SubscriptionManager.test.js | 21 +++++++++++++++++++++ web/src/components/hooks.js | 7 ++----- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index 0b880b6c..385d299f 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2276,6 +2276,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release * A tier with a daily email limit of zero sends no emails, a tier edited in place no longer loses pending user stats, and a clock that goes back no longer takes tokens away * Accept terabyte units (e.g. `1T`) in size settings such as attachment and tier limits, matching how sizes are displayed ([#2078](https://github.com/binwiederhier/ntfy/pull/2078), thanks to [@cangen185-sudo](https://github.com/cangen185-sudo) for the contribution) * Web app: Deleted notifications no longer come back when a poll replays messages published in the same second as the last one ([#2098](https://github.com/binwiederhier/ntfy/issues/2098), thanks to [@mattford63](https://github.com/mattford63) and [@arminus](https://github.com/arminus) for reporting) +* Web app: A message replayed after a reconnect keeps its read state and no longer notifies again, and an older version of a sequence no longer replaces a newer one ([#2090](https://github.com/binwiederhier/ntfy/issues/2090), thanks to [@tobykaspar79](https://github.com/tobykaspar79) for reporting) **Security:** diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index 7e28824f..33456571 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -204,6 +204,15 @@ export class SubscriptionManager { if ((await this.db.notifications.get(notification.id)) || (await this.wasDeleted(subscriptionId, notification))) { return false; } + + // An update replaces the stored versions of its sequence, unless a newer one is stored + // already (e.g. a reconnect replayed an older version) + const sequenceId = notification.sequence_id || notification.id; + const versions = await this.db.notifications.where({ subscriptionId, sequenceId }).toArray(); + if (versions.some((v) => v.time > notification.time)) { + return false; + } + await this.db.notifications.bulkDelete(versions.map((v) => v.id)); await this.db.notifications.add({ ...messageWithSequenceId(notification), subscriptionId, diff --git a/web/src/app/SubscriptionManager.test.js b/web/src/app/SubscriptionManager.test.js index 8311093c..277d1115 100644 --- a/web/src/app/SubscriptionManager.test.js +++ b/web/src/app/SubscriptionManager.test.js @@ -158,6 +158,27 @@ describe("SubscriptionManager.addNotification", () => { expect((await db.subscriptions.get(subscriptionId)).last).toBe("m2"); }); + it("replaces an older version of the same sequence", async () => { + const { db, manager } = await setup(); + await manager.addNotification(subscriptionId, message("m1", { sequence_id: "s1", message: "Downloading" })); + + expect( + await manager.addNotification(subscriptionId, message("m2", { sequence_id: "s1", message: "Download complete", time: 1700000001 })), + ).toBe(true); + + expect((await db.notifications.toArray()).map((n) => [n.id, n.new])).toEqual([["m2", 1]]); + }); + + it("does not replace a newer version of the same sequence when the WebSocket replays an older one", async () => { + const { db, manager } = await setup(); + await manager.addNotification(subscriptionId, message("m2", { sequence_id: "s1", message: "Download complete", time: 1700000001 })); + + expect(await manager.addNotification(subscriptionId, message("m1", { sequence_id: "s1", message: "Downloading" }))).toBe(false); + + expect((await db.notifications.toArray()).map((n) => n.message)).toEqual(["Download complete"]); + expect((await db.subscriptions.get(subscriptionId)).last).toBe("m2"); + }); + it("does not store delete and clear events", async () => { const { db, manager } = await setup(); expect(await manager.addNotification(subscriptionId, message("d1", { event: "message_delete", sequence_id: "s1" }))).toBe(false); diff --git a/web/src/components/hooks.js b/web/src/components/hooks.js index ba62f54f..150e7540 100644 --- a/web/src/components/hooks.js +++ b/web/src/components/hooks.js @@ -65,11 +65,8 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop await subscriptionManager.markNotificationReadBySequenceId(subscription.id, notification.sequence_id); await notifier.cancel(subscription, notification); } else { - // Regular message: delete existing and add new - const sequenceId = notification.sequence_id || notification.id; - if (sequenceId) { - await subscriptionManager.deleteNotificationBySequenceId(subscription.id, sequenceId); - } + // Regular message: add it, replacing older versions of its sequence; a replayed duplicate + // keeps its local state (e.g. read) and doesn't notify again const added = await subscriptionManager.addNotification(subscription.id, notification); if (added) { await subscriptionManager.notify(subscription.id, notification); From c43c475be33b32c94c6279b154e2ed6c8f8d021d Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 30 Oct 2026 20:54:03 +0000 Subject: [PATCH 3/4] Web app: mark deleted notifications instead of keeping a separate store Like the Android app, a deleted notification stays in the notifications table with a deleted flag, so the existing duplicate checks skip it when the server replays it, and a deleted newer version keeps an older one out. The lists leave deleted notifications out, and they are pruned with the others. The service worker does not store a deleted message again. --- web/public/sw.js | 5 +++ web/src/app/SubscriptionManager.js | 57 +++++++------------------ web/src/app/SubscriptionManager.test.js | 36 ++++++++++------ web/src/app/db.js | 5 --- 4 files changed, 44 insertions(+), 59 deletions(-) diff --git a/web/public/sw.js b/web/public/sw.js index 1c0235d3..2730ec5a 100644 --- a/web/public/sw.js +++ b/web/public/sw.js @@ -64,6 +64,11 @@ const handlePushMessage = async (data) => { }), ); + // Don't store a message again that the user deleted (e.g. delivered via WebSocket first) + if ((await db.notifications.get(message.id))?.deleted) { + return; + } + // Delete existing notification with same sequence ID (if any) const sequenceId = message.sequence_id || message.id; if (sequenceId) { diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index 33456571..8681f25e 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -163,7 +163,6 @@ export class SubscriptionManager { async remove(subscription) { await this.db.subscriptions.delete(subscription.id); await this.db.notifications.where({ subscriptionId: subscription.id }).delete(); - await this.db.deleted.where({ subscriptionId: subscription.id }).delete(); } async first() { @@ -177,7 +176,7 @@ export class SubscriptionManager { return this.db.notifications .orderBy("time") // Sort by time - .filter((n) => n.subscriptionId === subscriptionId) + .filter((n) => n.subscriptionId === subscriptionId && !n.deleted) .reverse() .toArray(); } @@ -185,6 +184,7 @@ export class SubscriptionManager { async getAllNotifications() { return this.db.notifications .orderBy("time") // Efficient, see docs + .filter((n) => !n.deleted) .reverse() .toArray(); } @@ -200,13 +200,13 @@ export class SubscriptionManager { // Add notification to database; check and add in one transaction, so a poll storing the // same message concurrently (see addNotifications) can't slip in between - const added = await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { - if ((await this.db.notifications.get(notification.id)) || (await this.wasDeleted(subscriptionId, notification))) { + const added = await this.db.transaction("rw", this.db.notifications, async () => { + if (await this.db.notifications.get(notification.id)) { return false; } // An update replaces the stored versions of its sequence, unless a newer one is stored - // already (e.g. a reconnect replayed an older version) + // already (e.g. a reconnect replayed an older version), or was deleted const sequenceId = notification.sequence_id || notification.id; const versions = await this.db.notifications.where({ subscriptionId, sequenceId }).toArray(); if (versions.some((v) => v.time > notification.time)) { @@ -240,13 +240,12 @@ export class SubscriptionManager { * older versions of the same sequence; will not throw if they exist */ async addNotifications(subscriptionId, notifications) { - const added = await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { + const added = await this.db.transaction("rw", this.db.notifications, async () => { // Skip notifications that are already stored (e.g. delivered via WebSocket while this poll - // was in flight), so overwriting them doesn't drop their "new" marker, and ones the user deleted + // was in flight), so overwriting them doesn't drop their "new" marker, or deleted const existing = await this.db.notifications.bulkGet(notifications.map((n) => n.id)); - const deleted = await Promise.all(notifications.map((n) => this.wasDeleted(subscriptionId, n))); const notificationsWithSubscriptionId = notifications - .filter((_, i) => !existing[i] && !deleted[i]) + .filter((_, i) => !existing[i]) .map((notification) => ({ ...messageWithSequenceId(notification), subscriptionId, @@ -290,14 +289,13 @@ export class SubscriptionManager { return true; } + /** + * Marks a notification as deleted instead of removing it: the server replays the whole second of a + * since= marker, so a poll can return it again, and the stored row keeps it from being re-added. + * Deleted notifications are pruned with the others. + */ async deleteNotification(notificationId) { - await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { - const notification = await this.db.notifications.get(notificationId); - if (notification) { - await this.db.deleted.put(deletedNotification(notification)); - await this.db.notifications.delete(notificationId); - } - }); + await this.db.notifications.where({ id: notificationId }).modify({ deleted: 1, new: 0 }); } async deleteNotificationBySequenceId(subscriptionId, sequenceId) { @@ -305,24 +303,7 @@ export class SubscriptionManager { } async deleteNotifications(subscriptionId) { - await this.db.transaction("rw", this.db.notifications, this.db.deleted, async () => { - const notifications = await this.db.notifications.where({ subscriptionId }).toArray(); - await this.db.deleted.bulkPut(notifications.map(deletedNotification)); - await this.db.notifications.where({ subscriptionId }).delete(); - }); - } - - /** - * Whether the user deleted this message, or a newer version of its sequence. The server replays - * the whole second of a since= marker, so a poll can return messages the user deleted. - */ - async wasDeleted(subscriptionId, notification) { - if (await this.db.deleted.get(notification.id)) { - return true; - } - const sequenceId = notification.sequence_id || notification.id; - const versions = await this.db.deleted.where({ subscriptionId, sequenceId }).toArray(); - return versions.some((v) => notification.time < v.time); + await this.db.notifications.where({ subscriptionId }).modify({ deleted: 1, new: 0 }); } async markNotificationRead(notificationId) { @@ -361,15 +342,7 @@ export class SubscriptionManager { async pruneNotifications(thresholdTimestamp) { await this.db.notifications.where("time").below(thresholdTimestamp).delete(); - await this.db.deleted.where("time").below(thresholdTimestamp).delete(); } } -const deletedNotification = (notification) => ({ - id: notification.id, - subscriptionId: notification.subscriptionId, - sequenceId: notification.sequenceId, - time: notification.time, -}); - export default new SubscriptionManager(db()); diff --git a/web/src/app/SubscriptionManager.test.js b/web/src/app/SubscriptionManager.test.js index 277d1115..53aecd0b 100644 --- a/web/src/app/SubscriptionManager.test.js +++ b/web/src/app/SubscriptionManager.test.js @@ -255,50 +255,52 @@ describe("SubscriptionManager.addNotifications", () => { }); }); +const visibleIds = async (manager) => (await manager.getNotifications(subscriptionId)).map((n) => n.id).sort(); + // The server replays the whole second of a since= marker, so a poll returns messages the // client already has, or had and deleted (#2098) describe("SubscriptionManager deleted notifications", () => { it("does not add a deleted message again when a poll replays it", async () => { - const { db, manager } = await setup(); + const { manager } = await setup(); await manager.addNotifications(subscriptionId, [message("m1"), message("m2"), message("m3")]); await manager.deleteNotification("m1"); await manager.deleteNotification("m2"); await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); - expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m3"]); + expect(await visibleIds(manager)).toEqual(["m3"]); }); it("does not add a deleted message again when the WebSocket replays it", async () => { - const { db, manager } = await setup(); + const { manager } = await setup(); await manager.addNotification(subscriptionId, message("m1")); await manager.deleteNotification("m1"); expect(await manager.addNotification(subscriptionId, message("m1"))).toBe(false); - expect(await db.notifications.count()).toBe(0); + expect(await visibleIds(manager)).toEqual([]); }); it("does not add older versions of a deleted sequence again, but newer ones", async () => { - const { db, manager } = await setup(); + const { manager } = await setup(); await manager.addNotifications(subscriptionId, [message("m2", { sequence_id: "s1", message: "Download complete", time: 1700000001 })]); await manager.deleteNotification("m2"); await manager.addNotifications(subscriptionId, [message("m1", { sequence_id: "s1", message: "Downloading" })]); - expect(await db.notifications.count()).toBe(0); + expect(await visibleIds(manager)).toEqual([]); await manager.addNotification(subscriptionId, message("m3", { sequence_id: "s1", message: "Download failed", time: 1700000002 })); - expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m3"]); + expect(await visibleIds(manager)).toEqual(["m3"]); }); it("does not add messages again after deleting all of them", async () => { - const { db, manager } = await setup(); + const { manager } = await setup(); await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); await manager.deleteNotifications(subscriptionId); await manager.addNotifications(subscriptionId, [message("m1"), message("m2")]); - expect(await db.notifications.count()).toBe(0); + expect(await visibleIds(manager)).toEqual([]); }); it("keeps the last message when a poll only replays known or deleted messages", async () => { @@ -311,18 +313,28 @@ describe("SubscriptionManager deleted notifications", () => { expect((await db.subscriptions.get(subscriptionId)).last).toBe("m3"); }); + it("does not list deleted messages or count them as new", async () => { + const { manager } = await setup(); + await manager.addNotification(subscriptionId, message("m1")); + await manager.addNotification(subscriptionId, message("m2")); + await manager.deleteNotification("m1"); + + expect((await manager.getAllNotifications()).map((n) => n.id)).toEqual(["m2"]); + expect((await manager.all()).find((s) => s.id === subscriptionId).new).toBe(1); + }); + it("forgets deleted messages when the subscription is removed or they are pruned", async () => { - const { db, manager } = await setup(); + const { manager } = await setup(); await manager.addNotifications(subscriptionId, [message("old", { time: 1600000000 }), message("m1")]); await manager.deleteNotifications(subscriptionId); await manager.pruneNotifications(1650000000); await manager.addNotifications(subscriptionId, [message("old", { time: 1600000000 })]); - expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["old"]); + expect(await visibleIds(manager)).toEqual(["old"]); await manager.remove({ id: subscriptionId }); await manager.upsert(baseUrl, "mytopic"); await manager.addNotifications(subscriptionId, [message("m1")]); - expect((await db.notifications.toArray()).map((n) => n.id)).toEqual(["m1"]); + expect(await visibleIds(manager)).toEqual(["m1"]); }); }); diff --git a/web/src/app/db.js b/web/src/app/db.js index 132936c6..e088a267 100644 --- a/web/src/app/db.js +++ b/web/src/app/db.js @@ -18,11 +18,6 @@ const createDatabase = (username) => { prefs: "&key", }); - // Deleted notifications, so that a replay of the marker's second (since=) doesn't add them again - db.version(4).stores({ - deleted: "&id,subscriptionId,time,[subscriptionId+sequenceId]", - }); - // When another connection (e.g., service worker or another tab) wants to upgrade, // close this connection gracefully to allow the upgrade to proceed db.on("versionchange", () => { From e6a6049d231b0fdd2d8d831088038057ed462119 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 30 Oct 2026 21:14:26 +0000 Subject: [PATCH 4/4] Web app: notify for a message only a poll stored, and test replays end to end A message the poll stored before its live delivery has no unread marker yet, so the WebSocket delivery that follows marks it unread and notifies again, while a replayed message that came in live keeps its state. The e2e tests publish within one second and reload, so the reconnect and the startup poll replay the earlier messages: deleted ones stay gone, and read ones stay read without a new browser notification. --- web/e2e/notifications.spec.js | 68 +++++++++++++++++++++++++ web/src/app/SubscriptionManager.js | 8 ++- web/src/app/SubscriptionManager.test.js | 10 ++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/web/e2e/notifications.spec.js b/web/e2e/notifications.spec.js index 9ce83ea7..5f422100 100644 --- a/web/e2e/notifications.spec.js +++ b/web/e2e/notifications.spec.js @@ -9,6 +9,31 @@ let topic; // Browser notifications the app showed, recorded by the init script in beforeEach const shown = (page) => page.evaluate(() => window.shownNotifications.map((n) => n.body)); +// Publishes messages within one second, right after a second starts. A since= marker replays +// every message from its second, so the earlier ones come back to a client that has the last one. +const publishInOneSecond = async (request, topic, messages) => { + await new Promise((resolve) => setTimeout(resolve, 1000 - (Date.now() % 1000))); + const times = []; + for (const message of messages) { + times.push((await (await request.post(`/${topic}`, { data: message })).json()).time); + } + expect(new Set(times).size, `published in one second: ${times}`).toBe(1); +}; + +// Messages the topic's WebSocket received from the server, from now on +const liveMessages = (page, topic) => { + const received = []; + page.on("websocket", (ws) => { + if (new URL(ws.url()).pathname.endsWith(`/${topic}/ws`)) { + ws.on("framereceived", (frame) => { + const event = JSON.parse(frame.payload); + if (event.event === "message") received.push(event.message); + }); + } + }); + return received; +}; + test.beforeEach(async ({ page }) => { // Record browser notifications instead of showing them; headless Chrome can't show them anyway await page.addInitScript(() => { @@ -268,6 +293,49 @@ test.describe("sequences", () => { }); }); +test.describe("replays", () => { + test("deleted messages don't come back when a reconnect replays them", async ({ page, request }) => { + await publishInOneSecond(request, topic, ["first", "second", "third"]); + const items = page.getByRole("listitem", { name: "Notification" }); + await expect(items).toHaveCount(3); + for (const message of ["first", "second", "third"]) { + await items.filter({ hasText: message }).getByRole("button", { name: "Delete" }).click(); + } + await expect(items).toHaveCount(0); + + // Reconnecting and the startup poll ask for since=, and get the rest of its second again + const live = liveMessages(page, topic); + const polled = page.waitForResponse((r) => new URL(r.url()).pathname.endsWith(`/${topic}/json`)); + await page.reload(); + await page.clock.runFor(2000); + expect(await (await polled).text()).toContain("first"); + await expect.poll(() => live).toContain("first"); + await page.waitForTimeout(1000); // Let the app store what it got + await expect(items).toHaveCount(0); + await expect(page.getByText("You haven't received any notifications for this topic yet.")).toBeVisible(); + }); + + test("a message replayed on reconnect stays read and doesn't notify again", async ({ page, request }) => { + await publishInOneSecond(request, topic, ["first", "second"]); + await expect(page.getByRole("listitem", { name: "Notification" })).toHaveCount(2); + + // Read them by opening the topic, then reconnect from another one, so one that comes back unread is counted + const other = unique("other"); + await subscribe(page, other); + await expect(page).toHaveTitle("(2) ntfy"); + await page.getByRole("button", { name: topic, exact: true }).click(); + await expect(page).toHaveTitle("ntfy"); + await page.getByRole("button", { name: other, exact: true }).click(); + const live = liveMessages(page, topic); + await page.reload(); + await page.clock.runFor(2000); + await expect.poll(() => live).toContain("first"); + await page.waitForTimeout(1000); // Let the app store what it got + await expect(page).toHaveTitle("ntfy"); + expect(await shown(page)).toEqual([]); + }); +}); + test.describe("browser notifications", () => { test("a new message is shown", async ({ page, request }) => { await request.post(`/${topic}`, { data: "pop up" }); diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index 8681f25e..6014a3a4 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -201,7 +201,13 @@ export class SubscriptionManager { // Add notification to database; check and add in one transaction, so a poll storing the // same message concurrently (see addNotifications) can't slip in between const added = await this.db.transaction("rw", this.db.notifications, async () => { - if (await this.db.notifications.get(notification.id)) { + // A replayed message keeps its local state (e.g. read or deleted), but one that only a poll + // stored (no "new" marker, see addNotifications) wasn't delivered live yet, so it's new + const existing = await this.db.notifications.get(notification.id); + if (existing && existing.new === undefined) { + await this.db.notifications.update(notification.id, { new: 1 }); + return true; + } else if (existing) { return false; } diff --git a/web/src/app/SubscriptionManager.test.js b/web/src/app/SubscriptionManager.test.js index 53aecd0b..82f3b169 100644 --- a/web/src/app/SubscriptionManager.test.js +++ b/web/src/app/SubscriptionManager.test.js @@ -179,6 +179,16 @@ describe("SubscriptionManager.addNotification", () => { expect((await db.subscriptions.get(subscriptionId)).last).toBe("m2"); }); + it("marks a message the poll stored as unread when the WebSocket delivers it", async () => { + const { db, manager } = await setup(); + await manager.addNotifications(subscriptionId, [message("m1")]); + + // The poll stored it before the live delivery arrived; the user hasn't seen it yet + expect(await manager.addNotification(subscriptionId, message("m1"))).toBe(true); + + expect((await db.notifications.get("m1")).new).toBe(1); + }); + it("does not store delete and clear events", async () => { const { db, manager } = await setup(); expect(await manager.addNotification(subscriptionId, message("d1", { event: "message_delete", sequence_id: "s1" }))).toBe(false);