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 7715176af..a45ccaa57 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' && diff --git a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js index 1d5d6b6a1..36ff9a775 100644 --- a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js +++ b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js @@ -26,7 +26,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, tapRoute, } from '../../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../../helpers/pushWorker.js'; @@ -498,6 +498,15 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { void handleNotificationTap({ uid: entry.uid, notification: entry.notification, appUid: entry.appUid }, tapSurface); }; + const actOnStored = storedTapHandler({ + fetchRow: getNotification, + surface: tapSurface, + found: (uid) => void acknowledge(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }), + missing: () => open(), + }); + const actOnPush = async (push) => { await reveal_dashboard(); if ( ! loaded ) await refresh(); @@ -506,15 +515,7 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { actOn(entry); return; } - const tap = push.stored ? await storedTapItem(push.uid, getNotification) : { uid: push.uid, push }; - if ( ! tap ) { - open(); - return; - } - void acknowledge(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - await handleNotificationTap(tap, tapSurface); + await actOnStored(push); }; // -- Toasts ------------------------------------------------------------- diff --git a/src/gui/src/UI/UIDesktop.js b/src/gui/src/UI/UIDesktop.js index cd6a0da91..7d8552ef2 100644 --- a/src/gui/src/UI/UIDesktop.js +++ b/src/gui/src/UI/UIDesktop.js @@ -51,7 +51,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, } from '../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../helpers/pushWorker.js'; import { get_app_by_uid } from '../helpers/getAppByUid.js'; @@ -251,26 +251,25 @@ async function UIDesktop (options) { }; // The desktop has no notification list, so other types only clear. - const tap_notification = (tap) => handleNotificationTap(tap, { + const tap_surface = { share: (notification) => share_notification_click(notification)?.(), openApp: (route) => openAppForTap(route, { getApp: get_app_by_uid, launch: launch_app }), - }); + }; + const tap_notification = (tap) => handleNotificationTap(tap, tap_surface); const toasts_for = (uid) => $(elementsWithData($('.notification').get(), 'uid', uid)); - setPushTapHandler(async (push) => { - let tap = { uid: push.uid, push }; - if ( push.stored ) { - tap = await storedTapItem(push.uid, getNotification); - if ( ! tap ) return; - } - toasts_for(push.uid).remove(); - update_tab_notif_count_badge(); - void markNotificationAcknowledged(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - return tap_notification(tap); - }); + setPushTapHandler(storedTapHandler({ + fetchRow: getNotification, + surface: tap_surface, + found: (uid) => { + toasts_for(uid).remove(); + update_tab_notif_count_badge(); + void markNotificationAcknowledged(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }); + }, + })); startPushWorker(); /** diff --git a/src/gui/src/helpers/notificationTap.js b/src/gui/src/helpers/notificationTap.js index 31b3056f0..3c9420f3c 100644 --- a/src/gui/src/helpers/notificationTap.js +++ b/src/gui/src/helpers/notificationTap.js @@ -27,6 +27,8 @@ export const PUSH_TAP_PARAM = 'puter.notification'; const UID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; const UID_MAX = 64; +// A tap for a uid acted on this recently is the same tap again. +const TAP_REPEAT_MS = 10_000; const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : undefined); @@ -38,11 +40,10 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** + * A tap from the worker or the URL names only a notification; what it leads + * to is read off the signed-in user's own row. * @typedef {Object} PushTap * @property {string} uid - * @property {{ app?: string, path?: string }} [target] - * @property {unknown} [data] - * @property {true} [stored] - Only the uid is known; act on the stored row */ /** @@ -52,60 +53,37 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** - * @param {{ uid?: string, notification?: Object|null, appUid?: string|null, push?: PushTap }} tap + * @param {{ uid?: string, notification?: Object|null, appUid?: string|null }} tap * @returns {TapRoute} */ -export const tapRoute = ({ uid, notification, appUid, push }) => { - if ( notification && typeof notification === 'object' ) { - if ( notification.type === APP_MESSAGE_TYPE ) { - const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); - if ( ! app ) return { kind: 'panel' }; - const target = typeof notification.target === 'string' ? notification.target : undefined; - return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; - } - if ( isShareNotification(notification) ) return { kind: 'share', notification }; - return { kind: 'panel' }; - } - const app = nonEmpty(push?.target?.app); - if ( app ) { - return { kind: 'app', appUid: app, click: { uid, target: push.target.path, data: push.data } }; - } - const path = push?.target?.path; - if ( typeof path === 'string' && path.startsWith('/') ) { - return { - kind: 'share', - notification: { type: 'share.received', fields: { target: { path } } }, - }; +export const tapRoute = ({ uid, notification, appUid }) => { + if ( ! notification || typeof notification !== 'object' ) return { kind: 'panel' }; + if ( notification.type === APP_MESSAGE_TYPE ) { + const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); + if ( ! app ) return { kind: 'panel' }; + const target = typeof notification.target === 'string' ? notification.target : undefined; + return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; } + if ( isShareNotification(notification) ) return { kind: 'share', notification }; return { kind: 'panel' }; }; +/** @returns {PushTap|null} */ +const tapFor = (uid) => ( + typeof uid === 'string' && uid.length <= UID_MAX && UID_PATTERN.test(uid) ? { uid } : null +); + /** * @param {unknown} raw - The worker's `notification` object - * @returns {PushTap|null} + * @returns {PushTap|null} The notification it names; nothing else it carries */ -export const normalizePushTap = (raw) => { - if ( ! raw || typeof raw !== 'object' ) return null; - if ( ! nonEmpty(raw.uid) || raw.uid.length > UID_MAX ) return null; - const target = {}; - if ( nonEmpty(raw.target?.app) ) target.app = raw.target.app; - if ( nonEmpty(raw.target?.path) ) target.path = raw.target.path; - return { - uid: raw.uid, - ...(Object.keys(target).length ? { target } : {}), - ...(raw.data !== undefined ? { data: raw.data } : {}), - }; -}; +export const normalizePushTap = (raw) => (raw && typeof raw === 'object' ? tapFor(raw.uid) : null); /** * @param {URLSearchParams} [params] - * @returns {PushTap|null} A tap naming only a notification uid + * @returns {PushTap|null} */ -export const pushTapFromParams = (params) => { - const uid = params?.get?.(PUSH_TAP_PARAM); - if ( typeof uid !== 'string' || uid.length > UID_MAX || ! UID_PATTERN.test(uid) ) return null; - return { uid, stored: true }; -}; +export const pushTapFromParams = (params) => tapFor(params?.get?.(PUSH_TAP_PARAM)); /** * @param {string} uid @@ -205,10 +183,27 @@ export const handleNotificationTap = async (tap, { share, panel, openApp }) => { return panel?.(); }; +/** + * @param {{ fetchRow: (uid: string) => Promise, + * surface: { share: Function, panel?: Function, openApp: Function }, + * found?: (uid: string) => unknown, missing?: () => unknown }} deps + * `found` runs before routing a row that was read; `missing` instead of + * routing when there is none + * @returns {(tap: PushTap) => Promise} + */ +export const storedTapHandler = ({ fetchRow, surface, found, missing }) => async ({ uid }) => { + const tap = await storedTapItem(uid, fetchRow); + if ( ! tap ) return missing?.(); + found?.(uid); + return handleNotificationTap(tap, surface); +}; + /** @type {((tap: PushTap) => unknown) | null} */ let pushTapHandler = null; /** @type {PushTap[]} */ const pendingPushTaps = []; +/** @type {Map} uid → when it was last handed over */ +const recentTaps = new Map(); /** @param {(tap: PushTap) => unknown} handler */ export const setPushTapHandler = (handler) => { @@ -216,15 +211,21 @@ export const setPushTapHandler = (handler) => { for ( const tap of pendingPushTaps.splice(0) ) void handler(tap); }; +const seenLately = (uid, now) => { + for ( const [seen, at] of recentTaps ) { + if ( now - at >= TAP_REPEAT_MS ) recentTaps.delete(seen); + } + if ( recentTaps.has(uid) ) return true; + recentTaps.set(uid, now); + return false; +}; + /** @param {PushTap|null} tap */ const deliverPushTap = (tap) => { - if ( ! tap ) return; + if ( ! tap || seenLately(tap.uid, Date.now()) ) return; if ( pushTapHandler ) void pushTapHandler(tap); else pendingPushTaps.push(tap); }; -/** @param {unknown} raw - The worker's `notification` object */ +/** @param {unknown} raw - The worker's `notification` object, or a tap from the URL */ export const dispatchPushTap = (raw) => deliverPushTap(normalizePushTap(raw)); - -/** @param {PushTap} tap - From {@link pushTapFromParams} */ -export const dispatchStoredTap = (tap) => deliverPushTap(tap?.stored ? tap : null); diff --git a/src/gui/src/helpers/notificationTap.test.js b/src/gui/src/helpers/notificationTap.test.js index e8d9bffd2..74a9488d6 100644 --- a/src/gui/src/helpers/notificationTap.test.js +++ b/src/gui/src/helpers/notificationTap.test.js @@ -24,12 +24,15 @@ import { normalizePushTap, openAppForTap, pushTapFromParams, + storedTapHandler, storedTapItem, tapRoute, withoutNotificationClick, } from './notificationTap.js'; const UID = '0b6a2c1e-6f1d-4a57-9c1e-3d2f8a9b7c10'; +const OTHER_UID = '7d3e9f10-2b4c-4d5e-8f6a-1b2c3d4e5f60'; +const THIRD_UID = 'c1d2e3f4-5a6b-4c7d-8e9f-0a1b2c3d4e5f'; const appMessage = (over = {}) => ({ type: 'app.message', @@ -75,24 +78,22 @@ describe('tapRoute', () => { .toEqual({ kind: 'panel' }); }); - it('reads a push tap: an app target is the app, a path is a shared item', () => { + it('is the list without a notification, whatever else the tap carries', () => { expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { app: 'app-1', path: '/rooms/7' }, data: 5 } })) - .toEqual({ kind: 'app', appUid: 'app-1', click: { uid: 'n1', target: '/rooms/7', data: 5 } }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { path: '/alice/Shared/a.txt' } } })).toEqual({ - kind: 'share', - notification: { type: 'share.received', fields: { target: { path: '/alice/Shared/a.txt' } } }, - }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1' } })).toEqual({ kind: 'panel' }); + .toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1', target: { app: 'app-1' }, notification: null })).toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1' })).toEqual({ kind: 'panel' }); }); }); describe('normalizePushTap', () => { - it('keeps the expected fields and drops the rest', () => { - expect(normalizePushTap({ uid: 'n1', target: { app: 'a', path: 7, x: 1 }, data: null, extra: 1 })) - .toEqual({ uid: 'n1', target: { app: 'a' }, data: null }); + it('keeps the notification uid and nothing else', () => { + expect(normalizePushTap({ uid: UID, target: { app: 'a', path: '/x' }, data: null, extra: 1 })) + .toEqual({ uid: UID }); expect(normalizePushTap({ uid: '' })).toBeNull(); - expect(normalizePushTap({ uid: 'x'.repeat(65) })).toBeNull(); - expect(normalizePushTap('n1')).toBeNull(); + expect(normalizePushTap({ uid: 'n1' })).toBeNull(); + expect(normalizePushTap({ uid: `${UID}${'0'.repeat(40)}` })).toBeNull(); + expect(normalizePushTap(UID)).toBeNull(); }); }); @@ -100,7 +101,7 @@ describe('pushTapFromParams', () => { const params = (value) => new URLSearchParams({ 'puter.notification': value }); it('reads a notification uid, and nothing else, from the URL', () => { - expect(pushTapFromParams(params(UID))).toEqual({ uid: UID, stored: true }); + expect(pushTapFromParams(params(UID))).toEqual({ uid: UID }); }); it('takes nothing that is not a notification uid', () => { @@ -251,17 +252,78 @@ describe('handleNotificationTap', () => { }); }); +describe('storedTapHandler', () => { + const surface = () => ({ share: vi.fn(), panel: vi.fn(), openApp: vi.fn() }); + const row = { uid: UID, value: appMessage({ target: '/rooms/9', data: { room: 9 } }), app_uid: 'app-row' }; + const rows = async (uid) => (uid === UID ? row : null); + + it('routes from the row the tap names, after `found`', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: UID }); + expect(deps.fetchRow).toHaveBeenCalledWith(UID); + expect(deps.found).toHaveBeenCalledWith(UID); + expect(deps.surface.openApp).toHaveBeenCalledWith({ + kind: 'app', + appUid: 'app-row', + click: { uid: UID, target: '/rooms/9', data: { room: 9 } }, + }); + expect(deps.missing).not.toHaveBeenCalled(); + }); + + it('opens nothing for a row the user cannot read, running `missing` instead', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: OTHER_UID }); + expect(deps.missing).toHaveBeenCalledOnce(); + expect(deps.found).not.toHaveBeenCalled(); + expect(deps.surface.openApp).not.toHaveBeenCalled(); + expect(deps.surface.panel).not.toHaveBeenCalled(); + expect(deps.surface.share).not.toHaveBeenCalled(); + }); +}); + describe('push tap dispatch', () => { + afterEach(() => { + vi.useRealTimers(); + }); + it('holds taps until a handler is set, then hands them over in order', async () => { - const { dispatchPushTap, dispatchStoredTap, setPushTapHandler } = await import('./notificationTap.js'); - dispatchPushTap({ uid: 'n1' }); + const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); + dispatchPushTap({ uid: UID }); dispatchPushTap({ uid: '' }); - dispatchStoredTap({ uid: UID, stored: true }); - dispatchStoredTap({ uid: UID }); + dispatchPushTap({ uid: 'n2' }); + dispatchPushTap({ uid: OTHER_UID, stored: true }); const handler = vi.fn(); setPushTapHandler(handler); - expect(handler.mock.calls).toEqual([[{ uid: 'n1' }], [{ uid: UID, stored: true }]]); - dispatchPushTap({ uid: 'n3' }); - expect(handler).toHaveBeenLastCalledWith({ uid: 'n3' }); + expect(handler.mock.calls).toEqual([[{ uid: UID }], [{ uid: OTHER_UID }]]); + dispatchPushTap({ uid: THIRD_UID }); + expect(handler).toHaveBeenLastCalledWith({ uid: THIRD_UID }); + }); + + it('hands a worker tap over as the notification it names, nothing more', async () => { + const { dispatchPushTap, setPushTapHandler, storedTapHandler: make } = await import('./notificationTap.js'); + const openApp = vi.fn(); + const fetchRow = vi.fn(async () => null); + setPushTapHandler(make({ fetchRow, surface: { share: vi.fn(), openApp } })); + dispatchPushTap({ uid: OTHER_UID, target: { app: 'app-1', path: '/rooms/7' }, data: { room: 7 } }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(fetchRow).toHaveBeenCalledWith(OTHER_UID); + expect(openApp).not.toHaveBeenCalled(); + }); + + it('hands the same notification over once within ten seconds', async () => { + vi.useFakeTimers(); + const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); + const handler = vi.fn(); + setPushTapHandler(handler); + dispatchPushTap({ uid: UID }); + dispatchPushTap({ uid: UID }); + dispatchPushTap({ uid: OTHER_UID }); + expect(handler.mock.calls).toEqual([[{ uid: UID }], [{ uid: OTHER_UID }]]); + vi.advanceTimersByTime(9_999); + dispatchPushTap({ uid: UID }); + expect(handler).toHaveBeenCalledTimes(2); + vi.advanceTimersByTime(1); + dispatchPushTap({ uid: UID }); + expect(handler).toHaveBeenCalledTimes(3); }); }); diff --git a/src/gui/src/helpers/pushWorker.js b/src/gui/src/helpers/pushWorker.js index e0783189c..73cb78d62 100644 --- a/src/gui/src/helpers/pushWorker.js +++ b/src/gui/src/helpers/pushWorker.js @@ -23,7 +23,6 @@ import { httpsIconFor, isShareNotification } from '../UI/Dashboard/notificationC import { APP_MESSAGE_TYPE, dispatchPushTap, - dispatchStoredTap, normalizePushTap, pushTapFromParams, PUSH_TAP_PARAM, @@ -69,9 +68,12 @@ const nonEmpty = (value) => (typeof value === 'string' && value.trim() !== '' ? * @returns {Object} The push payload shape `sw.js` shows */ export const pushMessageFor = (uid, notification, appUid = null) => { - const message = { uid, group: nonEmpty(notification.group) ?? uid }; + const group = nonEmpty(notification.group); + const message = { uid, group: group ?? uid }; if ( notification.type === APP_MESSAGE_TYPE ) { const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); + // An app's group is its own, as the server names it on a push. + if ( app && group ) message.group = `${app}:${group}`; if ( notification.sensitive === true ) { return { ...message, title: nonEmpty(notification.app?.name), ...(app ? { target: { app } } : {}) }; } @@ -144,7 +146,7 @@ export const startPushWorker = async ({ if ( query?.has?.(PUSH_TAP_PARAM) ) { const opened = pushTapFromParams(query); clearParam(PUSH_TAP_PARAM); - if ( opened ) dispatchStoredTap(opened); + if ( opened ) dispatchPushTap(opened); } // Leave an existing worker: removing it ends this browser's subscription. @@ -155,7 +157,8 @@ export const startPushWorker = async ({ dispatchPushTap(event.data.notification); }); try { - await nav.serviceWorker.register(WORKER_URL, { scope: '/' }); + // Scripts the worker imports are fetched fresh on each update check. + await nav.serviceWorker.register(WORKER_URL, { scope: '/', updateViaCache: 'none' }); } catch ( err ) { console.warn('Could not register the service worker:', err); } diff --git a/src/gui/src/helpers/pushWorker.test.js b/src/gui/src/helpers/pushWorker.test.js index 7e2359f27..d233a46f6 100644 --- a/src/gui/src/helpers/pushWorker.test.js +++ b/src/gui/src/helpers/pushWorker.test.js @@ -74,7 +74,7 @@ describe('pushMessageFor', () => { app: { uid: 'app-1', name: 'Chat' }, })).toEqual({ uid: 'n1', - group: 'room-7', + group: 'app-1:room-7', title: 'New reply', body: 'see you', icon: 'https://example.com/i.png', @@ -102,6 +102,16 @@ describe('pushMessageFor', () => { }); }); + it("tags an app message's group with its app, so two apps never share one", () => { + const message = (app, over = {}) => ({ type: 'app.message', title: 't', group: 'room-7', app: { uid: app }, ...over }); + expect(pushMessageFor('n1', message('app-1')).group).toBe('app-1:room-7'); + expect(pushMessageFor('n1', message('app-2')).group).toBe('app-2:room-7'); + expect(pushMessageFor('n1', message('app-1', { sensitive: true })).group).toBe('app-1:room-7'); + expect(pushMessageFor('n1', message('app-b'), 'app-a').group).toBe('app-a:room-7'); + expect(pushMessageFor('n1', message('app-1', { group: undefined })).group).toBe('n1'); + expect(pushMessageFor('n2', { ...share, group: 'room-7' }).group).toBe('room-7'); + }); + it("uses the row's app uid over the one the message names", () => { expect(pushMessageFor('n1', { type: 'app.message', title: 't' }, 'app-9').target).toEqual({ app: 'app-9' }); expect(pushMessageFor('n1', { type: 'app.message', title: 't', app: { uid: 'app-b' } }, 'app-a').target) @@ -207,17 +217,17 @@ describe('startPushWorker', () => { mod.setPushTapHandler(handler); const nav = makeNav(); await start(mod, { nav }); - expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/' }); + expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/', updateViaCache: 'none' }); const source = { postMessage: vi.fn() }; - nav.listeners.message({ data: { type: 'other', notification: { uid: 'x' } }, source }); - nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: '' } }, source }); + nav.listeners.message({ data: { type: 'other', notification: { uid: UID } }, source }); + nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: 'n1' } }, source }); nav.listeners.message({ - data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: 'n1', target: { app: 'a' } } }, + data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: UID, target: { app: 'a' } } }, source, }); expect(source.postMessage.mock.calls).toEqual([[{ type: 'puter-notification-click-ack', id: 'click-1' }]]); - expect(handler.mock.calls).toEqual([[{ uid: 'n1', target: { app: 'a' } }]]); + expect(handler.mock.calls).toEqual([[{ uid: UID }]]); }); it('registers nothing inside a frame', async () => { @@ -245,7 +255,7 @@ describe('startPushWorker', () => { clearParam, }); expect(clearParam).toHaveBeenCalledWith('puter.notification'); - expect(handler).toHaveBeenCalledWith({ uid: UID, stored: true }); + expect(handler).toHaveBeenCalledWith({ uid: UID }); }); it('clears a malformed parameter and hands nothing over', async () => {