diff --git a/src/backend/clients/event/types.ts b/src/backend/clients/event/types.ts index d54e4abce..99036d2ac 100644 --- a/src/backend/clients/event/types.ts +++ b/src/backend/clients/event/types.ts @@ -552,13 +552,17 @@ export type EventMap = { /** Events handler runs behind whatever created it. */ handlerDepth?: number; }; - /** Every non-silent write or update, only on the node that wrote it. */ + /** + * Every non-silent write or update, only on the node that wrote it. The + * value is the listener's own copy of what the socket carries. + */ 'notif.written': { userId: number; uid: string; type: string; - /** The row's app. Absent on an update, which keeps the row's own. */ - appUid?: string | null; + audience: string; + /** The row's app; null when it names none or an update could not tell. */ + appUid: string | null; value: Record; /** An earlier notification rewritten in place. */ updated?: true; diff --git a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts index 712900197..52dcea270 100644 --- a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts +++ b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts @@ -17,7 +17,7 @@ * along with this program. If not, see . */ -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; @@ -28,6 +28,42 @@ import type { IConfig } from '../../types.js'; import { peekRateLimit } from '../../core/http/middleware/rateLimit.js'; import type { NotificationDriver } from './NotificationDriver.js'; +/** + * Charges to a held key wait here until the test lets them go, so two requests + * can both be read before either is spent. Every charge is recorded by key. + */ +const charges = vi.hoisted(() => ({ + held: new Set(), + waiting: [] as Array<() => void>, + made: [] as string[], +})); + +vi.mock('../../core/http/middleware/rateLimit.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../../core/http/middleware/rateLimit.js') + >(); + return { + ...actual, + checkRateLimit: async (key: string, ...rest: unknown[]) => { + if (charges.held.has(key)) { + await new Promise((go) => charges.waiting.push(go)); + } + charges.made.push(key); + return ( + actual.checkRateLimit as (...a: unknown[]) => Promise + )(key, ...rest); + }, + }; +}); + +const until = async (ready: () => boolean): Promise => { + for (let i = 0; i < 400 && !ready(); i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + if (!ready()) throw new Error('timed out'); +}; + const makeUser = async (server: PuterServer): Promise => { const username = `am-${Math.random().toString(36).slice(2, 10)}`; const created = await server.stores.user.create({ @@ -347,6 +383,50 @@ describe('app.message with live delivery over the socket wire', () => { ).resolves.toMatchObject({ uid: expect.any(String) }); }); + it('spends the day window only on a request the minute window admits', async () => { + const holder = await makeUser(state.server); + const app = await makeApp(state.server, holder.user.id!); + const minuteKey = `notif:app-message:minute:${holder.user.id}:${app.uid}`; + const dayKey = `notif:app-message:day:${holder.user.id}:${app.uid}`; + for (let i = 0; i < 2; i++) { + await withActor(asApp(holder, app), () => + state.driver.create(message()), + ); + } + + charges.held.add(minuteKey).add(dayKey); + charges.made.length = 0; + let settled = false; + const outcome = Promise.allSettled([ + withActor(asApp(holder, app), () => state.driver.create(message())), + withActor(asApp(holder, app), () => state.driver.create(message())), + ]).finally(() => { + settled = true; + }); + try { + // Both requests read the windows as open before either spends. + await until(() => charges.waiting.length === 2); + while (!settled) { + await until(() => settled || charges.waiting.length > 0); + charges.waiting.shift()?.(); + } + } finally { + charges.held.clear(); + } + + const results = await outcome; + expect(results.map((r) => r.status).sort()).toEqual([ + 'fulfilled', + 'rejected', + ]); + const refused = results.find((r) => r.status === 'rejected') as + PromiseRejectedResult | undefined; + expect(refused?.reason).toMatchObject({ statusCode: 429 }); + // The refused request left the day window alone. + expect(charges.made.filter((k) => k === dayKey)).toEqual([dayKey]); + expect(await peekRateLimit(dayKey, 4, 24 * 60 * 60_000)).toBe(true); + }); + it("reads back the app's own messages and no other app's", async () => { const holder = await makeUser(state.server); const app = await makeApp(state.server, holder.user.id!); diff --git a/src/backend/drivers/notification/NotificationDriver.ts b/src/backend/drivers/notification/NotificationDriver.ts index 887a31e6d..7e58f3a80 100644 --- a/src/backend/drivers/notification/NotificationDriver.ts +++ b/src/backend/drivers/notification/NotificationDriver.ts @@ -340,23 +340,28 @@ export class NotificationDriver extends PuterDriver { }); } - /** Both windows are read before either is spent, so a refusal costs none. */ + /** + * Both windows are read before either is spent, so a refusal costs none. + * The minute window is spent first: it is the one most likely to run out + * between the read and the spend, and a refusal there leaves the day + * alone. + */ async #withinAppMessageLimits( userId: number, appUid: string, ): Promise { const configured = this.config.notificationAppMessageLimits ?? {}; const all: Array<[string, number, number]> = [ - [ - `notif:app-message:day:${userId}:${appUid}`, - configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, - DAY_MS, - ], [ `notif:app-message:minute:${userId}:${appUid}`, configured.perMinute ?? APP_MESSAGE_RATE_LIMITS.perMinute, MINUTE_MS, ], + [ + `notif:app-message:day:${userId}:${appUid}`, + configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, + DAY_MS, + ], ]; const windows = all.filter(([, limit]) => limit > 0); for (const [key, limit, windowMs] of windows) { diff --git a/src/backend/services/notification/NotificationService.test.ts b/src/backend/services/notification/NotificationService.test.ts index 68f8d14af..ab7065f2a 100644 --- a/src/backend/services/notification/NotificationService.test.ts +++ b/src/backend/services/notification/NotificationService.test.ts @@ -3,18 +3,19 @@ * * 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. + * 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. + * 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 . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import { v4 as uuidv4 } from 'uuid'; @@ -361,6 +362,7 @@ describe("NotificationService — 'notif.written'", () => { userId: u.id, uid: rows[i][0].uid, type: 'app.events.ended', + audience: 'app-user', appUid, value: { title: 'ended', type: 'app.events.ended' }, })); @@ -404,11 +406,82 @@ describe("NotificationService — 'notif.written'", () => { userId: user.id, uid, type: 'share.received', + audience: 'account', + appUid: null, value: { title: 'two shares', type: 'share.received' }, updated: true, }); written.stop(); }); + + it("names the row's app on a rewrite", async () => { + const user = await makeUser(); + const uid = uuidv4(); + const appUid = `app-${uuidv4()}`; + await server.stores.notification.insert({ + userId: user.id, + value: { title: 'one', type: 'share.received' }, + uid, + type: 'share.received', + audience: 'account', + appUid, + }); + const written = collect('notif.written'); + + await notifications.notifyUpdate( + uid, + user.id, + { title: 'two' }, + { type: 'share.received' }, + ); + await waitFor(() => written.seen.length === 1); + + expect(written.seen[0]).toMatchObject({ uid, appUid, updated: true }); + written.stop(); + }); + + it('hands each listener its own copy of the value', async () => { + const user = await makeUser(); + const handler = (_k: string, data: unknown) => { + const value = (data as { value: Record }).value; + value.title = 'changed by a listener'; + (value.fields as Record).path = '/elsewhere'; + }; + server.clients.event.on('notif.written', handler); + const pushed = collect('outer.gui.notif.message'); + try { + const uid = await notifications.notify( + [user.id], + { title: 'one share', fields: { path: '/a' } }, + { type: 'share.received' }, + ); + await notifications.notifyUpdate( + uid!, + user.id, + { title: 'two shares', fields: { path: '/b' } }, + { type: 'share.received' }, + ); + await waitFor(() => pushed.seen.length === 2); + } finally { + server.clients.event.off?.('notif.written', handler); + pushed.stop(); + } + + expect( + pushed.seen.map((p) => (p as NotifPush).response.notification), + ).toEqual([ + { + title: 'one share', + fields: { path: '/a' }, + type: 'share.received', + }, + { + title: 'two shares', + fields: { path: '/b' }, + type: 'share.received', + }, + ]); + }); }); describe('NotificationService.markAcknowledged', () => { @@ -608,7 +681,10 @@ describe('NotificationService — shown on delivery', () => { { type: 'share.received' }, ); - const [row] = await server.stores.notification.listByUserId(user.id, {}); + const [row] = await server.stores.notification.listByUserId( + user.id, + {}, + ); expect(row.uid).toBe(uid); expect(row.shown).toBeTruthy(); diff --git a/src/backend/services/notification/NotificationService.ts b/src/backend/services/notification/NotificationService.ts index c12962a23..a9835c005 100644 --- a/src/backend/services/notification/NotificationService.ts +++ b/src/backend/services/notification/NotificationService.ts @@ -170,14 +170,16 @@ export class NotificationService extends PuterService { } if (!opts.silent) { + // Listeners get their own copy; the socket sends `payload`. this.clients.event.emit( 'notif.written', { userId, uid, type: registered.type, + audience: registered.audience, appUid, - value: payload, + value: structuredClone(payload), }, {}, ); @@ -229,13 +231,21 @@ export class NotificationService extends PuterService { // to anything deduplicating on `event.id`. this.#socket().message(userId, uid, payload); this.#markDelivered(uid, userId); + const row = await this.stores.notification.getByUid(uid, { + userId, + }); + const app = notification.app as { uid?: unknown } | undefined; this.clients.event.emit( 'notif.written', { userId, uid, type: registered.type, - value: payload, + audience: registered.audience, + appUid: + row?.app_uid ?? + (typeof app?.uid === 'string' ? app.uid : null), + value: structuredClone(payload), updated: true, }, {}, diff --git a/src/backend/services/notification/appMessage.test.ts b/src/backend/services/notification/appMessage.test.ts index 48d4df0f1..717362010 100644 --- a/src/backend/services/notification/appMessage.test.ts +++ b/src/backend/services/notification/appMessage.test.ts @@ -212,4 +212,14 @@ describe('appMessageSender', () => { ).toEqual({ uid: 'app-1', name: 'chat' }); expect(appMessageSender('app-1', null)).toEqual({ uid: 'app-1' }); }); + + it('caps the name at the title length, counting an emoji once', () => { + const long = '😀'.repeat(500); + const { name } = appMessageSender('app-1', { title: long }); + expect([...name!].length).toBe(120); + expect(name).toBe('😀'.repeat(120)); + expect(appMessageSender('app-1', { title: 'a'.repeat(300) }).name).toBe( + 'a'.repeat(120), + ); + }); }); diff --git a/src/backend/services/notification/appMessage.ts b/src/backend/services/notification/appMessage.ts index a79155b65..a13bb84a4 100644 --- a/src/backend/services/notification/appMessage.ts +++ b/src/backend/services/notification/appMessage.ts @@ -69,7 +69,12 @@ export const appMessageSender = ( const name = [row?.title, row?.name].find( (v): v is string => typeof v === 'string' && v.trim() !== '', ); - if (name !== undefined) sender.name = cleanText(name).trim(); + if (name !== undefined) { + sender.name = [...cleanText(name)] + .slice(0, APP_MESSAGE_TITLE_MAX) + .join('') + .trim(); + } const icon = row?.icon; if ( typeof icon === 'string' &&