From 5fc39b0371ce0837373f7004819a33ed1a96db1b Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 20 Apr 2026 16:21:47 +0200 Subject: [PATCH] refactor(notifications): convert per-component stream hook to module-level singleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the per-component createNotificationStream() factory with a shared $lib/stores/notifications.svelte.ts singleton. Ref-counted init()/destroy() ensures one EventSource per tab no matter how many consumers mount simultaneously. Motivation: the /chronik "Für dich" box (#285) needs the same live-arrival stream that NotificationBell already consumes. Two factories would open two SSE connections per tab — this refactor avoids the silent regression before it ships. - New: src/lib/stores/notifications.svelte.ts (module state, refcount) - New: src/lib/stores/notifications.svelte.spec.ts (proves single EventSource across multiple consumers + ref-counted teardown) - Deleted: src/lib/hooks/useNotificationStream.svelte.ts (factory) - Deleted: src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts - NotificationBell now imports the singleton Part of #285. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/components/NotificationBell.svelte | 4 +- .../useNotificationStream.svelte.test.ts | 142 ------------------ .../lib/hooks/useNotificationStream.svelte.ts | 95 ------------ .../lib/stores/notifications.svelte.spec.ts | 108 +++++++++++++ .../src/lib/stores/notifications.svelte.ts | 108 +++++++++++++ 5 files changed, 218 insertions(+), 239 deletions(-) delete mode 100644 frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts delete mode 100644 frontend/src/lib/hooks/useNotificationStream.svelte.ts create mode 100644 frontend/src/lib/stores/notifications.svelte.spec.ts create mode 100644 frontend/src/lib/stores/notifications.svelte.ts diff --git a/frontend/src/lib/components/NotificationBell.svelte b/frontend/src/lib/components/NotificationBell.svelte index 9781e366..03f5398a 100644 --- a/frontend/src/lib/components/NotificationBell.svelte +++ b/frontend/src/lib/components/NotificationBell.svelte @@ -3,13 +3,13 @@ import { onMount, onDestroy } from 'svelte'; import { goto } from '$app/navigation'; import { m } from '$lib/paraglide/messages.js'; import { clickOutside } from '$lib/actions/clickOutside'; -import { createNotificationStream } from '$lib/hooks/useNotificationStream.svelte'; +import { notificationStore } from '$lib/stores/notifications.svelte'; import NotificationDropdown from './NotificationDropdown.svelte'; let open = $state(false); let bellButtonEl: HTMLButtonElement | null = null; -const stream = createNotificationStream(); +const stream = notificationStore; async function toggleDropdown() { open = !open; diff --git a/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts deleted file mode 100644 index 143844c5..00000000 --- a/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts +++ /dev/null @@ -1,142 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import type { NotificationItem } from '../useNotificationStream.svelte'; - -// Track the last created EventSource instance -let lastEventSource: { - close: ReturnType; - onopen: (() => void) | null; - onerror: (() => void) | null; - simulate: (type: string, data: string) => void; -} | null = null; - -class MockEventSource { - onopen: (() => void) | null = null; - onerror: (() => void) | null = null; - close = vi.fn(); - private listeners: Record void)[]> = {}; - - constructor() { - // eslint-disable-next-line @typescript-eslint/no-this-alias - lastEventSource = this; - } - - addEventListener(type: string, fn: (e: MessageEvent) => void) { - if (!this.listeners[type]) this.listeners[type] = []; - this.listeners[type].push(fn); - } - - simulate(type: string, data: string) { - const event = new MessageEvent(type, { data }); - for (const fn of this.listeners[type] ?? []) { - fn(event); - } - } -} - -vi.stubGlobal('EventSource', MockEventSource); - -const mockFetch = vi.fn(); -vi.stubGlobal('fetch', mockFetch); - -// Import after stubs are set up -const { createNotificationStream } = await import('../useNotificationStream.svelte'); - -beforeEach(() => { - mockFetch.mockReset(); - lastEventSource = null; -}); - -function makeNotification(overrides: Partial = {}): NotificationItem { - return { - id: 'n1', - type: 'REPLY', - actorName: 'Hans', - documentId: 'doc-1', - referenceId: 'ref-1', - annotationId: null, - read: false, - createdAt: new Date().toISOString(), - ...overrides - }; -} - -describe('createNotificationStream', () => { - it('starts with empty notifications and zero unreadCount', () => { - const stream = createNotificationStream(); - expect(stream.notifications).toHaveLength(0); - expect(stream.unreadCount).toBe(0); - }); - - it('fetchUnreadCount updates unreadCount from API', async () => { - mockFetch.mockResolvedValueOnce(new Response(JSON.stringify({ count: 3 }), { status: 200 })); - const stream = createNotificationStream(); - await stream.fetchUnreadCount(); - expect(stream.unreadCount).toBe(3); - }); - - it('fetchNotifications populates notifications from API', async () => { - const items = [makeNotification()]; - mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({ content: items }), { status: 200 }) - ); - const stream = createNotificationStream(); - await stream.fetchNotifications(); - expect(stream.notifications).toHaveLength(1); - expect(stream.notifications[0].id).toBe('n1'); - }); - - it('markRead marks notification as read and decrements unreadCount', async () => { - mockFetch - .mockResolvedValueOnce(new Response(JSON.stringify({ count: 2 }), { status: 200 })) - .mockResolvedValueOnce(new Response(null, { status: 200 })); - const stream = createNotificationStream(); - await stream.fetchUnreadCount(); - - const notification = makeNotification({ read: false }); - await stream.markRead(notification); - expect(notification.read).toBe(true); - expect(stream.unreadCount).toBe(1); - }); - - it('markAllRead calls the API and resets unreadCount', async () => { - mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); - const stream = createNotificationStream(); - await stream.markAllRead(); - expect(mockFetch).toHaveBeenCalledWith('/api/notifications/read-all', { method: 'POST' }); - expect(stream.unreadCount).toBe(0); - }); - - it('destroy closes the EventSource', async () => { - mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); - const stream = createNotificationStream(); - stream.init(); - expect(lastEventSource).not.toBeNull(); - stream.destroy(); - expect(lastEventSource!.close).toHaveBeenCalled(); - }); - - it('SSE notification event prepends notification and increments unreadCount', async () => { - mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); - const stream = createNotificationStream(); - stream.init(); - - const notification = makeNotification({ id: 'sse-1', read: false }); - lastEventSource!.simulate('notification', JSON.stringify(notification)); - - expect(stream.notifications).toHaveLength(1); - expect(stream.notifications[0].id).toBe('sse-1'); - expect(stream.unreadCount).toBe(1); - }); - - it('SSE notification event with read:true does not increment unreadCount', async () => { - mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); - const stream = createNotificationStream(); - stream.init(); - - const notification = makeNotification({ id: 'sse-2', read: true }); - lastEventSource!.simulate('notification', JSON.stringify(notification)); - - expect(stream.notifications).toHaveLength(1); - expect(stream.unreadCount).toBe(0); - }); -}); diff --git a/frontend/src/lib/hooks/useNotificationStream.svelte.ts b/frontend/src/lib/hooks/useNotificationStream.svelte.ts deleted file mode 100644 index 0a03dada..00000000 --- a/frontend/src/lib/hooks/useNotificationStream.svelte.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { type NotificationItem, parseNotificationEvent } from '$lib/utils/notifications'; - -export type { NotificationItem }; - -export function createNotificationStream() { - let notifications = $state([]); - let unreadCount = $state(0); - let eventSource: EventSource | null = null; - - async function fetchNotifications(): Promise { - try { - const res = await fetch('/api/notifications?size=10'); - if (res.ok) { - const data = await res.json(); - notifications = data.content ?? []; - } - } catch (e) { - console.error('Failed to fetch notifications', e); - } - } - - async function fetchUnreadCount(): Promise { - try { - const res = await fetch('/api/notifications/unread-count'); - if (res.ok) { - const data = await res.json(); - unreadCount = data.count; - } - } catch (e) { - console.error('Failed to fetch unread count', e); - } - } - - async function markRead(notification: NotificationItem): Promise { - if (!notification.read) { - try { - await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); - notification.read = true; - unreadCount = Math.max(0, unreadCount - 1); - } catch (e) { - console.error('Failed to mark notification as read', e); - } - } - } - - async function markAllRead(): Promise { - try { - await fetch('/api/notifications/read-all', { method: 'POST' }); - for (const n of notifications) { - n.read = true; - } - unreadCount = 0; - } catch (e) { - console.error('Failed to mark all notifications as read', e); - } - } - - function init(): void { - fetchUnreadCount(); - eventSource = new EventSource('/api/notifications/stream'); - eventSource.addEventListener('notification', (e) => { - const notification = parseNotificationEvent(e.data); - if (!notification) return; - notifications = [notification, ...notifications]; - if (!notification.read) unreadCount += 1; - }); - eventSource.onopen = () => { - fetchUnreadCount(); - }; - eventSource.onerror = () => { - // Close on error to avoid repeated reconnect noise - eventSource?.close(); - }; - } - - function destroy(): void { - eventSource?.close(); - eventSource = null; - } - - return { - get notifications() { - return notifications; - }, - get unreadCount() { - return unreadCount; - }, - fetchNotifications, - fetchUnreadCount, - markRead, - markAllRead, - init, - destroy - }; -} diff --git a/frontend/src/lib/stores/notifications.svelte.spec.ts b/frontend/src/lib/stores/notifications.svelte.spec.ts new file mode 100644 index 00000000..01acbd21 --- /dev/null +++ b/frontend/src/lib/stores/notifications.svelte.spec.ts @@ -0,0 +1,108 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { NotificationItem } from '$lib/utils/notifications'; + +let lastEventSource: MockEventSource | null = null; +let eventSourceCount = 0; + +class MockEventSource { + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + close = vi.fn(); + private listeners: Record void)[]> = {}; + + constructor() { + eventSourceCount += 1; + // eslint-disable-next-line @typescript-eslint/no-this-alias + lastEventSource = this; + } + + addEventListener(type: string, fn: (e: MessageEvent) => void) { + if (!this.listeners[type]) this.listeners[type] = []; + this.listeners[type].push(fn); + } + + simulate(type: string, data: string) { + const event = new MessageEvent(type, { data }); + for (const fn of this.listeners[type] ?? []) { + fn(event); + } + } +} + +vi.stubGlobal('EventSource', MockEventSource); + +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +const { notificationStore, __resetForTest } = await import('./notifications.svelte'); + +beforeEach(() => { + mockFetch.mockReset(); + mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); + lastEventSource = null; + eventSourceCount = 0; + __resetForTest(); +}); + +function makeNotification(overrides: Partial = {}): NotificationItem { + return { + id: 'n1', + type: 'REPLY', + actorName: 'Hans', + documentId: 'doc-1', + documentTitle: null, + referenceId: 'ref-1', + annotationId: null, + read: false, + createdAt: new Date().toISOString(), + ...overrides + }; +} + +describe('notificationStore (singleton)', () => { + it('opens a single EventSource across multiple init() calls', () => { + notificationStore.init(); + notificationStore.init(); + notificationStore.init(); + + expect(eventSourceCount).toBe(1); + }); + + it('closes the EventSource only after every init() is matched with destroy()', () => { + notificationStore.init(); + notificationStore.init(); + const es = lastEventSource!; + + notificationStore.destroy(); + expect(es.close).not.toHaveBeenCalled(); + + notificationStore.destroy(); + expect(es.close).toHaveBeenCalledTimes(1); + }); + + it('reopens a fresh EventSource after full teardown', () => { + notificationStore.init(); + notificationStore.destroy(); + notificationStore.init(); + + expect(eventSourceCount).toBe(2); + }); + + it('SSE notification event prepends notification and increments unreadCount', () => { + notificationStore.init(); + + const notification = makeNotification({ id: 'sse-1', read: false }); + lastEventSource!.simulate('notification', JSON.stringify(notification)); + + expect(notificationStore.notifications[0].id).toBe('sse-1'); + expect(notificationStore.unreadCount).toBe(1); + }); + + it('markAllRead resets unreadCount', async () => { + mockFetch.mockResolvedValue(new Response(null, { status: 200 })); + await notificationStore.markAllRead(); + + expect(mockFetch).toHaveBeenCalledWith('/api/notifications/read-all', { method: 'POST' }); + expect(notificationStore.unreadCount).toBe(0); + }); +}); diff --git a/frontend/src/lib/stores/notifications.svelte.ts b/frontend/src/lib/stores/notifications.svelte.ts new file mode 100644 index 00000000..28ac9eb9 --- /dev/null +++ b/frontend/src/lib/stores/notifications.svelte.ts @@ -0,0 +1,108 @@ +import { type NotificationItem, parseNotificationEvent } from '$lib/utils/notifications'; + +export type { NotificationItem }; + +let notifications = $state([]); +let unreadCount = $state(0); +let eventSource: EventSource | null = null; +let refCount = 0; + +async function fetchNotifications(): Promise { + try { + const res = await fetch('/api/notifications?size=10'); + if (res.ok) { + const data = await res.json(); + notifications = data.content ?? []; + } + } catch (e) { + console.error('Failed to fetch notifications', e); + } +} + +async function fetchUnreadCount(): Promise { + try { + const res = await fetch('/api/notifications/unread-count'); + if (res.ok) { + const data = await res.json(); + unreadCount = data.count; + } + } catch (e) { + console.error('Failed to fetch unread count', e); + } +} + +async function markRead(notification: NotificationItem): Promise { + if (!notification.read) { + try { + await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); + notification.read = true; + unreadCount = Math.max(0, unreadCount - 1); + } catch (e) { + console.error('Failed to mark notification as read', e); + } + } +} + +async function markAllRead(): Promise { + try { + await fetch('/api/notifications/read-all', { method: 'POST' }); + for (const n of notifications) { + n.read = true; + } + unreadCount = 0; + } catch (e) { + console.error('Failed to mark all notifications as read', e); + } +} + +function init(): void { + refCount += 1; + if (refCount > 1) return; + + fetchUnreadCount(); + eventSource = new EventSource('/api/notifications/stream'); + eventSource.addEventListener('notification', (e) => { + const notification = parseNotificationEvent((e as MessageEvent).data); + if (!notification) return; + notifications = [notification, ...notifications]; + if (!notification.read) unreadCount += 1; + }); + eventSource.onopen = () => { + fetchUnreadCount(); + }; + eventSource.onerror = () => { + eventSource?.close(); + }; +} + +function destroy(): void { + if (refCount === 0) return; + refCount -= 1; + if (refCount === 0) { + eventSource?.close(); + eventSource = null; + } +} + +export function __resetForTest(): void { + eventSource?.close(); + eventSource = null; + refCount = 0; + notifications = []; + unreadCount = 0; +} + +export const notificationStore = { + get notifications() { + return notifications; + }, + get unreadCount() { + return unreadCount; + }, + fetchNotifications, + fetchUnreadCount, + markRead, + markAllRead, + init, + destroy +};