From abae207d022648b655439dde5be1753212ad5afe Mon Sep 17 00:00:00 2001 From: 4gray Date: Mon, 12 Oct 2026 07:34:28 +0200 Subject: [PATCH] fix(server-storage): unsent writes survive reloads, guarded move, lazy storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - User-data writes the server has not confirmed are kept under iptvnator-server:unsent-writes and sent first on the next visit. - Moving browser data does not start while user-data writes are unsent, and a duplicate of a failed upload counts as failed, not merged. - The session reaches localStorage through the lazy adapter, so a browser that denies storage still boots. - Raise launch/renderer.domMutationsToFirstCard by 2 (569 → 571): the rail server shortcut and the offline banner each leave an @if anchor in every build. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/pwa-self-hosted.md | 6 +- .../server-migration.service.spec.ts | 45 ++++++++++- .../server-migration.service.ts | 13 ++- .../server-storage/server-storage-session.ts | 4 +- .../server-storage/server-unsent-writes.ts | 74 +++++++++++++++++ .../server-user-data-entries.ts | 35 ++++++++ .../server-user-data-storage.spec.ts | 46 +++++++++++ .../server-user-data-storage.ts | 79 ++++++++++--------- tools/performance/journey-baselines.json | 11 +-- 9 files changed, 264 insertions(+), 49 deletions(-) create mode 100644 libs/services/src/lib/server-storage/server-unsent-writes.ts create mode 100644 libs/services/src/lib/server-storage/server-user-data-entries.ts diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index bb7c66294..3bf545314 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -200,7 +200,8 @@ settings and guards read them in every build. each change sends only the playlist entries that differ, debounced, with a `keepalive` flush when the page hides. Batches go out one after another (the page-hide flush waits for a batch still travelling), a failed write, - including a refused `keepalive` one, stays queued for the next flush, and + including a refused `keepalive` one, stays queued for the next flush and + survives a reload under `iptvnator-server:unsent-writes`, and re-reads run one at a time and keep entries edited while they were in flight or not yet saved. A re-read whose result does not fit the cache keeps the old cache and baseline together. A response body cut off after @@ -237,7 +238,8 @@ settings and guards read them in every build. translated through the URL; nothing is dropped), and the legacy Xtream maps follow under the server id; `combineUserData` unions lists (playback positions by content type and Xtream id, renamed to the server playlist) and merges - maps, the server's value winning on a conflict. Browser data is never deleted, and + maps, the server's value winning on a conflict. Browser data is never + deleted, the move does not start while user-data writes are unsent, and the marker is written only when nothing failed, so Settings → Server keeps offering a partial move. diff --git a/libs/services/src/lib/server-storage/server-migration.service.spec.ts b/libs/services/src/lib/server-storage/server-migration.service.spec.ts index b8c036103..f5673edb5 100644 --- a/libs/services/src/lib/server-storage/server-migration.service.spec.ts +++ b/libs/services/src/lib/server-storage/server-migration.service.spec.ts @@ -23,7 +23,11 @@ describe('ServerMigrationService', () => { dbUpsertAppPlaylist: jest.Mock; }; api: { getJson: jest.Mock; sendJson: jest.Mock }; - userData: { flush: jest.Mock; hydrate: jest.Mock }; + userData: { + flush: jest.Mock; + hasUnsentWrites: jest.Mock; + hydrate: jest.Mock; + }; }; beforeEach(() => { @@ -42,6 +46,7 @@ describe('ServerMigrationService', () => { }, userData: { flush: jest.fn().mockResolvedValue(undefined), + hasUnsentWrites: jest.fn(() => false), hydrate: jest.fn().mockResolvedValue(undefined), }, }; @@ -265,4 +270,42 @@ describe('ServerMigrationService', () => { }); expect(migration.browserDataMoved()).toBe(true); }); + + it('does not start while user-data writes are still unsent', async () => { + session.userData.hasUnsentWrites.mockReturnValue(true); + const migration = TestBed.inject(ServerMigrationService); + + await expect(migration.moveBrowserData()).rejects.toThrow('Unsent'); + expect( + session.documentStore.dbUpsertAppPlaylist + ).not.toHaveBeenCalled(); + expect(session.api.sendJson).not.toHaveBeenCalled(); + expect(migration.browserDataMoved()).toBe(false); + }); + + it('counts a duplicate of a failed upload as failed, not merged', async () => { + TestBed.overrideProvider(NgxIndexedDBService, { + useValue: { + getAll: () => + of([ + browserPlaylists[1], + { ...browserPlaylists[1], _id: 'x2' }, + ]), + }, + }); + session.documentStore.dbUpsertAppPlaylist.mockRejectedValue( + new Error('500') + ); + localStorage.setItem( + 'xtream-favorites', + JSON.stringify({ x: ['live:1'], x2: ['live:2'] }) + ); + + const result = await TestBed.inject( + ServerMigrationService + ).moveBrowserData(); + + expect(result).toEqual({ imported: 0, merged: 0, failed: 2 }); + expect(session.api.sendJson).not.toHaveBeenCalled(); + }); }); diff --git a/libs/services/src/lib/server-storage/server-migration.service.ts b/libs/services/src/lib/server-storage/server-migration.service.ts index b47a1be2a..33fa27a4f 100644 --- a/libs/services/src/lib/server-storage/server-migration.service.ts +++ b/libs/services/src/lib/server-storage/server-migration.service.ts @@ -82,6 +82,11 @@ export class ServerMigrationService { } async moveBrowserData(): Promise { + // An edit still unsent could land after the combined values below. + await this.session.userData.flush(); + if (this.session.userData.hasUnsentWrites()) { + throw new Error('Unsent user data; try again when online'); + } const [browser, server] = await Promise.all([ this.readBrowserPlaylists(), this.session.documentStore.dbGetAppPlaylistMetas(), @@ -95,9 +100,14 @@ export class ServerMigrationService { let merged = 0; let failed = 0; + const onServer = new Set(server.map((playlist) => playlist._id)); for (const step of planServerMigration(browser, server)) { try { if (step.kind === 'merge') { + // A duplicate of a browser playlist whose upload failed. + if (!onServer.has(step.serverId)) { + throw new Error('Merge target was not uploaded'); + } const playlist = byId.get(step.browserId); if (playlist) { await this.mergeDocumentUserData( @@ -112,6 +122,7 @@ export class ServerMigrationService { step.playlist ); imported += 1; + onServer.add(step.playlist._id); targets.set(step.playlist._id, step.playlist._id); } } catch { @@ -119,8 +130,6 @@ export class ServerMigrationService { } } - // Queued edits go out first, so none lands after the combined values. - await this.session.userData.flush(); failed += await this.copyXtreamUserData(targets); await this.session.userData.hydrate(); if (failed === 0) { diff --git a/libs/services/src/lib/server-storage/server-storage-session.ts b/libs/services/src/lib/server-storage/server-storage-session.ts index b9bae97c4..89ebed52d 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.ts @@ -12,6 +12,7 @@ import { SERVER_SIGN_IN_PATH, ServerStorageStatusService, } from '../server-storage-status.service'; +import { createBrowserUserDataStorage } from '../user-data-storage'; import { ServerApiClient, type ServerApiError } from './server-api-client'; import { ServerPlaylistDocumentStore } from './server-playlist-document-store'; import { SERVER_STORAGE_CONFIG } from './server-storage.config'; @@ -39,7 +40,8 @@ export class ServerStorageSession { readonly documentStore = new ServerPlaylistDocumentStore(this.api); readonly userData = new ServerUserDataStorage( this.api, - globalThis.localStorage, + // Lazy: a browser that denies storage must not stop the app here. + createBrowserUserDataStorage(), () => this.announceWriteFailure() ); diff --git a/libs/services/src/lib/server-storage/server-unsent-writes.ts b/libs/services/src/lib/server-storage/server-unsent-writes.ts new file mode 100644 index 000000000..da1779d88 --- /dev/null +++ b/libs/services/src/lib/server-storage/server-unsent-writes.ts @@ -0,0 +1,74 @@ +import { + isServerUserStateNamespace, + type ServerUserStateNamespace, +} from '@iptvnator/shared/interfaces'; +import type { UserDataStorage } from '../user-data-storage'; + +/** + * User-data writes the server has not confirmed, kept across reloads: a + * failed write, or a page-hide write whose answer the page did not live to + * see, is sent again on the next visit instead of being replaced by the + * server's older value. + */ +export const UNSENT_WRITES_KEY = 'iptvnator-server:unsent-writes'; + +export interface UnsentWrite { + readonly namespace: ServerUserStateNamespace; + readonly playlistId: string; + /** `undefined` deletes the entry. */ + readonly value: unknown; +} + +interface StoredWrite { + readonly namespace: string; + readonly playlistId: string; + readonly value?: unknown; + readonly remove?: true; +} + +export function readUnsentWrites(local: UserDataStorage): UnsentWrite[] { + let stored: unknown; + try { + stored = JSON.parse(local.getItem(UNSENT_WRITES_KEY) ?? '[]'); + } catch { + return []; + } + const writes: UnsentWrite[] = []; + for (const write of Array.isArray(stored) + ? (stored as StoredWrite[]) + : []) { + if ( + write && + isServerUserStateNamespace(write.namespace) && + typeof write.playlistId === 'string' + ) { + writes.push({ + namespace: write.namespace, + playlistId: write.playlistId, + value: write.remove ? undefined : write.value, + }); + } + } + return writes; +} + +export function saveUnsentWrites( + local: UserDataStorage, + writes: readonly UnsentWrite[] +): void { + try { + if (writes.length === 0) { + local.removeItem(UNSENT_WRITES_KEY); + return; + } + const stored: StoredWrite[] = writes.map( + ({ namespace, playlistId, value }) => + value === undefined + ? { namespace, playlistId, remove: true } + : { namespace, playlistId, value } + ); + local.setItem(UNSENT_WRITES_KEY, JSON.stringify(stored)); + } catch { + // Storage unavailable or full: the queue stays in memory only. + } +} diff --git a/libs/services/src/lib/server-storage/server-user-data-entries.ts b/libs/services/src/lib/server-storage/server-user-data-entries.ts new file mode 100644 index 000000000..84ca4fd67 --- /dev/null +++ b/libs/services/src/lib/server-storage/server-user-data-entries.ts @@ -0,0 +1,35 @@ +import type { ServerUserStateNamespace } from '@iptvnator/shared/interfaces'; + +/** One playlist entry of a synced user-data map, on its way to the server. */ +export interface PendingEntry { + readonly namespace: ServerUserStateNamespace; + readonly playlistId: string; + /** `undefined` deletes the entry. */ + readonly value: unknown; + /** The edit this value came from; a retry never replaces a newer one. */ + readonly edit: number; +} + +export function entryKey(entry: PendingEntry): string { + return `${entry.namespace}|${entry.playlistId}`; +} + +export function splitEditKey(key: string): [string, string] { + const separator = key.indexOf('|'); + return [key.slice(0, separator), key.slice(separator + 1)]; +} + +export function entryPath(entry: PendingEntry): string { + return `/user-state/${entry.namespace}/${encodeURIComponent(entry.playlistId)}`; +} + +export function parseMap(value: string): Record { + try { + const parsed: unknown = JSON.parse(value); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) + ? (parsed as Record) + : {}; + } catch { + return {}; + } +} diff --git a/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts b/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts index 1fcd8d149..8acf02147 100644 --- a/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts @@ -406,4 +406,50 @@ describe('ServerUserDataStorage', () => { { value: ['live:1', 'live:3'] } ); }); + + it('keeps unconfirmed writes across a reload and sends them first', async () => { + const first = setup(); + await first.storage.hydrate(); + first.api.sendJson.mockRejectedValue(new Error('offline')); + first.storage.setItem( + 'xtream-favorites', + JSON.stringify({ p1: ['live:1', 'live:9'] }) + ); + await first.storage.flush(); + expect(first.storage.hasUnsentWrites()).toBe(true); + expect(first.local.values.get('iptvnator-server:unsent-writes')).toBe( + JSON.stringify([ + { + namespace: 'xtream-favorites', + playlistId: 'p1', + value: ['live:1', 'live:9'], + }, + ]) + ); + + // Reload: a new instance over the same browser storage. + const api = { + ...first.api, + sendJson: jest.fn().mockResolvedValue(undefined), + getJson: jest.fn().mockResolvedValue({ + entries: { 'xtream-favorites': { p1: ['live:1', 'live:9'] } }, + }), + }; + const reloaded = new ServerUserDataStorage( + api as never, + first.local, + jest.fn() + ); + await reloaded.hydrate(); + + expect(api.sendJson).toHaveBeenCalledWith( + 'PUT', + '/user-state/xtream-favorites/p1', + { value: ['live:1', 'live:9'] } + ); + expect(reloaded.hasUnsentWrites()).toBe(false); + expect(first.local.values.has('iptvnator-server:unsent-writes')).toBe( + false + ); + }); }); diff --git a/libs/services/src/lib/server-storage/server-user-data-storage.ts b/libs/services/src/lib/server-storage/server-user-data-storage.ts index 3244e9180..f3783f26a 100644 --- a/libs/services/src/lib/server-storage/server-user-data-storage.ts +++ b/libs/services/src/lib/server-storage/server-user-data-storage.ts @@ -6,20 +6,19 @@ import { } from '@iptvnator/shared/interfaces'; import type { UserDataStorage } from '../user-data-storage'; import type { ServerApiClient } from './server-api-client'; +import { + entryKey, + entryPath, + parseMap, + type PendingEntry, + splitEditKey, +} from './server-user-data-entries'; +import { readUnsentWrites, saveUnsentWrites } from './server-unsent-writes'; /** Server-mode cache keys; the legacy unprefixed keys are never touched. */ export const SERVER_CACHE_PREFIX = 'iptvnator-server:'; const FLUSH_DELAY_MS = 500; -interface PendingEntry { - readonly namespace: ServerUserStateNamespace; - readonly playlistId: string; - /** `undefined` deletes the entry. */ - readonly value: unknown; - /** The edit this value came from; a retry never replaces a newer one. */ - readonly edit: number; -} - type ServerApi = Pick< ServerApiClient, 'getJson' | 'sendJson' | 'request' | 'sendKeepalive' @@ -42,7 +41,8 @@ export class ServerUserDataStorage implements UserDataStorage { private sending: Promise = Promise.resolve(); private writesInFlight = 0; /** Page-hide writes still travelling, by `namespace|playlistId`. */ - private readonly keepaliveKeys = new Set(); + private readonly keepalive = new Map(); + private unsentSaved = false; /** Re-reads run one at a time, so an older one never lands last. */ private hydration: Promise = Promise.resolve(); /** Edit counter per `namespace|playlistId`, for hydrate to keep them. */ @@ -53,7 +53,19 @@ export class ServerUserDataStorage implements UserDataStorage { private readonly api: ServerApi, private readonly local: UserDataStorage, private readonly onWriteFailure: (error: unknown) => void - ) {} + ) { + // Writes a previous visit could not confirm go out with the first + // flush, and the first read keeps them. + for (const write of readUnsentWrites(local)) { + this.queue(write.namespace, write.playlistId, write.value); + } + this.unsentSaved = this.pending.size > 0; + } + + /** Whether a write is queued, being retried, or still travelling. */ + hasUnsentWrites(): boolean { + return this.pending.size > 0 || this.keepalive.size > 0; + } getItem(key: string): string | null { return this.isSynced(key) @@ -110,7 +122,7 @@ export class ServerUserDataStorage implements UserDataStorage { ); const unsaved = new Set([ ...this.pending.keys(), - ...this.keepaliveKeys, + ...this.keepalive.keys(), ]); for (const [key, edit] of this.edits) { if (edit > editsBefore) { @@ -144,7 +156,7 @@ export class ServerUserDataStorage implements UserDataStorage { if ( edit <= editsBefore && !this.pending.has(key) && - !this.keepaliveKeys.has(key) + !this.keepalive.has(key) ) { this.edits.delete(key); } @@ -192,6 +204,10 @@ export class ServerUserDataStorage implements UserDataStorage { this.writesInFlight -= 1; } }; + for (const entry of entries) { + this.keepalive.set(entryKey(entry), entry); + } + this.saveUnsent(); // Idle: send synchronously, while the hiding page still runs. const run = this.writesInFlight === 0 ? sendAll() : this.sending.then(sendAll); @@ -288,16 +304,15 @@ export class ServerUserDataStorage implements UserDataStorage { } private async sendKeepalive(entry: PendingEntry): Promise { - const key = entryKey(entry); const path = entryPath(entry); - this.keepaliveKeys.add(key); const ok = await (entry.value === undefined ? this.api.sendKeepalive('DELETE', path) : this.api.sendKeepalive('PUT', path, { value: entry.value })); - this.keepaliveKeys.delete(key); + this.keepalive.delete(entryKey(entry)); if (!ok) { this.requeue(entry); } + this.saveUnsent(); } private async send(entry: PendingEntry): Promise { @@ -309,33 +324,21 @@ export class ServerUserDataStorage implements UserDataStorage { value: entry.value, }); } + this.saveUnsent(); } catch (error) { this.requeue(entry); + this.saveUnsent(); this.onWriteFailure(error); } } -} -function entryKey(entry: PendingEntry): string { - return `${entry.namespace}|${entry.playlistId}`; -} - -function splitEditKey(key: string): [string, string] { - const separator = key.indexOf('|'); - return [key.slice(0, separator), key.slice(separator + 1)]; -} - -function entryPath(entry: PendingEntry): string { - return `/user-state/${entry.namespace}/${encodeURIComponent(entry.playlistId)}`; -} - -function parseMap(value: string): Record { - try { - const parsed: unknown = JSON.parse(value); - return parsed && typeof parsed === 'object' && !Array.isArray(parsed) - ? (parsed as Record) - : {}; - } catch { - return {}; + /** Persists what the server has not confirmed; skips idle rewrites. */ + private saveUnsent(): void { + const writes = [...this.pending.values(), ...this.keepalive.values()]; + if (writes.length === 0 && !this.unsentSaved) { + return; + } + saveUnsentWrites(this.local, writes); + this.unsentSaved = writes.length > 0; } } diff --git a/tools/performance/journey-baselines.json b/tools/performance/journey-baselines.json index 118d8ea81..ea01df02e 100644 --- a/tools/performance/journey-baselines.json +++ b/tools/performance/journey-baselines.json @@ -20,13 +20,14 @@ "measuredWith": "pnpm run perf:journeys (ubuntu-latest, xvfb), identical in 3 runs" }, "renderer.domMutationsToFirstCard": { - "value": 569, + "value": 571, "unit": "mutations", "slack": 0, - "updatedAt": "2026-10-10", - "evidencePr": 1871, - "evidenceRun": "https://github.com/4gray/iptvnator/actions/runs/38045444230", - "measuredWith": "pnpm run perf:journeys (ubuntu-latest, xvfb), identical in 3 runs" + "updatedAt": "2026-10-12", + "evidencePr": 1903, + "evidenceRun": "https://github.com/4gray/iptvnator/actions/runs/38206088310", + "measuredWith": "pnpm run perf:journeys (ubuntu-latest, xvfb), CI run on PR 1903", + "note": "Raised by 2 for self-hosted server storage: the rail server shortcut and the workspace offline banner each leave one @if anchor comment in every build, also when server storage is inactive." }, "main.modulesRegisteredBeforeWindow": { "value": 2,