chore: remove dead notification surfaces (PUT-1669) (#3673)

src/backend/controllers/notification/NotificationController.http.test.ts

@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2024-present Puter Technologies Inc.
+ *
+ * This file is part of Puter.
+ *
+ * Puter is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published
+ * by the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js';
+
+/**
+ * Route-level coverage for the notification endpoint. Dismissing is the only
+ * mark the desktop performs over HTTP, so this suite is what catches the route
+ * going missing or a gate turning a legitimate dismiss away.
+ */
+describe('notification endpoints over HTTP', () => {
+    let env: PuterTestEnv;
+
+    beforeAll(async () => {
+        env = await setupPuterTestEnv();
+    }, 120_000);
+
+    afterAll(async () => {
+        await env?.shutdown();
+    });
+
+    const post = (path: string, token: string, body: unknown) =>
+        fetch(new URL(path, env.apiOrigin), {
+            method: 'POST',
+            headers: {
+                'content-type': 'application/json',
+                authorization: `Bearer ${token}`,
+            },
+            body: JSON.stringify(body),
+        });
+
+    const seedNotification = async (username: string) => {
+        const user = await env.server.stores.user.getByUsername(username);
+        const row = await env.server.stores.notification.create({
+            userId: user!.id,
+            value: { title: 'dismiss me' },
+            type: 'share.received',
+        });
+        return { userId: user!.id as number, uid: row.uid as string };
+    };
+
+    it('dismisses a notification', async () => {
+        const owner = env.users.user;
+        const { userId, uid } = await seedNotification(owner.username);
+
+        const res = await post('/notif/mark-ack', owner.token, { uid });
+
+        expect(res.status).toBe(200);
+        await expect(res.json()).resolves.toEqual({});
+        const after = await env.server.stores.notification.getByUid(uid, {
+            userId,
+        });
+        expect(after?.acknowledged).toBeTruthy();
+    });
+
+    it('refuses an anonymous dismiss', async () => {
+        const { userId, uid } = await seedNotification(env.users.other.username);
+
+        const res = await fetch(new URL('/notif/mark-ack', env.apiOrigin), {
+            method: 'POST',
+            headers: { 'content-type': 'application/json' },
+            body: JSON.stringify({ uid }),
+        });
+
+        expect(res.status).toBeGreaterThanOrEqual(400);
+        const after = await env.server.stores.notification.getByUid(uid, {
+            userId,
+        });
+        expect(after?.acknowledged).toBeFalsy();
+    });
+});

src/backend/controllers/notification/NotificationController.test.ts

@@ -29,10 +29,9 @@ import type { NotificationController } from './NotificationController.js';
 //
 // Boots one PuterServer with the live wired NotificationController.
 // Tests seed real notification rows via the store, then drive the
-// controller's `markAck` / `markRead` handlers with stub req/res
-// objects. The controller's path through NotificationService updates
-// the underlying row, so we verify behaviour by reading the store
-// state back.
+// controller's `markAck` handler with stub req/res objects. The
+// controller's path through NotificationService updates the underlying
+// row, so we verify behaviour by reading the store state back.
 
 let server: PuterServer;
 let controller: NotificationController;
@@ -138,6 +137,14 @@ describe('NotificationController.markAck', () => {
         ).rejects.toMatchObject({ statusCode: 400 });
     });
 
+    it('rejects an empty uid string with 400', async () => {
+        const { actor } = await makeUser();
+        const { res } = makeRes();
+        await expect(
+            controller.markAck(makeReq({ body: { uid: '' }, actor }), res),
+        ).rejects.toMatchObject({ statusCode: 400 });
+    });
+
     it('rejects a non-string uid with 400', async () => {
         const { actor } = await makeUser();
         const { res } = makeRes();
@@ -176,45 +183,3 @@ describe('NotificationController.markAck', () => {
         expect(after?.acknowledged).toBeFalsy();
     });
 });
-
-// ── /notif/mark-read ────────────────────────────────────────────────
-
-describe('NotificationController.markRead', () => {
-    it('sets `shown` on the underlying notification row', async () => {
-        const { actor, userId } = await makeUser();
-        const created = await server.stores.notification.create({
-            userId,
-            value: {},
-        });
-
-        const { res, captured } = makeRes();
-        await controller.markRead(
-            makeReq({ body: { uid: created.uid }, actor }),
-            res,
-        );
-
-        expect(captured.body).toEqual({});
-        const after = await server.stores.notification.getByUid(
-            created.uid as string,
-            { userId },
-        );
-        expect(after?.shown).not.toBeNull();
-        // Marking read should NOT also set acknowledged.
-        expect(after?.acknowledged).toBeFalsy();
-    });
-
-    it('rejects an empty uid string with 400', async () => {
-        const { actor } = await makeUser();
-        const { res } = makeRes();
-        await expect(
-            controller.markRead(makeReq({ body: { uid: '' }, actor }), res),
-        ).rejects.toMatchObject({ statusCode: 400 });
-    });
-
-    it('throws 401 when there is no actor on the request', async () => {
-        const { res } = makeRes();
-        await expect(
-            controller.markRead(makeReq({ body: { uid: 'x' } }), res),
-        ).rejects.toMatchObject({ statusCode: 401 });
-    });
-});

src/backend/controllers/notification/NotificationController.ts

@@ -24,12 +24,12 @@ import type { NotificationService } from '../../services/notification/Notificati
 import { PuterController } from '../types.js';
 
 /**
- * GUI-facing notification endpoints. These supplement the `puter-notifications`
- * driver (which handles CRUD via `/drivers/call`) with two small mutation
- * routes that the puter desktop client calls directly.
+ * GUI-facing notification endpoint. Supplements the `puter-notifications`
+ * driver (which handles CRUD via `/drivers/call`) with the one mutation route
+ * the puter desktop client calls directly.
  *
- * Both routes emit `outer.gui.notif.ack` via the NotificationService so other
- * open tabs for the same user see the state change immediately.
+ * It emits `outer.gui.notif.ack` via the NotificationService so other open tabs
+ * for the same user see the state change immediately.
  */
 @Controller('/notif')
 export class NotificationController extends PuterController {
@@ -64,7 +64,8 @@ export class NotificationController extends PuterController {
             });
 
         const notifService = this.services.notification as unknown as
-            NotificationService | undefined;
+            | NotificationService
+            | undefined;
         if (notifService?.markAcknowledged) {
             await notifService.markAcknowledged(uid, userId);
         } else {
@@ -82,53 +83,4 @@ export class NotificationController extends PuterController {
         }
         res.json({});
     }
-
-    /**
-     * POST /notif/mark-read — user saw a notification. Sets `shown` timestamp;
-     * pushes ack event to sockets.
-     */
-    @Post('/mark-read', {
-        subdomain: 'api',
-        requireUserActor: true,
-        allowFullAccessToken: true,
-        // Fires per notification interaction, so the ceiling stays
-        // generous — it is here to catch a loop, not to pace a user.
-        rateLimit: {
-            scope: 'notification-mark',
-            limit: 300,
-            window: 60_000,
-            key: 'user',
-        },
-    })
-    async markRead(req: Request, res: Response): Promise<void> {
-        const uid = req.body?.uid;
-        if (typeof uid !== 'string' || uid.length === 0) {
-            throw new HttpError(400, '`uid` must be a non-empty string', {
-                legacyCode: 'bad_request',
-            });
-        }
-        const userId = req.actor?.user?.id;
-        if (!userId)
-            throw new HttpError(401, 'Unauthorized', {
-                legacyCode: 'unauthorized',
-            });
-
-        const notifService = this.services.notification as unknown as
-            NotificationService | undefined;
-        if (notifService?.markShown) {
-            await notifService.markShown(uid, userId);
-        } else {
-            await (
-                this.stores as Record<string, unknown> as {
-                    notification: {
-                        markShown: (
-                            uid: string,
-                            userId: number,
-                        ) => Promise<boolean>;
-                    };
-                }
-            ).notification.markShown(uid, userId);
-        }
-        res.json({});
-    }
 }

src/backend/drivers/notification/NotificationDriver.ts

@@ -173,7 +173,11 @@ export class NotificationDriver extends PuterDriver {
         return rows.map((r) => this.#toClient(r));
     }
 
-    /** Mark a notification as shown. Used by GUI when notification is displayed. */
+    /**
+     * Mark a notification as shown. Part of the driver's mailbox surface: the
+     * desktop marks over HTTP, but a client reading through `/drivers/call`
+     * needs a way to mark what it read.
+     */
     async mark_shown(args: Record<string, unknown>): Promise<unknown> {
         const actor = this.#requireUserActor();
         const uid = String(args.uid ?? '');
@@ -185,7 +189,7 @@ export class NotificationDriver extends PuterDriver {
         return { success: ok };
     }
 
-    /** Mark a notification as acknowledged (user dismissed it). */
+    /** Mark a notification as acknowledged (user dismissed it), same surface. */
     async mark_acknowledged(args: Record<string, unknown>): Promise<unknown> {
         const actor = this.#requireUserActor();
         const uid = String(args.uid ?? '');

src/backend/stores/notification/NotificationStore.js

@@ -20,11 +20,6 @@
 import { v4 as uuidv4 } from 'uuid';
 import { PuterStore } from '../types';
 
-// `markShown` intentionally doesn't invalidate unack count — it doesn't move it.
-
-const UNACK_CACHE_KEY_PREFIX = 'notifications:unack';
-const UNACK_CACHE_TTL_SECONDS = 5 * 60;
-
 export class NotificationStore extends PuterStore {
     // -- Reads --------------------------------------------------------
 
@@ -69,32 +64,6 @@ export class NotificationStore extends PuterStore {
         return rows.map((r) => this.#normalizeRow(r));
     }
 
-    async countUnacknowledged(userId) {
-        if (!userId) return 0;
-
-        const cacheKey = this.#unackCacheKey(userId);
-        try {
-            const raw = await this.clients.redis.get(cacheKey);
-            if (raw !== null && raw !== undefined) {
-                const parsed = Number(raw);
-                if (Number.isFinite(parsed)) return parsed;
-            }
-        } catch {
-            // Fall through to DB.
-        }
-
-        const rows = await this.clients.db.read(
-            'SELECT COUNT(*) AS n FROM `notification` WHERE `user_id` = ? AND `acknowledged` IS NULL',
-            [userId],
-        );
-        const count = Number(rows[0]?.n ?? 0);
-
-        this.clients.redis
-            .set(cacheKey, String(count), 'EX', UNACK_CACHE_TTL_SECONDS)
-            .catch(() => {});
-        return count;
-    }
-
     // -- Writes -------------------------------------------------------
 
     /**
@@ -130,15 +99,13 @@ export class NotificationStore extends PuterStore {
             'INSERT INTO `notification` (`uid`, `user_id`, `value`, `type`, `audience`, `app_uid`) VALUES (?, ?, ?, ?, ?, ?)',
             [uid, userId, serialized, type, audience, appUid],
         );
-        await this.#invalidateUnack(userId);
         return this.getByUid(uid, { userId });
     }
 
     /**
      * Rewrite a notification the recipient hasn't dismissed. `shown` is cleared
      * with it, so changed wording goes out again on their next connect —
-     * `#sendUnreads` only carries what was never shown. The unacknowledged
-     * count doesn't move, so there is nothing to invalidate.
+     * `#sendUnreads` only carries what was never shown.
      *
      * False when no such row exists, so callers can fall back to a fresh
      * notification instead of dropping what they were reporting.
@@ -161,9 +128,7 @@ export class NotificationStore extends PuterStore {
             'UPDATE `notification` SET `acknowledged` = ? WHERE `uid` = ? AND `user_id` = ? AND `acknowledged` IS NULL',
             [now, uid, userId],
         );
-        const changed = (result?.affectedRows ?? result?.changes ?? 0) > 0;
-        if (changed) await this.#invalidateUnack(userId);
-        return changed;
+        return (result?.affectedRows ?? result?.changes ?? 0) > 0;
     }
 
     async markShown(uid, userId) {
@@ -180,9 +145,7 @@ export class NotificationStore extends PuterStore {
             'DELETE FROM `notification` WHERE `uid` = ? AND `user_id` = ?',
             [uid, userId],
         );
-        const changed = (result?.affectedRows ?? result?.changes ?? 0) > 0;
-        if (changed) await this.#invalidateUnack(userId);
-        return changed;
+        return (result?.affectedRows ?? result?.changes ?? 0) > 0;
     }
 
     /**
@@ -222,14 +185,6 @@ export class NotificationStore extends PuterStore {
 
     // -- Internals ----------------------------------------------------
 
-    #unackCacheKey(userId) {
-        return `${UNACK_CACHE_KEY_PREFIX}:${userId}`;
-    }
-
-    async #invalidateUnack(userId) {
-        await this.publishCacheKeys({ keys: [this.#unackCacheKey(userId)] });
-    }
-
     #normalizeRow(row) {
         if (!row) return null;
         if (typeof row.value === 'string') {

src/backend/stores/notification/NotificationStore.test.js

@@ -21,12 +21,9 @@ import { v4 as uuidv4 } from 'uuid';
 import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 import { setupTestServer } from '../../testUtil.ts';
 
-const unackKey = (userId) => `notifications:unack:${userId}`;
-
 describe('NotificationStore', () => {
     let server;
     let store;
-    let redis;
     let user;
     let other;
 
@@ -43,7 +40,6 @@ describe('NotificationStore', () => {
     beforeAll(async () => {
         server = await setupTestServer();
         store = server.stores.notification;
-        redis = server.clients.redis;
         user = await makeUser();
         other = await makeUser();
     });
@@ -235,65 +231,6 @@ describe('NotificationStore', () => {
         );
     });
 
-    // -- unacknowledged count + cache ----------------------------------
-
-    it('returns zero for a falsy user without touching the database', async () => {
-        expect(await store.countUnacknowledged(undefined)).toBe(0);
-        expect(await store.countUnacknowledged(0)).toBe(0);
-    });
-
-    it('counts unacknowledged notifications and caches the result', async () => {
-        const u = await makeUser();
-        await store.create({ userId: u.id, value: {} });
-        await store.create({ userId: u.id, value: {} });
-
-        expect(await store.countUnacknowledged(u.id)).toBe(2);
-        expect(await redis.get(unackKey(u.id))).toBe('2');
-    });
-
-    it('serves a cached count without re-querying', async () => {
-        const u = await makeUser();
-        await store.create({ userId: u.id, value: {} });
-        await store.countUnacknowledged(u.id);
-
-        await redis.set(unackKey(u.id), '99');
-        expect(await store.countUnacknowledged(u.id)).toBe(99);
-    });
-
-    it('falls back to the database when the cached value is not a number', async () => {
-        const u = await makeUser();
-        await store.create({ userId: u.id, value: {} });
-        await redis.set(unackKey(u.id), 'garbage');
-
-        expect(await store.countUnacknowledged(u.id)).toBe(1);
-    });
-
-    it('invalidates the cached count on create', async () => {
-        const u = await makeUser();
-        await store.create({ userId: u.id, value: {} });
-        expect(await store.countUnacknowledged(u.id)).toBe(1);
-        expect(await redis.get(unackKey(u.id))).toBe('1');
-
-        await store.create({ userId: u.id, value: {} });
-        expect(await redis.get(unackKey(u.id))).toBeNull();
-        expect(await store.countUnacknowledged(u.id)).toBe(2);
-    });
-
-    it('invalidates the cached count on acknowledge and on delete', async () => {
-        const u = await makeUser();
-        const a = await store.create({ userId: u.id, value: {} });
-        const b = await store.create({ userId: u.id, value: {} });
-        await store.countUnacknowledged(u.id);
-
-        expect(await store.markAcknowledged(a.uid, u.id)).toBe(true);
-        expect(await redis.get(unackKey(u.id))).toBeNull();
-        expect(await store.countUnacknowledged(u.id)).toBe(1);
-
-        expect(await store.deleteByUid(b.uid, u.id)).toBe(true);
-        expect(await redis.get(unackKey(u.id))).toBeNull();
-        expect(await store.countUnacknowledged(u.id)).toBe(0);
-    });
-
     // -- mutations -----------------------------------------------------
 
     it('acknowledges only once and only for the owning user', async () => {
@@ -309,17 +246,17 @@ describe('NotificationStore', () => {
         expect(typeof row.acknowledged).toBe('number');
     });
 
-    it('marks shown only once and leaves the unacknowledged count alone', async () => {
+    it('marks shown only once and only for the owning user', async () => {
         const u = await makeUser();
         const n = await store.create({ userId: u.id, value: {} });
-        expect(await store.countUnacknowledged(u.id)).toBe(1);
 
         expect(await store.markShown(n.uid, other.id)).toBe(false);
         expect(await store.markShown(n.uid, u.id)).toBe(true);
         expect(await store.markShown(n.uid, u.id)).toBe(false);
 
-        // Cached count is deliberately untouched by markShown.
-        expect(await redis.get(unackKey(u.id))).toBe('1');
+        // Shown is not dismissed — the row stays unacknowledged.
+        const row = await store.getByUid(n.uid, { userId: u.id });
+        expect(row.acknowledged).toBeNull();
     });
 
     // -- retention -----------------------------------------------------