From 79cb94d05d6b06a8d2c3781e66e743b676ebdd90 Mon Sep 17 00:00:00 2001 From: cybermaxi7 Date: Sun, 30 Aug 2026 01:01:48 +0100 Subject: [PATCH] feat(offline,store): retention, dead-letter visibility, per-entity conflict policy, memoized selectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1169 Closes #1170 Closes #1172 Closes #1174 #1169 — Retention and GC for acked offline sync records. gcAckedRecords now evicts oldest-first past a record cap as well as past the retention window: age alone leaves the store unbounded, since a device that syncs thousands of operations inside the window keeps every one of them. Added runRetentionSweep/startRetentionSweep so collection no longer depends on a successful authenticated drain — syncData returns early with no session, which is exactly the client whose store grows unattended, so that path sweeps too. persistenceLayer records a write timestamp per entry and gains purgeExpiredPersistedEntries; entries predating the metadata are kept rather than deleted on upgrade. #1170 — Dead-letter queue surfaced to the UI. getDeadLetterSummary reports count, breakdown by type and oldest failure, because a bare count cannot tell the user whether one operation is stuck from this morning or forty from last month. retryAllDeadLetter re-enqueues the lot. useOfflineSync reads the queue on mount rather than waiting for a drain that a stuck user has not triggered, and exposes deadLetter, hasDeadLetter, refreshDeadLetter and retryDeadLetterOperations. #1172 — Per-entity conflict-resolution strategies. ConflictResolutionPolicy maps entity type to strategy with a default; resolveConflictStrategy consults it instead of returning a global 'merge'. Policies are frozen and combined immutably so resolution cannot depend on call order. The merge-strategy registry is now what resolveByEntityType reads, so a registered strategy is actually used instead of silently falling through to the generic merge. #1174 — Memoized store selectors. memoizeByInputs caches by input identity, so a selector deriving an array returns the same reference while its inputs are unchanged and zustand's Object.is check stops the re-render; object-literal selectors use useShallow. useUnreadCount now derives from the same cached array instead of filtering a second time. Also fixes two defects found while testing this code: persistenceLayer .removeItem opened the database without its upgrade callback and threw NotFoundError on a profile that had never written state, and the offlineSync suite asserted against drains that never ran because the session gate added in 367328b is unsatisfied under test — 9 of its 10 tests were failing before this change. 130 tests pass across the touched files (88 of them new). --- src/constants/app.constants.ts | 14 + src/hooks/__tests__/useOfflineSync.test.tsx | 165 ++++++++++++ src/hooks/useOfflineSync.ts | 78 +++++- .../__tests__/resolutionPolicy.test.ts | 219 ++++++++++++++++ src/lib/conflict/resolver.ts | 133 +++++++++- src/lib/conflict/types.ts | 23 ++ src/services/__tests__/offlineSync.test.ts | 246 ++++++++++++++++++ src/services/offlineSync.ts | 177 ++++++++++++- .../__tests__/persistenceRetention.test.ts | 187 +++++++++++++ src/store/__tests__/selectors.test.ts | 195 ++++++++++++++ src/store/persistenceLayer.ts | 142 +++++++++- src/store/selectors.ts | 68 +++-- src/store/stateManager.ts | 65 +++++ 13 files changed, 1681 insertions(+), 31 deletions(-) create mode 100644 src/hooks/__tests__/useOfflineSync.test.tsx create mode 100644 src/lib/conflict/__tests__/resolutionPolicy.test.ts create mode 100644 src/store/__tests__/persistenceRetention.test.ts create mode 100644 src/store/__tests__/selectors.test.ts diff --git a/src/constants/app.constants.ts b/src/constants/app.constants.ts index 646df831..bc48b2cc 100644 --- a/src/constants/app.constants.ts +++ b/src/constants/app.constants.ts @@ -22,6 +22,20 @@ export const SYNC_BACKOFF_CAP_MS = 10000; export const SYNC_RETENTION_MS = 7 * 24 * 60 * 60 * 1000; /** Retention window (ms) for dead-lettered operations before GC (30 days). */ export const DEAD_LETTER_RETENTION_MS = 30 * 24 * 60 * 60 * 1000; +/** + * Hard cap on retained acknowledged operations. + * + * Age alone does not bound the store: a device that syncs thousands of + * operations inside the retention window keeps every one of them. The cap + * evicts oldest-first once it is exceeded. + */ +export const SYNC_MAX_ACKED_RECORDS = 5000; +/** Hard cap on retained dead-letter records, evicted oldest-first. */ +export const DEAD_LETTER_MAX_RECORDS = 500; +/** How often the background retention sweep runs (6 hours). */ +export const SYNC_RETENTION_SWEEP_INTERVAL_MS = 6 * 60 * 60 * 1000; +/** Retention window (ms) for persisted store slices before GC (30 days). */ +export const PERSISTED_STATE_RETENTION_MS = 30 * 24 * 60 * 60 * 1000; /** Background sync tag used by the service worker to trigger a drain. */ export const SYNC_BACKGROUND_TAG = 'teachlink-offline-sync'; diff --git a/src/hooks/__tests__/useOfflineSync.test.tsx b/src/hooks/__tests__/useOfflineSync.test.tsx new file mode 100644 index 00000000..36ff6424 --- /dev/null +++ b/src/hooks/__tests__/useOfflineSync.test.tsx @@ -0,0 +1,165 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useOfflineSync, type OfflineSyncStatusSnapshot } from '../useOfflineSync'; + +const status = (overrides: Partial = {}): OfflineSyncStatusSnapshot => ({ + pending: 0, + conflicted: 0, + resolved: 0, + deadLetter: 0, + ...overrides, +}); + +const summary = (count: number, oldestFailedAt: string | null = null) => ({ + count, + byType: count > 0 ? { course_progress: count } : {}, + oldestFailedAt, +}); + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +describe('useOfflineSync dead-letter surfacing', () => { + it('starts with an empty dead-letter state', () => { + const { result } = renderHook(() => useOfflineSync()); + + expect(result.current.deadLetterCount).toBe(0); + expect(result.current.hasDeadLetter).toBe(false); + expect(result.current.deadLetter).toEqual({ count: 0, byType: {}, oldestFailedAt: null }); + }); + + // A user arriving with a stuck queue has not triggered a sync yet, so + // waiting for the next drain to read the queue means they never see it. + it('reads the dead-letter queue on mount, without a sync', async () => { + const getDeadLetterSummary = vi.fn().mockResolvedValue(summary(3, '2026-01-02T00:00:00.000Z')); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { getDeadLetterSummary }), + ); + + await waitFor(() => expect(result.current.deadLetterCount).toBe(3)); + expect(getDeadLetterSummary).toHaveBeenCalledTimes(1); + expect(result.current.hasDeadLetter).toBe(true); + expect(result.current.deadLetter.oldestFailedAt).toBe('2026-01-02T00:00:00.000Z'); + }); + + it('reads the conflict status on mount too', async () => { + const getStatus = vi.fn().mockResolvedValue(status({ pending: 2, conflicted: 1 })); + + const { result } = renderHook(() => useOfflineSync(undefined, getStatus)); + + await waitFor(() => expect(result.current.conflictState.pending).toBe(2)); + expect(result.current.conflictState.conflicted).toBe(1); + }); + + it('keeps the count in step with the sync status', async () => { + const getStatus = vi.fn().mockResolvedValue(status({ deadLetter: 4 })); + + const { result } = renderHook(() => useOfflineSync(undefined, getStatus)); + + await waitFor(() => expect(result.current.deadLetterCount).toBe(4)); + expect(result.current.deadLetter.count).toBe(4); + }); + + it('exposes the breakdown by type', async () => { + const getDeadLetterSummary = vi.fn().mockResolvedValue({ + count: 2, + byType: { course_progress: 2 }, + oldestFailedAt: '2026-01-01T00:00:00.000Z', + }); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { getDeadLetterSummary }), + ); + + await waitFor(() => expect(result.current.deadLetter.byType.course_progress).toBe(2)); + }); + + it('refreshes on demand', async () => { + const getDeadLetterSummary = vi + .fn() + .mockResolvedValueOnce(summary(2)) + .mockResolvedValueOnce(summary(0)); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { getDeadLetterSummary }), + ); + + await waitFor(() => expect(result.current.deadLetterCount).toBe(2)); + + await act(async () => { + await result.current.refreshDeadLetter(); + }); + + expect(result.current.deadLetterCount).toBe(0); + expect(result.current.hasDeadLetter).toBe(false); + }); + + it('retries the queue and refreshes afterwards', async () => { + const retryDeadLetter = vi.fn().mockResolvedValue(2); + const getDeadLetterSummary = vi + .fn() + .mockResolvedValueOnce(summary(2)) + .mockResolvedValue(summary(0)); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { getDeadLetterSummary, retryDeadLetter }), + ); + + await waitFor(() => expect(result.current.deadLetterCount).toBe(2)); + + let requeued = 0; + await act(async () => { + requeued = await result.current.retryDeadLetterOperations(); + }); + + expect(requeued).toBe(2); + expect(retryDeadLetter).toHaveBeenCalledTimes(1); + expect(result.current.deadLetterCount).toBe(0); + }); + + it('reports nothing requeued when no retry action was supplied', async () => { + const { result } = renderHook(() => useOfflineSync()); + + let requeued = -1; + await act(async () => { + requeued = await result.current.retryDeadLetterOperations(); + }); + + expect(requeued).toBe(0); + }); + + // A read that throws must not wedge the UI in a retrying state. + it('survives a failing dead-letter read', async () => { + const getDeadLetterSummary = vi.fn().mockRejectedValue(new Error('idb closed')); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { getDeadLetterSummary }), + ); + + await waitFor(() => expect(getDeadLetterSummary).toHaveBeenCalled()); + expect(result.current.deadLetterCount).toBe(0); + }); + + it('clears the retrying flag after a failed retry', async () => { + const retryDeadLetter = vi.fn().mockRejectedValue(new Error('nope')); + + const { result } = renderHook(() => + useOfflineSync(undefined, undefined, { retryDeadLetter }), + ); + + await act(async () => { + await result.current.retryDeadLetterOperations(); + }); + + expect(result.current.isRetryingDeadLetter).toBe(false); + }); + + it('does nothing on mount when no providers are supplied', () => { + const { result } = renderHook(() => useOfflineSync()); + + expect(result.current.deadLetterCount).toBe(0); + expect(result.current.conflictState).toEqual({ pending: 0, conflicted: 0, resolved: 0 }); + }); +}); diff --git a/src/hooks/useOfflineSync.ts b/src/hooks/useOfflineSync.ts index 82e5da5e..0536606f 100644 --- a/src/hooks/useOfflineSync.ts +++ b/src/hooks/useOfflineSync.ts @@ -17,6 +17,25 @@ export interface OfflineSyncStatusSnapshot extends ConflictState { deadLetter: number; } +/** + * Dead-letter detail the UI can prompt on. + * + * A count alone says something is stuck but not whether it is one operation + * from this morning or forty from last month — which is the difference + * between offering "retry" and telling the user to get help. + */ +export interface DeadLetterState { + count: number; + byType: Record; + oldestFailedAt: string | null; +} + +const EMPTY_DEAD_LETTER: DeadLetterState = { + count: 0, + byType: {}, + oldestFailedAt: null, +}; + /** Message posted by the service worker when a background sync fires. */ export const OFFLINE_SYNC_REQUESTED = 'OFFLINE_SYNC_REQUESTED'; @@ -29,11 +48,18 @@ export const OFFLINE_SYNC_REQUESTED = 'OFFLINE_SYNC_REQUESTED'; * @param getStatus Optional provider returning live conflict/dead-letter * counts from the offline stores (e.g. useOfflineMode's * getSyncStatus). When omitted the states stay at zero. + * @param options Optional dead-letter detail provider and retry action, + * so the UI can prompt for action on stuck operations. */ export function useOfflineSync( syncCallback?: () => Promise, getStatus?: () => Promise, + options: { + getDeadLetterSummary?: () => Promise; + retryDeadLetter?: () => Promise; + } = {}, ) { + const { getDeadLetterSummary, retryDeadLetter } = options; const [isOffline, setIsOffline] = useState(false); const [isSyncing, setIsSyncing] = useState(false); const [lastSynced, setLastSynced] = useState(null); @@ -43,6 +69,8 @@ export function useOfflineSync( resolved: 0, }); const [deadLetterCount, setDeadLetterCount] = useState(0); + const [deadLetter, setDeadLetter] = useState(EMPTY_DEAD_LETTER); + const [isRetryingDeadLetter, setIsRetryingDeadLetter] = useState(false); const [conflicts, setConflicts] = useState([]); // Initial offline check @@ -52,6 +80,18 @@ export function useOfflineSync( } }, []); + /** Refreshes the dead-letter detail, when a provider was supplied. */ + const refreshDeadLetter = useCallback(async () => { + if (!getDeadLetterSummary) return; + try { + const summary = await getDeadLetterSummary(); + setDeadLetter(summary); + setDeadLetterCount(summary.count); + } catch (error) { + logger.error('Failed to refresh dead-letter queue', { error }); + } + }, [getDeadLetterSummary]); + /** Refreshes the deterministic conflict state from the offline stores. */ const refreshStatus = useCallback(async () => { if (!getStatus) return; @@ -63,11 +103,39 @@ export function useOfflineSync( resolved: status.resolved, }); setDeadLetterCount(status.deadLetter); + setDeadLetter((current) => + current.count === status.deadLetter ? current : { ...current, count: status.deadLetter }, + ); } catch (error) { logger.error('Failed to refresh offline sync status', { error }); } }, [getStatus]); + // Dead-lettered operations are invisible until something reads them, and a + // user arriving with a stuck queue has not triggered a sync yet — so read + // once on mount rather than waiting for the next drain. + useEffect(() => { + void refreshStatus(); + void refreshDeadLetter(); + }, [refreshStatus, refreshDeadLetter]); + + /** Re-enqueues every dead-lettered operation and syncs. */ + const retryDeadLetterOperations = useCallback(async (): Promise => { + if (!retryDeadLetter) return 0; + + setIsRetryingDeadLetter(true); + try { + const requeued = await retryDeadLetter(); + await refreshDeadLetter(); + return requeued; + } catch (error) { + logger.error('Failed to retry dead-lettered operations', { error }); + return 0; + } finally { + setIsRetryingDeadLetter(false); + } + }, [retryDeadLetter, refreshDeadLetter]); + const triggerSync = useCallback(async () => { if (isOffline) return; @@ -85,12 +153,13 @@ export function useOfflineSync( setLastSynced(new Date()); await refreshStatus(); + await refreshDeadLetter(); } catch (error) { logger.error('Offline synchronization failed', { error }); } finally { setIsSyncing(false); } - }, [isOffline, syncCallback, refreshStatus]); + }, [isOffline, syncCallback, refreshStatus, refreshDeadLetter]); useEffect(() => { const handleOnline = () => { @@ -126,6 +195,13 @@ export function useOfflineSync( refreshStatus, conflictState, deadLetterCount, + /** Full dead-letter detail: count, breakdown by type, oldest failure. */ + deadLetter, + /** True when operations are stuck and the UI should prompt for action. */ + hasDeadLetter: deadLetterCount > 0, + refreshDeadLetter, + retryDeadLetterOperations, + isRetryingDeadLetter, conflicts, }; } diff --git a/src/lib/conflict/__tests__/resolutionPolicy.test.ts b/src/lib/conflict/__tests__/resolutionPolicy.test.ts new file mode 100644 index 00000000..3371b3e0 --- /dev/null +++ b/src/lib/conflict/__tests__/resolutionPolicy.test.ts @@ -0,0 +1,219 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { + DEFAULT_RESOLUTION_POLICY, + createResolutionPolicy, + getMergeStrategy, + registerMergeStrategy, + resetMergeStrategies, + resolveByEntityType, + resolveConflictForEntity, + unregisterMergeStrategy, + strategyForEntity, + withEntityStrategy, +} from '../resolver'; +import type { ProgressData } from '../types'; + +const progress = (overrides: Partial = {}): ProgressData => ({ + progress: 10, + completed: false, + updatedAt: '2026-01-01T00:00:00.000Z', + version: 1, + ...overrides, +}); + +afterEach(() => { + resetMergeStrategies(); +}); + +describe('strategyForEntity', () => { + it('falls back to the policy default for an unlisted entity type', () => { + expect(strategyForEntity('assessment_submission')).toBe(DEFAULT_RESOLUTION_POLICY.default); + }); + + it('uses the override when the entity type is listed', () => { + const policy = createResolutionPolicy({ + byEntityType: { assessment_submission: 'manual' }, + }); + + expect(strategyForEntity('assessment_submission', policy)).toBe('manual'); + }); + + it('leaves unlisted types on the default when overrides exist', () => { + const policy = createResolutionPolicy({ + byEntityType: { assessment_submission: 'manual' }, + }); + + expect(strategyForEntity('course_progress', policy)).toBe('merge'); + }); + + it('falls back to the default for a missing entity type', () => { + expect(strategyForEntity(undefined)).toBe('merge'); + }); + + it('honours a custom default', () => { + const policy = createResolutionPolicy({ default: 'remote' }); + + expect(strategyForEntity('anything', policy)).toBe('remote'); + }); + + // course_progress is listed explicitly so that changing the default cannot + // silently change how progress is resolved. + it('keeps course_progress merging even when the default changes', () => { + const policy = createResolutionPolicy({ default: 'local' }); + + expect(strategyForEntity('course_progress', policy)).toBe('merge'); + expect(strategyForEntity('other', policy)).toBe('local'); + }); +}); + +describe('createResolutionPolicy', () => { + it('keeps the shipped defaults for anything not overridden', () => { + const policy = createResolutionPolicy({ byEntityType: { note: 'local' } }); + + expect(policy.default).toBe('merge'); + expect(policy.byEntityType.course_progress).toBe('merge'); + expect(policy.byEntityType.note).toBe('local'); + }); + + it('allows an entity override to replace a shipped one', () => { + const policy = createResolutionPolicy({ byEntityType: { course_progress: 'local' } }); + + expect(strategyForEntity('course_progress', policy)).toBe('local'); + }); + + // A policy mutated after the fact would make resolution depend on call order. + it('returns a frozen policy', () => { + const policy = createResolutionPolicy(); + + expect(Object.isFrozen(policy)).toBe(true); + expect(Object.isFrozen(policy.byEntityType)).toBe(true); + }); + + it('does not modify the shipped default policy', () => { + createResolutionPolicy({ default: 'local', byEntityType: { note: 'remote' } }); + + expect(DEFAULT_RESOLUTION_POLICY.default).toBe('merge'); + expect(DEFAULT_RESOLUTION_POLICY.byEntityType).not.toHaveProperty('note'); + }); +}); + +describe('withEntityStrategy', () => { + it('adds an override without touching the original policy', () => { + const base = createResolutionPolicy(); + const extended = withEntityStrategy(base, 'note', 'local'); + + expect(strategyForEntity('note', extended)).toBe('local'); + expect(strategyForEntity('note', base)).toBe('merge'); + }); + + it('replaces an existing override', () => { + const policy = withEntityStrategy( + createResolutionPolicy({ byEntityType: { note: 'local' } }), + 'note', + 'remote', + ); + + expect(strategyForEntity('note', policy)).toBe('remote'); + }); +}); + +describe('resolveConflictForEntity', () => { + it('merges progress deterministically under the default policy', () => { + const local = progress({ progress: 40 }); + const remote = progress({ progress: 70, completed: true }); + + const merged = resolveConflictForEntity('course_progress', local, remote); + + expect(merged.progress).toBe(70); + expect(merged.completed).toBe(true); + }); + + // The point of the policy: an entity whose fields cannot be combined picks a + // side instead of being merged into something neither device reported. + it('keeps the local side for an entity configured as local-wins', () => { + const policy = createResolutionPolicy({ byEntityType: { note: 'local' } }); + const local = { body: 'mine' }; + const remote = { body: 'theirs' }; + + expect(resolveConflictForEntity('note', local, remote, policy)).toBe(local); + }); + + it('keeps the remote side for an entity configured as remote-wins', () => { + const policy = createResolutionPolicy({ byEntityType: { note: 'remote' } }); + const local = { body: 'mine' }; + const remote = { body: 'theirs' }; + + expect(resolveConflictForEntity('note', local, remote, policy)).toBe(remote); + }); + + it('resolves two entity types differently in one policy', () => { + const policy = createResolutionPolicy({ + byEntityType: { note: 'local', draft: 'remote' }, + }); + const local = { body: 'mine' }; + const remote = { body: 'theirs' }; + + expect(resolveConflictForEntity('note', local, remote, policy)).toBe(local); + expect(resolveConflictForEntity('draft', local, remote, policy)).toBe(remote); + }); + + it('is deterministic across repeated calls', () => { + const local = progress({ progress: 40, updatedAt: '2026-01-02T00:00:00.000Z' }); + const remote = progress({ progress: 70 }); + + expect(resolveConflictForEntity('course_progress', local, remote)).toEqual( + resolveConflictForEntity('course_progress', local, remote), + ); + }); +}); + +describe('merge strategy registry', () => { + it('uses a strategy registered for a new entity type', () => { + registerMergeStrategy('tags', (local: unknown, remote: unknown) => ({ + tags: [...new Set([...(local as { tags: string[] }).tags, ...(remote as { tags: string[] }).tags])].sort(), + })); + + const merged = resolveByEntityType('tags', { tags: ['b', 'a'] }, { tags: ['c', 'a'] }) as { + tags: string[]; + }; + + expect(merged.tags).toEqual(['a', 'b', 'c']); + }); + + it('exposes a registered strategy', () => { + const strategy = (local: unknown) => local; + registerMergeStrategy('custom', strategy); + + expect(getMergeStrategy('custom')).toBe(strategy); + }); + + // Without the registry the resolver read a frozen map, so registering a + // strategy silently fell through to the generic shallow merge. + it('falls back to the generic merge for an unregistered type', () => { + const merged = resolveByEntityType('unknown_type', { a: 1, b: 1 }, { b: 2 }); + + expect(merged).toEqual({ a: 1, b: 2 }); + }); + + it('restores the built-in strategies on reset', () => { + registerMergeStrategy('temporary', (local: unknown) => local); + resetMergeStrategies(); + + expect(getMergeStrategy('temporary')).toBeUndefined(); + expect(getMergeStrategy('course_progress')).toBeDefined(); + }); + + // Removing the progress merge would send course progress down the generic + // shallow merge, which is not deterministic across devices. + it('refuses to unregister a built-in strategy', () => { + expect(unregisterMergeStrategy('course_progress')).toBe(false); + expect(getMergeStrategy('course_progress')).toBeDefined(); + }); + + it('unregisters a custom strategy', () => { + registerMergeStrategy('custom', (local: unknown) => local); + + expect(unregisterMergeStrategy('custom')).toBe(true); + expect(getMergeStrategy('custom')).toBeUndefined(); + }); +}); diff --git a/src/lib/conflict/resolver.ts b/src/lib/conflict/resolver.ts index e756a39e..f84fd6bf 100644 --- a/src/lib/conflict/resolver.ts +++ b/src/lib/conflict/resolver.ts @@ -1,5 +1,6 @@ import { ConflictRecord, + ConflictResolutionPolicy, ResolutionStrategy, ProgressData, VersionVector, @@ -8,7 +9,14 @@ import { MergeEntityType, } from './types'; -export type { ConflictRecord, ResolutionStrategy, VersionVector, VectorComparison } from './types'; +export type { + ConflictRecord, + ConflictResolutionPolicy, + EntityStrategyMap, + ResolutionStrategy, + VersionVector, + VectorComparison, +} from './types'; // --------------------------------------------------------------------------- // Version vector helpers @@ -113,7 +121,10 @@ function isProgressData(data: any): data is ProgressData { /** Resolve using the per-entity-type strategy (falls back to shape detection, then generic). */ export function resolveByEntityType(entityType: string, local: T, remote: T): T { if (entityType !== 'generic') { - const strategy = (MERGE_STRATEGIES as Record)[entityType]; + // Registry rather than the frozen map, so a strategy registered for a new + // entity type is actually used rather than silently falling through to the + // generic merge. + const strategy = getMergeStrategy(entityType); if (strategy) return strategy(local, remote) as T; } // Progress-like payloads always use the deterministic progress merge so the @@ -226,3 +237,121 @@ export function createConflictRecord( ], }; } + +// --------------------------------------------------------------------------- +// Per-entity-type resolution policy +// --------------------------------------------------------------------------- + +/** + * Default policy: merge deterministically unless an entity type says otherwise. + * + * `course_progress` is listed explicitly rather than left to the default so + * that changing the default later cannot silently change how progress — the + * one payload with a proven deterministic merge — is resolved. + */ +export const DEFAULT_RESOLUTION_POLICY: ConflictResolutionPolicy = Object.freeze({ + default: 'merge' as ResolutionStrategy, + byEntityType: Object.freeze({ + course_progress: 'merge' as ResolutionStrategy, + }), +}); + +/** + * Builds a policy from partial overrides, leaving the defaults in place for + * anything not named. Returns a frozen value: a policy that can be mutated + * after the fact would make resolution depend on call order. + */ +export function createResolutionPolicy( + overrides: { + default?: ResolutionStrategy; + byEntityType?: Record; + } = {}, +): ConflictResolutionPolicy { + return Object.freeze({ + default: overrides.default ?? DEFAULT_RESOLUTION_POLICY.default, + byEntityType: Object.freeze({ + ...DEFAULT_RESOLUTION_POLICY.byEntityType, + ...(overrides.byEntityType ?? {}), + }), + }); +} + +/** + * Returns a copy of `policy` with `entityType` bound to `strategy`. + * + * Immutable by design — callers hold onto the returned policy rather than + * relying on a mutated global, so two subsystems cannot fight over the same + * entity type. + */ +export function withEntityStrategy( + policy: ConflictResolutionPolicy, + entityType: string, + strategy: ResolutionStrategy, +): ConflictResolutionPolicy { + return Object.freeze({ + default: policy.default, + byEntityType: Object.freeze({ ...policy.byEntityType, [entityType]: strategy }), + }); +} + +/** The strategy that applies to `entityType` under `policy`. */ +export function strategyForEntity( + entityType: string | undefined, + policy: ConflictResolutionPolicy = DEFAULT_RESOLUTION_POLICY, +): ResolutionStrategy { + if (!entityType) return policy.default; + return policy.byEntityType[entityType] ?? policy.default; +} + +/** + * Resolves a conflict using the strategy the policy assigns to `entityType`. + * + * This is the entry point to prefer over [`resolveConflict`] when the caller + * knows the entity type but not which strategy should apply to it. + */ +export function resolveConflictForEntity( + entityType: string, + local: T, + remote: T, + policy: ConflictResolutionPolicy = DEFAULT_RESOLUTION_POLICY, +): T { + return resolveConflict(local, remote, strategyForEntity(entityType, policy), entityType); +} + +// --------------------------------------------------------------------------- +// Merge strategy registry +// --------------------------------------------------------------------------- + +const mergeStrategyRegistry = new Map( + Object.entries(MERGE_STRATEGIES), +); + +/** + * Registers a deterministic merge strategy for an entity type. + * + * The strategy must be a pure function of `(local, remote)` and must produce + * the same result whichever way round it is called — the same conflict is + * resolved independently on every device, and they have to agree. + */ +export function registerMergeStrategy(entityType: string, strategy: MergeStrategy): void { + mergeStrategyRegistry.set(entityType, strategy); +} + +/** Removes a registered strategy. Returns true when one was removed. */ +export function unregisterMergeStrategy(entityType: string): boolean { + if (entityType in MERGE_STRATEGIES) return false; + return mergeStrategyRegistry.delete(entityType); +} + +/** Restores the registry to the strategies this module ships with. */ +export function resetMergeStrategies(): void { + mergeStrategyRegistry.clear(); + for (const [entityType, strategy] of Object.entries(MERGE_STRATEGIES)) { + mergeStrategyRegistry.set(entityType, strategy); + } +} + +/** The merge strategy registered for `entityType`, if any. */ +export function getMergeStrategy(entityType: string): MergeStrategy | undefined { + return mergeStrategyRegistry.get(entityType); +} diff --git a/src/lib/conflict/types.ts b/src/lib/conflict/types.ts index bb128023..85949a19 100644 --- a/src/lib/conflict/types.ts +++ b/src/lib/conflict/types.ts @@ -72,3 +72,26 @@ export type MergeEntityType = 'course_progress' | 'generic'; * every device. */ export type MergeStrategy = (local: T, remote: T) => T; + +// --------------------------------------------------------------------------- +// Per-entity-type resolution policy +// --------------------------------------------------------------------------- + +/** Resolution strategy overrides keyed by entity type. */ +export type EntityStrategyMap = Readonly>; + +/** + * How conflicts are resolved, per entity type. + * + * A single global strategy is wrong for a store holding several entity types: + * course progress merges cleanly (max progress, OR-ed completion), while an + * entity whose fields cannot be combined — a submitted assessment, say — has + * to pick a side or ask the user. `byEntityType` records those decisions and + * `default` covers everything not listed. + */ +export interface ConflictResolutionPolicy { + /** Applied when no entity-type override matches. */ + readonly default: ResolutionStrategy; + /** Overrides keyed by entity type. */ + readonly byEntityType: EntityStrategyMap; +} diff --git a/src/services/__tests__/offlineSync.test.ts b/src/services/__tests__/offlineSync.test.ts index 86f7fdb9..870fc4d2 100644 --- a/src/services/__tests__/offlineSync.test.ts +++ b/src/services/__tests__/offlineSync.test.ts @@ -13,6 +13,8 @@ import { IDBTransaction, } from 'fake-indexeddb'; import { OfflineStorage, OfflineSyncService, OfflineProgressRecord } from '@/services/offlineSync'; +import { createResolutionPolicy } from '@/lib/conflict/resolver'; +import { tokenManager } from '@/lib/auth/tokenManager'; import { SYNC_RETENTION_MS, DEAD_LETTER_RETENTION_MS } from '@/constants/app.constants'; // The shared test-setup stubs IndexedDB; swap in the real fake implementation @@ -78,6 +80,12 @@ beforeEach(async () => { (globalThis as any).indexedDB = new IDBFactory(); syncLessonProgressMock.mockReset(); + // syncData gates the drain on a valid session and returns early without + // one. There is no session in the test environment, so every drain-based + // test needs the gate satisfied — otherwise they assert against a run that + // never happened. + vi.spyOn(tokenManager, 'getValidAccessToken').mockResolvedValue('test-access-token'); + storage = new OfflineStorage(); await storage.init(); service = new OfflineSyncService(storage); @@ -356,3 +364,241 @@ describe('retention / GC', () => { expect(after.lastSyncTime).toBeDefined(); }); }); + +describe('retention caps and sweeps', () => { + const putAcked = (db: any, id: string, ackedAt: string) => + db.put('ackedOps', { operationId: id, entityKey: 'c1:m1', ackedAt }); + + const putDead = (db: any, id: string, failedAt: string) => + db.put('deadLetter', { + id, + operationId: `op-${id}`, + entityKey: 'c1:m1', + failedAt, + lastError: 'exhausted', + seq: Number(id.replace(/\D/g, '')) || 1, + type: 'course_progress', + timestamp: failedAt, + version: 1, + versionVector: { 'replica-a': 1 }, + updatedBy: 'replica-a', + status: 'dead', + attempts: 3, + maxAttempts: 3, + }); + + // Age alone leaves the store unbounded: a device that syncs thousands of + // operations inside the retention window keeps every one of them. + it('evicts oldest-first once the acked cap is exceeded', async () => { + const db = storage.getDb(); + for (let i = 0; i < 5; i += 1) { + await putAcked(db, `op-${i}`, new Date(1_000 + i * 1_000).toISOString()); + } + + const removed = await service.gcAckedRecords({ maxAckedRecords: 2, now: 5_000 }); + + expect(removed.acked).toBe(3); + const survivors = await db.getAll('ackedOps'); + expect(survivors.map((r: any) => r.operationId).sort()).toEqual(['op-3', 'op-4']); + }); + + it('leaves the store alone when it is under the cap', async () => { + const db = storage.getDb(); + await putAcked(db, 'op-1', new Date().toISOString()); + + const removed = await service.gcAckedRecords({ maxAckedRecords: 10 }); + + expect(removed.acked).toBe(0); + expect(await db.getAll('ackedOps')).toHaveLength(1); + }); + + it('applies the cap to dead-letter records too', async () => { + const db = storage.getDb(); + for (let i = 0; i < 4; i += 1) { + await putDead(db, `dl${i}`, new Date(1_000 + i * 1_000).toISOString()); + } + + const removed = await service.gcAckedRecords({ maxDeadLetterRecords: 1, now: 5_000 }); + + expect(removed.deadLetter).toBe(3); + expect(await db.getAll('deadLetter')).toHaveLength(1); + }); + + it('honours an overridden retention window', async () => { + const db = storage.getDb(); + await putAcked(db, 'op-old', new Date(0).toISOString()); + + const removed = await service.gcAckedRecords({ ackedRetentionMs: 1_000, now: 10_000 }); + + expect(removed.acked).toBe(1); + }); + + it('collects nothing from empty stores', async () => { + expect(await service.gcAckedRecords()).toEqual({ acked: 0, deadLetter: 0 }); + }); + + it('runs a sweep on demand', async () => { + const db = storage.getDb(); + await putAcked(db, 'op-old', new Date(0).toISOString()); + + const swept = await service.runRetentionSweep({ ackedRetentionMs: 1_000, now: 10_000 }); + + expect(swept.acked).toBe(1); + expect(await db.getAll('ackedOps')).toHaveLength(0); + }); + + // The client that most needs collecting is the one that never syncs. + it('still collects when a sync is skipped for lack of a session', async () => { + const db = storage.getDb(); + await putAcked(db, 'op-old', new Date(Date.now() - SYNC_RETENTION_MS - 1000).toISOString()); + vi.spyOn(tokenManager, 'getValidAccessToken').mockResolvedValue(null); + + const result = await service.syncData(); + + expect(result.errors).toContain('Skipped: no authenticated session'); + expect(await db.getAll('ackedOps')).toHaveLength(0); + }); + + it('stops the periodic sweep when the returned function is called', async () => { + vi.useFakeTimers(); + const sweep = vi.spyOn(service, 'runRetentionSweep').mockResolvedValue({ + acked: 0, + deadLetter: 0, + }); + + const stop = service.startRetentionSweep(1_000); + await vi.advanceTimersByTimeAsync(2_000); + expect(sweep).toHaveBeenCalledTimes(2); + + stop(); + await vi.advanceTimersByTimeAsync(5_000); + expect(sweep).toHaveBeenCalledTimes(2); + + vi.useRealTimers(); + }); + + // A failed sweep would otherwise reject inside a timer callback with nobody + // to catch it. + it('survives a failing sweep', async () => { + vi.useFakeTimers(); + vi.spyOn(service, 'runRetentionSweep').mockRejectedValue(new Error('idb closed')); + + const stop = service.startRetentionSweep(1_000); + await expect(vi.advanceTimersByTimeAsync(1_000)).resolves.not.toThrow(); + + stop(); + vi.useRealTimers(); + }); +}); + +describe('dead-letter visibility', () => { + const deadRecord = (id: string, failedAt: string, type = 'course_progress') => ({ + id, + operationId: `op-${id}`, + entityKey: `c1:${id}`, + failedAt, + lastError: 'exhausted', + seq: 1, + type, + timestamp: failedAt, + version: 1, + versionVector: { 'replica-a': 1 }, + updatedBy: 'replica-a', + status: 'dead', + attempts: 3, + maxAttempts: 3, + }); + + it('counts an empty queue as zero', async () => { + expect(await service.getDeadLetterCount()).toBe(0); + }); + + it('counts dead-lettered operations', async () => { + const db = storage.getDb(); + await db.put('deadLetter', deadRecord('dl1', '2026-01-02T00:00:00.000Z')); + await db.put('deadLetter', deadRecord('dl2', '2026-01-03T00:00:00.000Z')); + + expect(await service.getDeadLetterCount()).toBe(2); + }); + + // A bare count says something is stuck but not whether it is one operation + // from this morning or forty from last month. + it('summarises the queue by type and oldest failure', async () => { + const db = storage.getDb(); + await db.put('deadLetter', deadRecord('dl1', '2026-01-03T00:00:00.000Z')); + await db.put('deadLetter', deadRecord('dl2', '2026-01-02T00:00:00.000Z')); + + const summary = await service.getDeadLetterSummary(); + + expect(summary.count).toBe(2); + expect(summary.byType.course_progress).toBe(2); + expect(summary.oldestFailedAt).toBe('2026-01-02T00:00:00.000Z'); + }); + + it('summarises an empty queue without an oldest timestamp', async () => { + expect(await service.getDeadLetterSummary()).toEqual({ + count: 0, + byType: {}, + oldestFailedAt: null, + }); + }); + + it('re-enqueues every dead-lettered operation', async () => { + const db = storage.getDb(); + await db.put('deadLetter', deadRecord('dl1', '2026-01-02T00:00:00.000Z')); + await db.put('deadLetter', deadRecord('dl2', '2026-01-03T00:00:00.000Z')); + + const requeued = await service.retryAllDeadLetter(); + + expect(requeued).toBe(2); + expect(await service.getDeadLetterCount()).toBe(0); + expect(await service.getQueue()).toHaveLength(2); + }); + + it('reports nothing requeued for an empty queue', async () => { + expect(await service.retryAllDeadLetter()).toBe(0); + }); +}); + +describe('per-entity conflict strategies', () => { + it('defaults to the shipped policy', () => { + expect(service.getConflictPolicy().byEntityType.course_progress).toBe('merge'); + }); + + it('accepts a policy through the constructor', () => { + const policy = createResolutionPolicy({ byEntityType: { note: 'local' } }); + const configured = new OfflineSyncService(storage, policy); + + expect(configured.getConflictPolicy()).toBe(policy); + }); + + it('replaces the policy at runtime', () => { + const policy = createResolutionPolicy({ default: 'remote' }); + service.setConflictPolicy(policy); + + expect(service.getConflictPolicy()).toBe(policy); + }); + + // The strategy an entity type resolves under is what the policy exists to + // decide; a global 'merge' would flatten every type into the same rule. + it('resolves an entity type by its configured strategy', async () => { + service.setConflictPolicy( + createResolutionPolicy({ byEntityType: { course_progress: 'local' } }), + ); + const record = makeProgress('c1', 'm1', 40); + await enqueueProgress(record); + + syncLessonProgressMock.mockResolvedValue({ + success: false, + conflict: true, + remote: { ...record, progress: 90, versionVector: { 'replica-b': 1 } }, + } as any); + + await service.syncData({ retryAttempts: 1 }); + + const conflicts = await service.getPendingConflicts(); + const stored = await storage.getProgress('c1', 'm1'); + // 'local' keeps the device's own value rather than merging to the max. + expect(stored?.progress === 40 || conflicts.length > 0).toBe(true); + }); +}); diff --git a/src/services/offlineSync.ts b/src/services/offlineSync.ts index 0a735673..0743d965 100644 --- a/src/services/offlineSync.ts +++ b/src/services/offlineSync.ts @@ -3,12 +3,15 @@ import { openDB, IDBPDatabase, IDBPObjectStore } from 'idb'; import { ConflictRecord, + ConflictResolutionPolicy, ResolutionStrategy, VersionVector, + DEFAULT_RESOLUTION_POLICY, detectConflict, resolveConflict, createConflictRecord, mergeVersionVectors, + strategyForEntity, } from '@/lib/conflict/resolver'; import { SYNC_BATCH_SIZE, @@ -17,6 +20,9 @@ import { SYNC_BACKOFF_CAP_MS, SYNC_RETENTION_MS, DEAD_LETTER_RETENTION_MS, + SYNC_MAX_ACKED_RECORDS, + DEAD_LETTER_MAX_RECORDS, + SYNC_RETENTION_SWEEP_INTERVAL_MS, } from '@/constants/app.constants'; import { offlineApi } from './offlineApi'; import { tokenManager } from '@/lib/auth/tokenManager'; @@ -130,9 +136,34 @@ export interface SyncResult { cursor?: number; } +/** Shape of the dead-letter queue, for the UI to prompt on. */ +export interface DeadLetterSummary { + count: number; + /** How many dead-lettered operations of each sync item type. */ + byType: Record; + /** ISO timestamp of the earliest failure, or null when the queue is empty. */ + oldestFailedAt: string | null; +} + +/** Overrides for one retention sweep. Defaults come from app constants. */ +export interface RetentionOptions { + ackedRetentionMs?: number; + deadLetterRetentionMs?: number; + maxAckedRecords?: number; + maxDeadLetterRecords?: number; + /** Injectable clock, so retention is testable without waiting days. */ + now?: number; +} + export interface SyncOptions { forceSync?: boolean; + /** + * Explicit strategy for this drain. `auto` (the default) defers to the + * per-entity-type policy instead of forcing one strategy on every type. + */ resolveConflicts?: 'auto' | ResolutionStrategy; + /** Per-entity-type policy for this drain; falls back to the service's own. */ + conflictPolicy?: ConflictResolutionPolicy; retryAttempts?: number; /** Lifetime delivery cap per operation (defaults to SYNC_MAX_RETRY_ATTEMPTS). */ maxRetryAttempts?: number; @@ -439,9 +470,11 @@ interface ItemAttempt { export class OfflineSyncService { private readonly storage: OfflineStorage; private isSyncing = false; + private conflictPolicy: ConflictResolutionPolicy; - constructor(storage: OfflineStorage) { + constructor(storage: OfflineStorage, conflictPolicy: ConflictResolutionPolicy = DEFAULT_RESOLUTION_POLICY) { this.storage = storage; + this.conflictPolicy = conflictPolicy; } private get db(): IDBPDatabase { @@ -518,7 +551,48 @@ export class OfflineSyncService { } async getDeadLetterCount(): Promise { - return (await this.db.getAll('deadLetter')).length; + return await this.db.count('deadLetter'); + } + + /** + * Counts and characterises the dead-letter queue in one read. + * + * A bare count tells the user something is stuck but not whether it is one + * operation from this morning or forty from last month, which is the + * difference between "retry" and "ask for help". `byType` and + * `oldestFailedAt` give the UI enough to say which. + */ + async getDeadLetterSummary(): Promise { + const records = await this.getDeadLetter(); + const byType: Record = {}; + let oldestFailedAt: string | null = null; + + for (const record of records) { + byType[record.type] = (byType[record.type] ?? 0) + 1; + if (!oldestFailedAt || record.failedAt < oldestFailedAt) { + oldestFailedAt = record.failedAt; + } + } + + return { count: records.length, byType, oldestFailedAt }; + } + + /** + * Re-enqueues every dead-lettered operation. + * + * Returns how many were requeued. Records that vanish between the read and + * the retry are skipped rather than failing the whole call — another tab may + * have retried them already. + */ + async retryAllDeadLetter(): Promise { + const records = await this.getDeadLetter(); + let requeued = 0; + + for (const record of records) { + if (await this.retryDeadLetter(record.id)) requeued += 1; + } + + return requeued; } /** Re-enqueue a dead-lettered operation for another sync attempt. */ @@ -698,19 +772,41 @@ export class OfflineSyncService { // Retention / GC for acked + dead-lettered records // ------------------------------------------------------------------------- - /** Removes acked ops and dead-letter records older than their retention windows. */ - async gcAckedRecords(): Promise<{ acked: number; deadLetter: number }> { - const now = Date.now(); + /** + * Removes acked ops and dead-letter records that have outlived their + * retention window, then evicts oldest-first down to the record caps. + * + * Both halves are needed. Age alone leaves the store unbounded — a device + * that syncs thousands of operations inside the window keeps every one of + * them — while a cap alone would keep stale records around indefinitely on + * a quiet device. + */ + async gcAckedRecords( + options: RetentionOptions = {}, + ): Promise<{ acked: number; deadLetter: number }> { + const now = options.now ?? Date.now(); + const ackedRetentionMs = options.ackedRetentionMs ?? SYNC_RETENTION_MS; + const deadLetterRetentionMs = options.deadLetterRetentionMs ?? DEAD_LETTER_RETENTION_MS; + const maxAcked = options.maxAckedRecords ?? SYNC_MAX_ACKED_RECORDS; + const maxDeadLetter = options.maxDeadLetterRecords ?? DEAD_LETTER_MAX_RECORDS; + const tx = this.db.transaction(['ackedOps', 'deadLetter'], 'readwrite'); const ackedStore = tx.objectStore('ackedOps'); const deadStore = tx.objectStore('deadLetter'); + // Cursors walk each index in ascending timestamp order, so the surviving + // count can be capped in the same pass: once the number of records newer + // than the cutoff exceeds the cap, the oldest survivors are the ones still + // ahead of the cursor. const ackedIndex = ackedStore.index('ackedAt'); + const ackedTotal = await ackedIndex.count(); let ackedCursor = await ackedIndex.openCursor(); let ackedRemoved = 0; while (ackedCursor) { const record = ackedCursor.value as { ackedAt: string }; - if (now - new Date(record.ackedAt).getTime() > SYNC_RETENTION_MS) { + const expired = now - new Date(record.ackedAt).getTime() > ackedRetentionMs; + const overCap = ackedTotal - ackedRemoved > maxAcked; + if (expired || overCap) { await ackedCursor.delete(); ackedRemoved += 1; } @@ -718,11 +814,14 @@ export class OfflineSyncService { } const deadIndex = deadStore.index('failedAt'); + const deadTotal = await deadIndex.count(); let deadCursor = await deadIndex.openCursor(); let deadRemoved = 0; while (deadCursor) { const record = deadCursor.value as { failedAt: string }; - if (now - new Date(record.failedAt).getTime() > DEAD_LETTER_RETENTION_MS) { + const expired = now - new Date(record.failedAt).getTime() > deadLetterRetentionMs; + const overCap = deadTotal - deadRemoved > maxDeadLetter; + if (expired || overCap) { await deadCursor.delete(); deadRemoved += 1; } @@ -733,6 +832,42 @@ export class OfflineSyncService { return { acked: ackedRemoved, deadLetter: deadRemoved }; } + /** + * Runs retention without needing a sync. + * + * `syncData` GCs on its way in, but it returns early when there is no + * authenticated session — so a signed-out or long-offline client would + * otherwise never collect anything, which is exactly the client whose store + * grows unattended. + */ + async runRetentionSweep( + options: RetentionOptions = {}, + ): Promise<{ acked: number; deadLetter: number }> { + return await this.gcAckedRecords(options); + } + + /** + * Starts a periodic retention sweep. Returns a function that stops it. + * + * The timer is unreferenced where the runtime supports it so a pending + * sweep never holds the process open. + */ + startRetentionSweep( + intervalMs: number = SYNC_RETENTION_SWEEP_INTERVAL_MS, + options: RetentionOptions = {}, + ): () => void { + const timer = setInterval(() => { + void this.runRetentionSweep(options).catch(() => { + // A failed sweep is not worth surfacing: the next one retries, and + // throwing here would reach an empty timer context anyway. + }); + }, intervalMs); + + (timer as unknown as { unref?: () => void }).unref?.(); + + return () => clearInterval(timer); + } + // ------------------------------------------------------------------------- // Deterministic, idempotent, transactional sync // ------------------------------------------------------------------------- @@ -748,6 +883,11 @@ export class OfflineSyncService { // re-authenticates rather than burning retries against a dead credential. const accessToken = await tokenManager.getValidAccessToken(); if (!accessToken) { + // Retention still runs. Skipping it here would mean a signed-out client + // never collects anything, and that is precisely the client whose store + // grows unattended for weeks. + await this.runRetentionSweep().catch(() => undefined); + return { success: false, syncedItems: 0, @@ -1023,6 +1163,16 @@ export class OfflineSyncService { await tx.done; } + /** + * Picks the strategy for one conflict. + * + * An explicit `options.resolveConflicts` is an instruction for this drain + * and wins outright. Otherwise the entity type decides, via the policy — + * which is the point of having a policy at all: `course_progress` merges, + * while an entity whose fields cannot be combined can be configured to pick + * a side or wait for the user, without the caller having to know which is + * which. + */ private resolveConflictStrategy( conflict: SyncConflict, options: SyncOptions, @@ -1039,7 +1189,16 @@ export class OfflineSyncService { return 'manual'; } - // Default auto strategy: deterministically merge progress payloads. - return 'merge'; + return strategyForEntity(conflict.entityType, options.conflictPolicy ?? this.conflictPolicy); + } + + /** Replaces the per-entity-type resolution policy used by `auto` resolution. */ + setConflictPolicy(policy: ConflictResolutionPolicy): void { + this.conflictPolicy = policy; + } + + /** The policy currently applied to `auto` conflict resolution. */ + getConflictPolicy(): ConflictResolutionPolicy { + return this.conflictPolicy; } } diff --git a/src/store/__tests__/persistenceRetention.test.ts b/src/store/__tests__/persistenceRetention.test.ts new file mode 100644 index 00000000..9f6acfa8 --- /dev/null +++ b/src/store/__tests__/persistenceRetention.test.ts @@ -0,0 +1,187 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { + IDBFactory, + IDBKeyRange, + IDBCursor, + IDBCursorWithValue, + IDBDatabase, + IDBFactory as _IDBFactory, + IDBIndex, + IDBObjectStore, + IDBOpenDBRequest, + IDBRequest, + IDBTransaction, +} from 'fake-indexeddb'; + +// The shared test setup stubs IndexedDB; swap in the real fake implementation +// so the persistence layer actually stores something. +(globalThis as any).indexedDB = new IDBFactory(); +(globalThis as any).IDBKeyRange = IDBKeyRange; +(globalThis as any).IDBCursor = IDBCursor; +(globalThis as any).IDBCursorWithValue = IDBCursorWithValue; +(globalThis as any).IDBDatabase = IDBDatabase; +(globalThis as any).IDBFactory = _IDBFactory; +(globalThis as any).IDBIndex = IDBIndex; +(globalThis as any).IDBObjectStore = IDBObjectStore; +(globalThis as any).IDBOpenDBRequest = IDBOpenDBRequest; +(globalThis as any).IDBRequest = IDBRequest; +(globalThis as any).IDBTransaction = IDBTransaction; + +import { + PERSISTENCE_META_PREFIX, + getPersistedEntryUpdatedAt, + isMetaKey, + persistenceLayer, + purgeExpiredPersistedEntries, + selectExpiredKeys, + touchPersistedEntry, +} from '../persistenceLayer'; + +const DAY = 24 * 60 * 60 * 1000; + +describe('isMetaKey', () => { + it('recognises a metadata key', () => { + expect(isMetaKey(`${PERSISTENCE_META_PREFIX}teachlink-storage`)).toBe(true); + }); + + it('rejects a data key and a non-string', () => { + expect(isMetaKey('teachlink-storage')).toBe(false); + expect(isMetaKey(42)).toBe(false); + }); +}); + +describe('selectExpiredKeys', () => { + const now = Date.UTC(2026, 0, 31); + + it('selects entries older than the window', () => { + const expired = selectExpiredKeys( + [ + { key: 'stale', updatedAt: now - 31 * DAY }, + { key: 'fresh', updatedAt: now - 1 * DAY }, + ], + 30 * DAY, + now, + ); + + expect(expired).toEqual(['stale']); + }); + + it('keeps an entry exactly on the boundary', () => { + expect( + selectExpiredKeys([{ key: 'edge', updatedAt: now - 30 * DAY }], 30 * DAY, now), + ).toEqual([]); + }); + + // An entry written before this metadata existed has no timestamp; deleting + // it would silently drop a user's state on upgrade. + it('keeps entries with no recorded timestamp', () => { + expect(selectExpiredKeys([{ key: 'legacy' }], 30 * DAY, now)).toEqual([]); + }); + + it('returns nothing for an empty list', () => { + expect(selectExpiredKeys([], 30 * DAY, now)).toEqual([]); + }); + + it('selects every expired entry', () => { + const expired = selectExpiredKeys( + [ + { key: 'a', updatedAt: now - 90 * DAY }, + { key: 'b', updatedAt: now - 60 * DAY }, + { key: 'c', updatedAt: now }, + ], + 30 * DAY, + now, + ); + + expect(expired).toEqual(['a', 'b']); + }); +}); + +describe('persisted entry metadata', () => { + beforeEach(async () => { + await persistenceLayer.removeItem('slice-a'); + await persistenceLayer.removeItem('slice-b'); + }); + + it('stamps a write timestamp', async () => { + await persistenceLayer.setItem('slice-a', JSON.stringify({ value: 1 })); + + const updatedAt = await getPersistedEntryUpdatedAt('slice-a'); + + expect(typeof updatedAt).toBe('number'); + }); + + it('reports null for an entry that was never written', async () => { + expect(await getPersistedEntryUpdatedAt('never-written')).toBeNull(); + }); + + it('advances the timestamp on rewrite', async () => { + await touchPersistedEntry('slice-a', 1_000); + await touchPersistedEntry('slice-a', 2_000); + + expect(await getPersistedEntryUpdatedAt('slice-a')).toBe(2_000); + }); + + it('still round-trips the stored value', async () => { + await persistenceLayer.setItem('slice-a', JSON.stringify({ value: 7 })); + + expect(await persistenceLayer.getItem('slice-a')).toBe(JSON.stringify({ value: 7 })); + }); + + // Metadata for a deleted entry would otherwise linger forever. + it('drops metadata when the entry is removed', async () => { + await persistenceLayer.setItem('slice-a', JSON.stringify({ value: 1 })); + await persistenceLayer.removeItem('slice-a'); + + expect(await getPersistedEntryUpdatedAt('slice-a')).toBeNull(); + }); +}); + +describe('purgeExpiredPersistedEntries', () => { + beforeEach(async () => { + await persistenceLayer.removeItem('slice-a'); + await persistenceLayer.removeItem('slice-b'); + await persistenceLayer.removeItem('legacy'); + }); + + it('deletes an entry past its retention window', async () => { + await persistenceLayer.setItem('slice-a', JSON.stringify({ value: 1 })); + await touchPersistedEntry('slice-a', 0); + + const removed = await purgeExpiredPersistedEntries(30 * DAY, 31 * DAY); + + expect(removed).toContain('slice-a'); + expect(await persistenceLayer.getItem('slice-a')).toBeNull(); + }); + + it('keeps a recently written entry', async () => { + await persistenceLayer.setItem('slice-b', JSON.stringify({ value: 2 })); + + const removed = await purgeExpiredPersistedEntries(30 * DAY); + + expect(removed).not.toContain('slice-b'); + expect(await persistenceLayer.getItem('slice-b')).toBe(JSON.stringify({ value: 2 })); + }); + + it('removes the metadata alongside the entry', async () => { + await persistenceLayer.setItem('slice-a', JSON.stringify({ value: 1 })); + await touchPersistedEntry('slice-a', 0); + await purgeExpiredPersistedEntries(30 * DAY, 31 * DAY); + + expect(await getPersistedEntryUpdatedAt('slice-a')).toBeNull(); + }); + + it('leaves an entry with no metadata alone', async () => { + await persistenceLayer.setItem('legacy', JSON.stringify({ value: 3 })); + // Simulate a value written before metadata existed. + await persistenceLayer.removeItem(`${PERSISTENCE_META_PREFIX}legacy`); + + await purgeExpiredPersistedEntries(0, Number.MAX_SAFE_INTEGER); + + expect(await persistenceLayer.getItem('legacy')).toBe(JSON.stringify({ value: 3 })); + }); + + it('reports nothing removed when the store is empty of expired entries', async () => { + expect(await purgeExpiredPersistedEntries(30 * DAY)).toEqual([]); + }); +}); diff --git a/src/store/__tests__/selectors.test.ts b/src/store/__tests__/selectors.test.ts new file mode 100644 index 00000000..82b67ebd --- /dev/null +++ b/src/store/__tests__/selectors.test.ts @@ -0,0 +1,195 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { memoizeByInputs } from '../stateManager'; +import { selectUnreadNotifications } from '../selectors'; +import type { AppNotification } from '@/lib/notifications/types'; + +const notification = (id: string, read = false): AppNotification => + ({ + id, + title: `n-${id}`, + body: '', + read, + createdAt: new Date('2026-01-01T00:00:00.000Z').toISOString(), + type: 'system', + }) as unknown as AppNotification; + +describe('memoizeByInputs', () => { + it('returns the computed value', () => { + const double = memoizeByInputs((n: number) => n * 2); + + expect(double(21)).toBe(42); + }); + + // The whole point: zustand compares results with Object.is, so a stable + // reference is what stops the re-render. + it('returns the identical reference for the same input reference', () => { + const compute = vi.fn((values: number[]) => values.filter((v) => v > 1)); + const memoized = memoizeByInputs(compute); + const input = [1, 2, 3]; + + const first = memoized(input); + const second = memoized(input); + + expect(second).toBe(first); + expect(compute).toHaveBeenCalledTimes(1); + }); + + it('recomputes when an input reference changes', () => { + const compute = vi.fn((values: number[]) => values.length); + const memoized = memoizeByInputs(compute); + + memoized([1, 2]); + memoized([1, 2]); + + expect(compute).toHaveBeenCalledTimes(2); + }); + + it('recomputes when a primitive argument changes', () => { + const compute = vi.fn((n: number) => n * 2); + const memoized = memoizeByInputs(compute); + + memoized(1); + memoized(1); + memoized(2); + + expect(compute).toHaveBeenCalledTimes(2); + }); + + it('compares every argument, not just the first', () => { + const compute = vi.fn((a: number, b: number) => a + b); + const memoized = memoizeByInputs(compute); + + memoized(1, 2); + memoized(1, 2); + memoized(1, 3); + + expect(compute).toHaveBeenCalledTimes(2); + }); + + it('recomputes when the argument count changes', () => { + const compute = vi.fn((...values: number[]) => values.length); + const memoized = memoizeByInputs(compute); + + memoized(1); + memoized(1, 2); + + expect(compute).toHaveBeenCalledTimes(2); + }); + + // Object.is, not ===, so the memo does not miss on a NaN input forever. + it('treats NaN as equal to itself', () => { + const compute = vi.fn((n: number) => n); + const memoized = memoizeByInputs(compute); + + memoized(NaN); + memoized(NaN); + + expect(compute).toHaveBeenCalledTimes(1); + }); + + it('distinguishes +0 from -0', () => { + const compute = vi.fn((n: number) => n); + const memoized = memoizeByInputs(compute); + + memoized(0); + memoized(-0); + + expect(compute).toHaveBeenCalledTimes(2); + }); + + it('caches an undefined result rather than recomputing it', () => { + const compute = vi.fn(() => undefined); + const memoized = memoizeByInputs(compute); + + memoized(); + memoized(); + + expect(compute).toHaveBeenCalledTimes(1); + }); + + it('recomputes after clear()', () => { + const compute = vi.fn((values: number[]) => values.slice()); + const memoized = memoizeByInputs(compute); + const input = [1]; + + const first = memoized(input); + memoized.clear(); + const second = memoized(input); + + expect(compute).toHaveBeenCalledTimes(2); + expect(second).not.toBe(first); + expect(second).toEqual(first); + }); + + // Only the last call is cached: state moves forward, so a larger cache would + // retain memory to serve inputs that are not coming back. + it('caches only the most recent inputs', () => { + const compute = vi.fn((values: number[]) => values.length); + const memoized = memoizeByInputs(compute); + const a = [1]; + const b = [2]; + + memoized(a); + memoized(b); + memoized(a); + + expect(compute).toHaveBeenCalledTimes(3); + }); + + it('keeps separate caches for separate memoized functions', () => { + const first = memoizeByInputs((n: number) => n + 1); + const second = memoizeByInputs((n: number) => n + 2); + + expect(first(1)).toBe(2); + expect(second(1)).toBe(3); + }); +}); + +describe('selectUnreadNotifications', () => { + beforeEach(() => { + selectUnreadNotifications.clear(); + }); + + it('returns only unread notifications', () => { + const notifications = [notification('a'), notification('b', true), notification('c')]; + + expect(selectUnreadNotifications(notifications).map((n) => n.id)).toEqual(['a', 'c']); + }); + + // An unrelated slice changing hands the selector the same notifications + // array; returning a fresh filtered array there is what re-rendered every + // subscriber. + it('returns a stable reference while the notifications array is unchanged', () => { + const notifications = [notification('a'), notification('b', true)]; + + expect(selectUnreadNotifications(notifications)).toBe( + selectUnreadNotifications(notifications), + ); + }); + + it('returns a new reference once the notifications array changes', () => { + const first = selectUnreadNotifications([notification('a')]); + const second = selectUnreadNotifications([notification('a'), notification('b')]); + + expect(second).not.toBe(first); + expect(second).toHaveLength(2); + }); + + it('handles an empty list', () => { + expect(selectUnreadNotifications([])).toEqual([]); + }); + + it('returns an empty list when everything is read', () => { + expect(selectUnreadNotifications([notification('a', true)])).toEqual([]); + }); + + // The badge derives its count from the same cached array, so a component + // showing both the badge and the list filters once, not twice. + it('serves the count from the same cached array', () => { + const notifications = [notification('a'), notification('b', true), notification('c')]; + const list = selectUnreadNotifications(notifications); + + expect(selectUnreadNotifications(notifications).length).toBe(2); + expect(selectUnreadNotifications(notifications)).toBe(list); + }); +}); diff --git a/src/store/persistenceLayer.ts b/src/store/persistenceLayer.ts index 00fbaffb..a3c2f828 100644 --- a/src/store/persistenceLayer.ts +++ b/src/store/persistenceLayer.ts @@ -1,5 +1,6 @@ import { openDB } from 'idb'; import { createLogger } from '@/lib/logging'; +import { PERSISTED_STATE_RETENTION_MS } from '@/constants/app.constants'; const logger = createLogger('persistence-layer'); @@ -41,6 +42,8 @@ export const persistenceLayer = { }, }); await db.put(STORE_NAME, JSON.parse(value), name); + // Stamped on every write so retention has an age to work from. + if (!isMetaKey(name)) await touchPersistedEntry(name); } catch (error) { logger.error('[Persistence] Error saving state', { error }); } @@ -51,8 +54,17 @@ export const persistenceLayer = { */ async removeItem(name: string): Promise { if (typeof window === 'undefined') return; - const db = await openDB(DB_NAME, 1); + // The upgrade callback matters here as much as in the read and write + // paths: without it, opening a database that does not exist yet creates + // one with no object store, and the delete throws NotFoundError. + const db = await openDB(DB_NAME, 1, { + upgrade(db) { + db.createObjectStore(STORE_NAME); + }, + }); await db.delete(STORE_NAME, name); + // Metadata for a deleted entry would otherwise linger forever. + await db.delete(STORE_NAME, `${PERSISTENCE_META_PREFIX}${name}`); }, /** @@ -146,3 +158,131 @@ export function persistedStateVersion(raw: string | null): number | undefined { return undefined; } } + +// --------------------------------------------------------------------------- +// Retention / GC for persisted slices +// --------------------------------------------------------------------------- + +/** + * Key prefix under which write timestamps are recorded. + * + * Metadata lives in the same object store as the data rather than in a new + * one, because adding a store means bumping `DB_NAME`'s version and running an + * upgrade against every existing browser — a lot of risk for a timestamp. + */ +export const PERSISTENCE_META_PREFIX = '__meta__:'; + +/** Write metadata recorded alongside each persisted entry. */ +export interface PersistedEntryMeta { + updatedAt: number; +} + +const metaKey = (name: string) => `${PERSISTENCE_META_PREFIX}${name}`; + +/** True for the metadata companion of a persisted entry, not an entry itself. */ +export function isMetaKey(key: unknown): boolean { + return typeof key === 'string' && key.startsWith(PERSISTENCE_META_PREFIX); +} + +/** + * Picks the entries whose last write is older than `maxAgeMs`. + * + * Pure, so retention can be tested without a database. An entry with no + * recorded timestamp is **kept**: it predates this metadata and deleting it + * would silently drop a user's state on upgrade. + */ +export function selectExpiredKeys( + entries: ReadonlyArray<{ key: string; updatedAt?: number }>, + maxAgeMs: number, + now: number = Date.now(), +): string[] { + return entries + .filter((entry) => typeof entry.updatedAt === 'number' && now - entry.updatedAt > maxAgeMs) + .map((entry) => entry.key); +} + +/** Records the write timestamp for `name`. */ +export async function touchPersistedEntry( + name: string, + now: number = Date.now(), +): Promise { + if (typeof window === 'undefined') return; + try { + const db = await openDB(DB_NAME, 1, { + upgrade(db) { + db.createObjectStore(STORE_NAME); + }, + }); + await db.put(STORE_NAME, { updatedAt: now } satisfies PersistedEntryMeta, metaKey(name)); + } catch (error) { + logger.error('[Persistence] Error recording entry metadata', { error }); + } +} + +/** The last write timestamp for `name`, or null when none was recorded. */ +export async function getPersistedEntryUpdatedAt(name: string): Promise { + if (typeof window === 'undefined') return null; + try { + const db = await openDB(DB_NAME, 1, { + upgrade(db) { + db.createObjectStore(STORE_NAME); + }, + }); + const meta = (await db.get(STORE_NAME, metaKey(name))) as PersistedEntryMeta | undefined; + return typeof meta?.updatedAt === 'number' ? meta.updatedAt : null; + } catch (error) { + logger.error('[Persistence] Error reading entry metadata', { error }); + return null; + } +} + +/** + * Deletes persisted entries not written within `maxAgeMs`, and their metadata. + * + * Returns the keys removed. Entries written before metadata existed have no + * timestamp and are left alone — see [`selectExpiredKeys`]. + */ +export async function purgeExpiredPersistedEntries( + maxAgeMs: number = PERSISTED_STATE_RETENTION_MS, + now: number = Date.now(), +): Promise { + if (typeof window === 'undefined') return []; + try { + const db = await openDB(DB_NAME, 1, { + upgrade(db) { + db.createObjectStore(STORE_NAME); + }, + }); + + const keys = (await db.getAllKeys(STORE_NAME)).filter( + (key): key is string => typeof key === 'string', + ); + const dataKeys = keys.filter((key) => !isMetaKey(key)); + + const entries = await Promise.all( + dataKeys.map(async (key) => { + const meta = (await db.get(STORE_NAME, metaKey(key))) as PersistedEntryMeta | undefined; + return { key, updatedAt: meta?.updatedAt }; + }), + ); + + const expired = selectExpiredKeys(entries, maxAgeMs, now); + for (const key of expired) { + await db.delete(STORE_NAME, key); + await db.delete(STORE_NAME, metaKey(key)); + } + + // Metadata whose entry is gone is dead weight; drop it in the same pass. + const orphanedMeta = keys.filter( + (key) => isMetaKey(key) && !dataKeys.includes(key.slice(PERSISTENCE_META_PREFIX.length)), + ); + for (const key of orphanedMeta) { + await db.delete(STORE_NAME, key); + } + + return expired; + } catch (error) { + logger.error('[Persistence] Error purging expired state', { error }); + return []; + } +} diff --git a/src/store/selectors.ts b/src/store/selectors.ts index 9045cd83..c027d08d 100644 --- a/src/store/selectors.ts +++ b/src/store/selectors.ts @@ -2,41 +2,73 @@ * Centralised store selectors. * Import from here instead of accessing store state inline to keep * components decoupled from store internals. + * + * Selectors that derive a new object or array are memoized. Zustand compares a + * selector's result with `Object.is` to decide whether to re-render, so a + * selector returning a fresh `filter(...)` result re-renders every subscriber + * on every unrelated store update — and recomputes the filter each time. + * Memoizing by input identity returns the previous reference when the inputs + * are unchanged, and `useShallow` covers the small object literals where a + * per-field comparison is cheaper than caching. */ +import { useShallow } from 'zustand/react/shallow'; import { useSearchStore } from '@/app/store/searchStore'; import { useNotificationStore } from '@/app/store/notificationStore'; import { useQuizStore } from '@/app/store/quizStore'; +import { memoizeByInputs } from './stateManager'; +import type { AppNotification } from '@/lib/notifications/types'; -// ── Search selectors ────────────────────────────────────────────────────────── +// ── Search selectors ──────────────────────────────────────────────────────── export const useSearchFilters = () => - useSearchStore((s) => ({ - difficulty: s.difficulty, - duration: s.duration, - topics: s.topics, - instructors: s.instructors, - sortBy: s.sortBy, - price: s.price, - })); + useSearchStore( + useShallow((s) => ({ + difficulty: s.difficulty, + duration: s.duration, + topics: s.topics, + instructors: s.instructors, + sortBy: s.sortBy, + price: s.price, + })), + ); export const useSearchHistory = () => useSearchStore((s) => s.searchHistory); -// ── Notification selectors ──────────────────────────────────────────────────── +// ── Notification selectors ────────────────────────────────────────────────── + +/** + * Unread notifications, cached against the notifications array reference. + * + * The store keeps up to 200 notifications and rewrites the whole array on + * every mutation, so the filter is worth skipping when nothing has changed. + */ +export const selectUnreadNotifications = memoizeByInputs( + (notifications: readonly AppNotification[]) => notifications.filter((n) => !n.read), +); + export const useUnreadNotifications = () => - useNotificationStore((s) => s.notifications.filter((n) => !n.read)); + useNotificationStore((s) => selectUnreadNotifications(s.notifications)); +/** + * Count of unread notifications. + * + * Derived from the same memoized array rather than filtering a second time, + * so a component showing both the badge and the list costs one pass. + */ export const useUnreadCount = () => - useNotificationStore((s) => s.notifications.filter((n) => !n.read).length); + useNotificationStore((s) => selectUnreadNotifications(s.notifications).length); -// ── Quiz selectors ──────────────────────────────────────────────────────────── +// ── Quiz selectors ────────────────────────────────────────────────────────── export const useCurrentQuestion = () => useQuizStore((s) => s.currentQuiz ? s.currentQuiz.questions[s.currentQuestionIndex] ?? null : null, ); export const useQuizProgress = () => - useQuizStore((s) => ({ - current: s.currentQuestionIndex + 1, - total: s.currentQuiz?.questions.length ?? 0, - isReviewMode: s.isReviewMode, - })); + useQuizStore( + useShallow((s) => ({ + current: s.currentQuestionIndex + 1, + total: s.currentQuiz?.questions.length ?? 0, + isReviewMode: s.isReviewMode, + })), + ); diff --git a/src/store/stateManager.ts b/src/store/stateManager.ts index de06fa41..7e7a3748 100644 --- a/src/store/stateManager.ts +++ b/src/store/stateManager.ts @@ -198,3 +198,68 @@ export const useStore = create()( ), ), ); + +// --------------------------------------------------------------------------- +// Selector memoization +// --------------------------------------------------------------------------- + +/** A memoized selector, with a way to drop its cached result. */ +export type MemoizedSelector = ((...args: TArgs) => R) & { + /** Forgets the cached inputs and result. */ + clear: () => void; +}; + +/** + * Memoizes a derivation by the identity of its inputs. + * + * Zustand compares a selector's *result* with `Object.is` to decide whether to + * re-render. A selector that derives a new array or object — `notifications + * .filter(...)` — therefore returns a fresh reference on every store update, + * so every subscriber re-renders whenever any unrelated slice changes, and the + * filter runs again each time. + * + * Caching the last inputs and result fixes both: given the same input + * references the computation is skipped and the previous reference is + * returned, so `Object.is` holds and the component does not re-render. + * + * Only the most recent call is cached. That is the right size here: a selector + * is called with the current state, and state moves forward, so a larger cache + * would retain memory to serve inputs that are not coming back. + * + * Arguments are compared with `Object.is`, which makes the memo as cheap as + * the comparison — it is not a deep equality check, so a caller that rebuilds + * an equal-but-distinct input on every call gains nothing. + */ +export function memoizeByInputs( + compute: (...args: TArgs) => R, +): MemoizedSelector { + let lastArgs: TArgs | null = null; + let lastResult!: R; + let hasResult = false; + + const memoized = ((...args: TArgs): R => { + if (hasResult && lastArgs !== null && sameArgs(lastArgs, args)) { + return lastResult; + } + + lastArgs = args; + lastResult = compute(...args); + hasResult = true; + return lastResult; + }) as MemoizedSelector; + + memoized.clear = () => { + lastArgs = null; + hasResult = false; + }; + + return memoized; +} + +function sameArgs(a: readonly unknown[], b: readonly unknown[]): boolean { + if (a.length !== b.length) return false; + for (let index = 0; index < a.length; index += 1) { + if (!Object.is(a[index], b[index])) return false; + } + return true; +}