diff --git a/src/backend/controllers/notification/NotificationController.http.test.ts b/src/backend/controllers/notification/NotificationController.http.test.ts new file mode 100644 index 000000000..0a9e07fdd --- /dev/null +++ b/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 . + */ + +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(); + }); +}); diff --git a/src/backend/controllers/notification/NotificationController.test.ts b/src/backend/controllers/notification/NotificationController.test.ts index bbb846f28..6f49e6f60 100644 --- a/src/backend/controllers/notification/NotificationController.test.ts +++ b/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 }); - }); -}); diff --git a/src/backend/controllers/notification/NotificationController.ts b/src/backend/controllers/notification/NotificationController.ts index 5bb96d853..1a145826f 100644 --- a/src/backend/controllers/notification/NotificationController.ts +++ b/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 { - 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 as { - notification: { - markShown: ( - uid: string, - userId: number, - ) => Promise; - }; - } - ).notification.markShown(uid, userId); - } - res.json({}); - } } diff --git a/src/backend/drivers/notification/NotificationDriver.ts b/src/backend/drivers/notification/NotificationDriver.ts index 80d6ae9f0..8187d4f4a 100644 --- a/src/backend/drivers/notification/NotificationDriver.ts +++ b/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): Promise { 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): Promise { const actor = this.#requireUserActor(); const uid = String(args.uid ?? ''); diff --git a/src/backend/stores/notification/NotificationStore.js b/src/backend/stores/notification/NotificationStore.js index 92e3f9e1d..f5903d89d 100644 --- a/src/backend/stores/notification/NotificationStore.js +++ b/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') { diff --git a/src/backend/stores/notification/NotificationStore.test.js b/src/backend/stores/notification/NotificationStore.test.js index b2fc8bb6c..1b16cb0d2 100644 --- a/src/backend/stores/notification/NotificationStore.test.js +++ b/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 -----------------------------------------------------