From 6f1fe7929c486ec6ff20c7f80a066e624d67f7c0 Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 14 Oct 2026 00:24:13 +0200 Subject: [PATCH] fix(server-storage): recheck the instance after offline, back-forward cache pages, Stalker server rows Going offline now marks the instance unchecked: the token and writes wait until /instance answers with this page's instance (another one reloads the page). A replaced volume at the same address no longer gets the old token and queued user-state writes on reconnect. The session checks as soon as it goes offline, so a blip costs no retry interval. A page restored from the back-forward cache whose record another tab adopted meanwhile (its timers were frozen, so the record went stale) reloads instead of sending those writes again after newer edits. The Stalker playlist metadata migration also runs on server rows, once per server: a desktop database copied to the server can predate it, and its Stalker rows lacked isFullStalkerPortal. The take-over and change-diff loops move out of the user-data facade, and the adoption specs get their own file. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/pwa-self-hosted.md | 13 +- .../src/lib/playlists.service.spec.ts | 167 ++++++++++++++---- libs/services/src/lib/playlists.service.ts | 34 +++- .../src/lib/server-storage-status.service.ts | 11 +- .../server-storage/server-api-client.spec.ts | 2 +- .../lib/server-storage/server-api-client.ts | 12 +- .../server-storage-session.spec.ts | 86 +++++++++ .../server-storage/server-storage-session.ts | 33 +++- .../server-storage/server-unsent-writes.ts | 32 +++- .../server-user-data-adopt.spec.ts | 159 +++++++++++++++++ .../server-storage/server-user-data-adopt.ts | 31 ++++ .../server-storage/server-user-data-edit.ts | 19 ++ .../server-user-data-storage.ts | 70 ++++---- .../server-user-data-sync.spec.ts | 103 ----------- 14 files changed, 569 insertions(+), 203 deletions(-) create mode 100644 libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 08abfc313..5e3d4d590 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -234,7 +234,9 @@ settings and guards read them in every build. tab remembers its page load in `sessionStorage`, so a reload takes its writes over and sends them first (the old copy goes only once the new one is saved, at the latest with this page's next saved copy). A page marks - its record closed on `pagehide` (not into the back-forward cache), and + its record closed on `pagehide` (not into the back-forward cache, where + its record goes stale; restored after another tab adopted it, the page + reloads rather than send those writes again), and only a closed, stale or missing record is taken over: a duplicated tab, which copies `sessionStorage`, leaves the running tab its writes. A page load that still looked live (or whose close flag storage refused) is @@ -328,7 +330,10 @@ settings and guards read them in every build. therefore remembers the server along with the token, or reports storage trouble. A page started from memory first checks that the instance at the address is the remembered one (`ensureVerified`), and reloads if not, - before sign-in sends the token. Both turn + before sign-in sends the token. So does a page that went offline before + its next read: until then it sends neither the token nor writes (a + replaced volume at the same address is another database), and it checks + as soon as it goes offline, so a blip costs no retry interval. Both turn the guard's 401 (`isServerAccessChallenge`) into `reportAccessDenied()`, as the storage API client does with any 401, so a token changed on the server sends an open page back to sign-in; a late 401 for a token @@ -337,7 +342,9 @@ settings and guards read them in every build. - **Moving browser data.** `ServerMigrationService` offers, once per server instance (marker `iptvnator-server:migration:`), to move the playlists this browser kept in IndexedDB (`planServerMigration`), Stalker - rows from before the portal-mode flag normalized first. A + rows from before the portal-mode flag normalized first (server rows get + the same Stalker metadata migration once per server, as a desktop + database copied there can predate it). A playlist the server already has, by id or by source (Xtream server and user, Stalker portal and MAC, M3U URL), merges into the server's copy; the others upload with their ids. A merged playlist gains the browser diff --git a/libs/services/src/lib/playlists.service.spec.ts b/libs/services/src/lib/playlists.service.spec.ts index f9e395f9a..d5eb14db4 100644 --- a/libs/services/src/lib/playlists.service.spec.ts +++ b/libs/services/src/lib/playlists.service.spec.ts @@ -74,7 +74,10 @@ describe('PlaylistsService', () => { } it('announces a committed SQLite connection edit', async () => { - const playlist = { _id: 'source', url: 'https://old.test/list' } as Playlist; + const playlist = { + _id: 'source', + url: 'https://old.test/list', + } as Playlist; testWindow.electron = { dbGetAppState: jest.fn().mockResolvedValue('1'), dbSetAppState: jest.fn(), @@ -85,8 +88,22 @@ describe('PlaylistsService', () => { const service = createService(); const next = jest.fn(); Object.assign(service, { healthEvidence: { connections: { next } } }); - await firstValueFrom(service.updatePlaylistMeta({ ...playlist, title: 'Source', count: 0, importDate: '', autoRefresh: false, serverUrl: 'https://new.test' })); - expect(next).toHaveBeenCalledWith({ id: 'source', playlist: expect.objectContaining({ serverUrl: 'https://new.test' }) }); + await firstValueFrom( + service.updatePlaylistMeta({ + ...playlist, + title: 'Source', + count: 0, + importDate: '', + autoRefresh: false, + serverUrl: 'https://new.test', + }) + ); + expect(next).toHaveBeenCalledWith({ + id: 'source', + playlist: expect.objectContaining({ + serverUrl: 'https://new.test', + }), + }); }); it('announces committed deletion but does not retire health after a failed write', async () => { @@ -96,9 +113,15 @@ describe('PlaylistsService', () => { await firstValueFrom(service.deletePlaylist('retired')); expect(next).toHaveBeenCalledWith({ id: 'retired' }); next.mockClear(); - const failed = createService({ delete: jest.fn(() => { throw new Error('write failed'); }) }); + const failed = createService({ + delete: jest.fn(() => { + throw new Error('write failed'); + }), + }); Object.assign(failed, { healthEvidence: { connections: { next } } }); - await expect(firstValueFrom(failed.deletePlaylist('kept'))).rejects.toThrow(); + await expect( + firstValueFrom(failed.deletePlaylist('kept')) + ).rejects.toThrow(); expect(next).not.toHaveBeenCalled(); }); @@ -555,6 +578,60 @@ describe('PlaylistsService', () => { ); }); + it('migrates legacy Stalker portal flags on the server too', async () => { + // A desktop database copied to the server, from before the flag. + let storedPlaylists: Playlist[] = [ + { + _id: 'stalker-1', + title: 'Copied Stalker', + count: 0, + importDate: new Date('2026-03-28T00:00:00.000Z').toISOString(), + lastUsage: new Date('2026-03-28T00:00:00.000Z').toISOString(), + autoRefresh: false, + macAddress: '00:1A:79:AA:BB:CC', + portalUrl: 'http://example.com/stalker_portal/c/', + } as Playlist, + ]; + const appState = new Map(); + const server = { + dbGetAppPlaylists: jest.fn(async () => storedPlaylists), + dbGetAppState: jest.fn( + async (key: string) => appState.get(key) ?? null + ), + dbSetAppState: jest.fn(async (key: string, value: string) => { + appState.set(key, value); + }), + dbUpsertAppPlaylist: jest.fn(), + dbUpsertAppPlaylists: jest.fn(async (playlists: Playlist[]) => { + storedPlaylists = playlists; + }), + dbRecoverLegacyPlaylists: jest.fn(), + }; + testWindow.electron = undefined; + const service = createService(); + Object.assign(service as object, { + storageBackend: { + kind: () => 'server', + documentStore: () => server, + }, + }); + + await expect( + firstValueFrom(service.getAllPlaylists()) + ).resolves.toEqual([ + expect.objectContaining({ + _id: 'stalker-1', + isFullStalkerPortal: true, + }), + ]); + + expect(appState.get(STALKER_PLAYLIST_METADATA_MIGRATION_FLAG)).toBe( + '1' + ); + // The desktop-only steps stay with the desktop. + expect(server.dbRecoverLegacyPlaylists).not.toHaveBeenCalled(); + }); + it('does not rerun the SQLite Stalker metadata migration after the flag is set', async () => { const electron = { dbGetAppPlaylists: jest.fn(async () => []), @@ -1110,7 +1187,9 @@ describe('PlaylistsService', () => { ] as Playlist[]; const dbService = { getAll: jest.fn(() => of([])), - getByID: jest.fn((_store: string, id: string) => of(playlists.find((p) => p._id === id))), + getByID: jest.fn((_store: string, id: string) => + of(playlists.find((p) => p._id === id)) + ), update: jest.fn((_storeName: string, playlist: Playlist) => of(playlist) ), @@ -1143,19 +1222,32 @@ describe('PlaylistsService', () => { ); }); - it.each([false, true])('does not resurrect a deleted source when startup refresh finishes (SQLite=%s)', async (sqlite) => { - const upsert = jest.fn(); - testWindow.electron = sqlite ? { - dbGetAppState: jest.fn().mockResolvedValue('1'), dbSetAppState: jest.fn(), - dbGetAppPlaylists: jest.fn().mockResolvedValue([]), dbGetAppPlaylist: jest.fn().mockResolvedValue(null), - dbUpsertAppPlaylist: upsert, - } : undefined; - const update = jest.fn(() => of(undefined)); - const service = createService({ update }); - expect(await firstValueFrom(service.updateManyPlaylists([{ _id: 'deleted', autoRefresh: true } as Playlist]))).toEqual([]); - expect(update).not.toHaveBeenCalled(); - expect(upsert).not.toHaveBeenCalled(); - }); + it.each([false, true])( + 'does not resurrect a deleted source when startup refresh finishes (SQLite=%s)', + async (sqlite) => { + const upsert = jest.fn(); + testWindow.electron = sqlite + ? { + dbGetAppState: jest.fn().mockResolvedValue('1'), + dbSetAppState: jest.fn(), + dbGetAppPlaylists: jest.fn().mockResolvedValue([]), + dbGetAppPlaylist: jest.fn().mockResolvedValue(null), + dbUpsertAppPlaylist: upsert, + } + : undefined; + const update = jest.fn(() => of(undefined)); + const service = createService({ update }); + expect( + await firstValueFrom( + service.updateManyPlaylists([ + { _id: 'deleted', autoRefresh: true } as Playlist, + ]) + ) + ).toEqual([]); + expect(update).not.toHaveBeenCalled(); + expect(upsert).not.toHaveBeenCalled(); + } + ); it('short-circuits updateManyPlaylists when no playlists are provided', async () => { const dbService = { @@ -1905,19 +1997,30 @@ describe('PlaylistsService', () => { ); }); - it.each([false, true])('does not recreate a removed source on singular refresh (SQLite: %s)', async (sqlite) => { - const upsert = jest.fn(); - testWindow.electron = sqlite ? { - dbGetAllPlaylists: jest.fn().mockResolvedValue([]), - dbGetAppPlaylist: jest.fn().mockResolvedValue(null), - dbUpsertAppPlaylist: upsert, - } as unknown as typeof testWindow.electron : undefined; - const update = jest.fn(() => of(undefined)); - const service = createService({ update }); - await expect(firstValueFrom(service.updatePlaylist('deleted', { _id: 'deleted' } as Playlist))).rejects.toThrow('Playlist no longer exists'); - expect(update).not.toHaveBeenCalled(); - expect(upsert).not.toHaveBeenCalled(); - }); + it.each([false, true])( + 'does not recreate a removed source on singular refresh (SQLite: %s)', + async (sqlite) => { + const upsert = jest.fn(); + testWindow.electron = sqlite + ? ({ + dbGetAllPlaylists: jest.fn().mockResolvedValue([]), + dbGetAppPlaylist: jest.fn().mockResolvedValue(null), + dbUpsertAppPlaylist: upsert, + } as unknown as typeof testWindow.electron) + : undefined; + const update = jest.fn(() => of(undefined)); + const service = createService({ update }); + await expect( + firstValueFrom( + service.updatePlaylist('deleted', { + _id: 'deleted', + } as Playlist) + ) + ).rejects.toThrow('Playlist no longer exists'); + expect(update).not.toHaveBeenCalled(); + expect(upsert).not.toHaveBeenCalled(); + } + ); it('keeps hiddenGroupTitles when refreshing a playlist payload', async () => { const existingPlaylist: Playlist = { _id: 'playlist-2', diff --git a/libs/services/src/lib/playlists.service.ts b/libs/services/src/lib/playlists.service.ts index b9dc4733b..1cd0821e4 100644 --- a/libs/services/src/lib/playlists.service.ts +++ b/libs/services/src/lib/playlists.service.ts @@ -84,7 +84,9 @@ export function resolvePlaylistParser(parserModule: PlaylistParserModule) { providedIn: 'root', }) export class PlaylistsService { - private readonly healthEvidence = inject(SourceHealthEvidenceService, { optional: true }); + private readonly healthEvidence = inject(SourceHealthEvidenceService, { + optional: true, + }); private readonly dbService = inject(NgxIndexedDBService); private readonly databaseService = inject(DatabaseService); private readonly snackBar = inject(MatSnackBar); @@ -131,14 +133,18 @@ export class PlaylistsService { } private async ensureElectronPlaylistMigrations(): Promise { - if (!this.usesElectronStorage) { + if (!this.usesDocumentStore) { return; } if (!this.electronMigrationPromise) { this.electronMigrationPromise = (async () => { - await this.migrateIndexedDbPlaylistsToSqlite(); - await this.documentStore?.dbRecoverLegacyPlaylists?.(); + if (this.usesElectronStorage) { + await this.migrateIndexedDbPlaylistsToSqlite(); + await this.documentStore?.dbRecoverLegacyPlaylists?.(); + } + // Server rows too, once per server: a desktop database + // copied to the server can predate this migration. await this.migrateStalkerPlaylistMetadataInSqlite(); })().catch((error) => { this.electronMigrationPromise = null; @@ -375,7 +381,10 @@ export class PlaylistsService { } else { await store.dbUpsertAppPlaylist(playlist, operationId); } - this.healthEvidence?.connections.next({ id: playlist._id, playlist }); + this.healthEvidence?.connections.next({ + id: playlist._id, + playlist, + }); return playlist; }); } @@ -389,7 +398,12 @@ export class PlaylistsService { this.startupMetas = null; await store.dbUpsertAppPlaylists(playlists); - playlists.forEach((playlist) => this.healthEvidence?.connections.next({ id: playlist._id, playlist })); + playlists.forEach((playlist) => + this.healthEvidence?.connections.next({ + id: playlist._id, + playlist, + }) + ); return playlists; }); } @@ -565,7 +579,9 @@ export class PlaylistsService { ); return delete$.pipe( - tap(() => this.healthEvidence?.connections.next({ id: playlistId })), + tap(() => + this.healthEvidence?.connections.next({ id: playlistId }) + ), switchMap(() => from(this.runPlaylistDeleteCleanups(playlistId))), map((cleanupWarnings) => cleanupWarnings @@ -1032,7 +1048,9 @@ export class PlaylistsService { return nextPlaylist; }) ) - ).pipe(map((rows) => rows.filter((row): row is Playlist => row !== null))); + ).pipe( + map((rows) => rows.filter((row): row is Playlist => row !== null)) + ); } getFavoriteChannels(playlistId: string) { diff --git a/libs/services/src/lib/server-storage-status.service.ts b/libs/services/src/lib/server-storage-status.service.ts index e3d7e3784..b5893b08b 100644 --- a/libs/services/src/lib/server-storage-status.service.ts +++ b/libs/services/src/lib/server-storage-status.service.ts @@ -30,7 +30,8 @@ export class ServerStorageStatusService { /** * `verified` is false for a start from a remembered instance that did - * not answer: the address may lead to another server by now. + * not answer: the address may lead to another server by now. So it is + * after going offline, until the instance is checked again. */ activate( instance: ServerInstanceInfo, @@ -48,6 +49,11 @@ export class ServerStorageStatusService { this.verified = true; } + /** Whether the token and writes may go out (see `activate`). */ + isVerified(): boolean { + return this.verified; + } + /** * The stored token, only while server mode runs on an instance this * start verified: a token must never reach a server it was not given @@ -73,6 +79,9 @@ export class ServerStorageStatusService { reportOffline(): void { // A late failure after sign-out must not unlock the workspace. if (this.isActive() && this.stateSignal() !== 'unauthorized') { + // It may come back as another database (a replaced volume at + // the same address): no token, no writes until it is checked. + this.verified = false; this.stateSignal.set('offline'); } } diff --git a/libs/services/src/lib/server-storage/server-api-client.spec.ts b/libs/services/src/lib/server-storage/server-api-client.spec.ts index 135ebfe4d..976381b05 100644 --- a/libs/services/src/lib/server-storage/server-api-client.spec.ts +++ b/libs/services/src/lib/server-storage/server-api-client.spec.ts @@ -155,7 +155,7 @@ describe('ServerApiClient', () => { status, fetchFn: fetchFn as unknown as typeof fetch, accessToken: () => null, - holdWrites: true, + holdWrites: () => true, }); await expect( diff --git a/libs/services/src/lib/server-storage/server-api-client.ts b/libs/services/src/lib/server-storage/server-api-client.ts index a10ffb087..e30e093c1 100644 --- a/libs/services/src/lib/server-storage/server-api-client.ts +++ b/libs/services/src/lib/server-storage/server-api-client.ts @@ -44,11 +44,11 @@ export interface ServerApiClientOptions { readonly fetchFn?: typeof fetch; readonly timeoutMs?: number; /** - * Refuse writes as if offline: a start from a remembered instance that - * has not answered yet, whose address may lead to another database by - * the time it does. + * Refuse writes as if offline while true: an instance not checked yet + * (a start from memory, or one that went offline), whose address may + * lead to another database by the time it answers. */ - readonly holdWrites?: boolean; + readonly holdWrites?: () => boolean; /** The token to send (`ServerStorageStatusService.accessToken`). */ readonly accessToken: () => string | null; } @@ -74,7 +74,7 @@ export class ServerApiClient { const token = this.token(); let response: Response; try { - if (isWrite && this.options.holdWrites) { + if (isWrite && this.options.holdWrites?.()) { throw new Error('Writes wait for the instance to answer.'); } response = await this.fetchFn()(this.url(path), { @@ -181,7 +181,7 @@ export class ServerApiClient { body?: unknown, headers: Record = {} ): Promise { - if (this.options.holdWrites) { + if (this.options.holdWrites?.()) { return { status: 'failed' }; } try { diff --git a/libs/services/src/lib/server-storage/server-storage-session.spec.ts b/libs/services/src/lib/server-storage/server-storage-session.spec.ts index 792898de2..7ebba30b2 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.spec.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.spec.ts @@ -154,6 +154,92 @@ describe('ServerStorageSession', () => { expect(session.userData.hasUnsentWrites()).toBe(false); }); + it('reloads a page back from the back-forward cache whose writes another tab took', async () => { + const { session } = setup('none'); + const reload = jest + .spyOn(session as unknown as { reloadPage(): void }, 'reloadPage') + .mockImplementation(() => undefined); + await session.start(); + const resumed = jest + .spyOn(session.userData, 'resumed') + .mockReturnValue(true); + const pageshow = (persisted: boolean) => + window.dispatchEvent( + Object.assign(new Event('pageshow'), { persisted }) + ); + + pageshow(false); + pageshow(true); + expect(resumed).toHaveBeenCalledTimes(1); + expect(reload).not.toHaveBeenCalled(); + + resumed.mockReturnValue(false); + pageshow(true); + expect(reload).toHaveBeenCalled(); + }); + + describe('after losing the server', () => { + const answer = (body: unknown) => ({ + status: 200, + ok: true, + headers: new Headers(), + json: async () => body, + }); + + it('checks the instance before the token and writes go out again', async () => { + writeServerAccessToken('t', 'i-1'); + const { session, status } = setup('token'); + await session.start(); + expect(status.accessToken()).toBe('t'); + + status.reportOffline(); + + expect(status.accessToken()).toBeNull(); + fetchMock.mockClear(); + await expect( + session.api.sendJson('PUT', '/user-state/a/b', { value: 1 }) + ).rejects.toThrow(); + expect(fetchMock).not.toHaveBeenCalled(); + + fetchMock.mockResolvedValue( + answer({ instanceId: 'i-1', entries: {} }) + ); + await session.refresh(); + + expect(fetchMock.mock.calls[0][0]).toBe('/api/v1/instance'); + expect(fetchMock.mock.calls[0][1].headers).not.toHaveProperty( + 'Authorization' + ); + expect(status.accessToken()).toBe('t'); + expect(fetchMock).toHaveBeenCalledWith( + '/api/v1/user-state', + expect.anything() + ); + }); + + it('reloads when another instance answers at the address', async () => { + const { session, status } = setup('none'); + const reload = jest + .spyOn( + session as unknown as { reloadPage(): void }, + 'reloadPage' + ) + .mockImplementation(() => undefined); + await session.start(); + status.reportOffline(); + fetchMock.mockClear(); + fetchMock.mockResolvedValue(answer({ instanceId: 'other' })); + + await session.refresh(); + + expect(reload).toHaveBeenCalled(); + expect(fetchMock).not.toHaveBeenCalledWith( + '/api/v1/user-state', + expect.anything() + ); + }); + }); + describe('after an offline start', () => { it('stays offline until the server answers, then reloads once', async () => { const { session, status } = setup('none', true); 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 15ed0d7f5..de18904e9 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.ts @@ -1,4 +1,4 @@ -import { effect, inject, Injectable } from '@angular/core'; +import { effect, inject, Injectable, untracked } from '@angular/core'; import { MatSnackBar } from '@angular/material/snack-bar'; import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; @@ -44,10 +44,9 @@ export class ServerStorageSession { baseUrl: this.config.baseUrl, status: this.status, onWriteFailure: (error) => this.announceWriteFailure(error), - // A remembered instance may answer as another database: writes - // wait for the reload that checks which one it is, and the token - // goes out only once it is checked. - holdWrites: this.config.startedOffline, + // A remembered instance, or one that went offline, may answer as + // another database: writes and the token wait until it is checked. + holdWrites: () => !this.status.isVerified(), accessToken: () => this.status.accessToken(), }); readonly documentStore = new ServerPlaylistDocumentStore( @@ -72,6 +71,13 @@ export class ServerStorageSession { void this.router.navigateByUrl(SERVER_SIGN_IN_PATH); } }); + effect(() => { + // Gone offline, the token and writes wait for the instance + // check: after a blip it passes at once, not at the next retry. + if (this.status.state() === 'offline') { + untracked(() => void this.refresh()); + } + }); } /** App initializer: must run before the first playlist read. */ @@ -107,6 +113,8 @@ export class ServerStorageSession { return; } try { + // Back from offline: the instance first, as the read flushes. + await this.ensureVerified(); await this.userData.hydrate(); } catch { // The API client already reported offline or unauthorized. @@ -130,12 +138,12 @@ export class ServerStorageSession { } /** - * Before a token goes out from a page started from memory: the instance - * answering at the address must be the remembered one. Another one - * reloads the page, which starts against it. + * Before a token or a write goes out from a page started from memory, + * or one that went offline: the instance answering at the address must + * be this page's. Another one reloads the page, which starts against it. */ async ensureVerified(): Promise { - if (!this.config.startedOffline) { + if (this.status.isVerified()) { return; } const { instanceId } = @@ -166,6 +174,13 @@ export class ServerStorageSession { void this.refresh(); } }); + window.addEventListener('pageshow', (event) => { + // Back from the back-forward cache: another tab may have taken + // its writes over while it was frozen, so this page reloads. + if (event.persisted && !this.userData.resumed()) { + this.reloadPage(); + } + }); window.addEventListener('pagehide', (event) => { this.userData.flushWithKeepalive(); // Not into the back-forward cache: the page is gone for good. diff --git a/libs/services/src/lib/server-storage/server-unsent-writes.ts b/libs/services/src/lib/server-storage/server-unsent-writes.ts index fab8bf29a..c2eff6b37 100644 --- a/libs/services/src/lib/server-storage/server-unsent-writes.ts +++ b/libs/services/src/lib/server-storage/server-unsent-writes.ts @@ -404,25 +404,43 @@ export function pageIsLive( ); } -/** Writes only this page load's record (none without keys). */ +/** + * Writes only this page load's record (none without keys); true when one + * now stands for it. + */ export function markUnconfirmed( local: UserDataStorage, instanceId: string, tabId: string, keys: readonly string[], now = Date.now() -): void { +): boolean { try { if (keys.length === 0) { local.removeItem(recordKey(instanceId, tabId)); - } else { - local.setItem( - recordKey(instanceId, tabId), - JSON.stringify({ at: now, keys: [...keys] }) - ); + return false; } + local.setItem( + recordKey(instanceId, tabId), + JSON.stringify({ at: now, keys: [...keys] }) + ); + return true; } catch { // Storage full: re-reads in other tabs lose this protection only. + return false; + } +} + +/** Whether a page load's record still stands (another tab may remove it). */ +export function hasRecord( + local: UserDataStorage, + instanceId: string, + page: string +): boolean { + try { + return local.getItem(recordKey(instanceId, page)) !== null; + } catch { + return true; } } diff --git a/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts new file mode 100644 index 000000000..c5ead1632 --- /dev/null +++ b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts @@ -0,0 +1,159 @@ +import { ServerApiError } from './server-api-client'; +import { UNCONFIRMED_TTL_MS } from './server-unsent-writes'; +import { ServerUserDataStorage } from './server-user-data-storage'; +import { fakeServer, memoryStorage } from './server-user-data.test-helpers'; + +describe('ServerUserDataStorage taking over other page loads', () => { + it("keeps progress this tab saved over a closed tab's older edit", async () => { + const at = (seconds: number) => [ + { + contentType: 'vod', + contentXtreamId: 1, + positionSeconds: seconds, + }, + ]; + const server = fakeServer({ + 'xtream-playback-positions': { p1: at(10) }, + }); + const shared = memoryStorage(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const closed = new ServerUserDataStorage( + { + ...server.api, + sendJson: jest.fn(() => + Promise.reject(new ServerApiError(0, 'network', 'offline')) + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await closed.hydrate(); + closed.setItem( + 'xtream-playback-positions', + JSON.stringify({ p1: at(20) }) + ); + await closed.flush(); + closed.close(); + + // This tab saves newer progress before it adopts the closed one. + const tab = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await tab.hydrate(); + tab.setItem( + 'xtream-playback-positions', + JSON.stringify({ p1: at(30) }) + ); + // Sent on the server's value: the closed tab's write is unconfirmed. + await tab.flush(); + await tab.flush(); + expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); + + clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); + await tab.hydrate(); + await tab.flush(); + await tab.flush(); + clock.mockRestore(); + + expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); + expect( + JSON.parse(tab.getItem('xtream-playback-positions') ?? '{}')['p1'] + ).toEqual(at(30)); + }); + + it("sends a closed tab's saved write from an idle tab", async () => { + const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); + const shared = memoryStorage(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); + // Read, and with nothing to sync it never reads again. + const idle = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await idle.hydrate(); + const closed = new ServerUserDataStorage( + { + ...server.api, + sendJson: jest.fn(() => + Promise.reject(new ServerApiError(0, 'network', 'offline')) + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await closed.hydrate(); + closed.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); + await closed.flush(); + closed.close(); + + clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); + expect(idle.needsSync()).toBe(false); + idle.touch(); + await idle.flush(); + await idle.flush(); + clock.mockRestore(); + + expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'X']); + }); + + it('lets a page back from the back-forward cache give up writes another tab took', async () => { + const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); + const shared = memoryStorage(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const page = () => + new ServerUserDataStorage( + { + ...server.api, + sendJson: jest.fn(() => + Promise.reject( + new ServerApiError(0, 'network', 'offline') + ) + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + // Two pages frozen in the cache with a write each, not closed. + const adopted = page(); + await adopted.hydrate(); + adopted.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); + await adopted.flush(); + clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); + const other = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await other.hydrate(); + await other.flush(); + await other.flush(); + expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'X']); + const kept = page(); + await kept.hydrate(); + kept.setItem( + 'xtream-favorites', + JSON.stringify({ p1: ['S', 'X', 'Y'] }) + ); + await kept.flush(); + clock.mockRestore(); + + // Sent by the other tab: sending it again could undo newer edits. + expect(adopted.resumed()).toBe(false); + expect(kept.resumed()).toBe(true); + }); +}); diff --git a/libs/services/src/lib/server-storage/server-user-data-adopt.ts b/libs/services/src/lib/server-storage/server-user-data-adopt.ts index 4baf7a20f..82e70f591 100644 --- a/libs/services/src/lib/server-storage/server-user-data-adopt.ts +++ b/libs/services/src/lib/server-storage/server-user-data-adopt.ts @@ -7,6 +7,9 @@ import { } from './server-user-data-entries'; import { replaySteps } from './server-user-data-edit'; import type { UserDataWriteQueue } from './server-user-data-queue'; +import type { UserDataStorage } from '../user-data-storage'; +import type { DeferredEdit } from './server-user-data-edit'; +import { readDeferredEdits, readUnsentWrites } from './server-unsent-writes'; /** * A closed tab's saved edits, ahead of this tab's own (which may have been @@ -73,3 +76,31 @@ export function extendUnsent( }); } } + +/** + * A reload of this tab: its last page load's saved writes are queued and + * its early edits kept, both as this page's own. + */ +export function restorePage( + writes: UserDataWriteQueue, + deferred: Map, + local: UserDataStorage, + instanceId: string, + page: string +): void { + for (const write of readUnsentWrites(local, instanceId, page)) { + writes.queue( + write.namespace, + write.playlistId, + write.value, + undefined, + { + tag: write.tag, + steps: write.steps, + } + ); + } + for (const edit of readDeferredEdits(local, instanceId, page)) { + deferred.set(`${edit.namespace}|${edit.playlistId}`, edit); + } +} diff --git a/libs/services/src/lib/server-storage/server-user-data-edit.ts b/libs/services/src/lib/server-storage/server-user-data-edit.ts index 14da90886..8671ab7a7 100644 --- a/libs/services/src/lib/server-storage/server-user-data-edit.ts +++ b/libs/services/src/lib/server-storage/server-user-data-edit.ts @@ -140,6 +140,25 @@ export function replaySteps( return value; } +/** The playlist entries `next` changed against `previous`; removed ones as `undefined`. */ +export function changedEntries( + previous: Readonly>, + next: Readonly> +): [string, unknown][] { + const changed: [string, unknown][] = []; + for (const [playlistId, value] of Object.entries(next)) { + if (JSON.stringify(previous[playlistId]) !== JSON.stringify(value)) { + changed.push([playlistId, value]); + } + } + for (const playlistId of Object.keys(previous)) { + if (!(playlistId in next)) { + changed.push([playlistId, undefined]); + } + } + return changed; +} + /** The entries `next` changed against `read`, applied to `current`. */ export function mergeEdits( current: Record, 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 0be7127fd..a5d03a287 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 @@ -13,9 +13,14 @@ import { splitEditKey, UNKNOWN_TAG, } from './server-user-data-entries'; -import { adoptEdits, extendUnsent } from './server-user-data-adopt'; +import { + adoptEdits, + extendUnsent, + restorePage, +} from './server-user-data-adopt'; import { UserDataCache } from './server-user-data-cache'; import { + changedEntries, type DeferredEdit, mergeEdits, refreshedNamespace, @@ -24,6 +29,7 @@ import { import { type ServerApi, UserDataWriteQueue } from './server-user-data-queue'; import { claimCache, + hasRecord, markClosed, markUnconfirmed, orphanedPages, @@ -57,6 +63,8 @@ export class ServerUserDataStorage implements UserDataStorage { private hydrated = false; /** The previous page load in this tab, while it still looks live. */ private awaited: string | null = null; + /** A record stands for this page: a stale one is adopted elsewhere. */ + private recorded = false; /** Playlists deleted here, in order: a read sent earlier skips them. */ private readonly deleted: (string | undefined)[] = []; /** Closed tabs' page loads this one took over (once each). */ @@ -119,26 +127,13 @@ export class ServerUserDataStorage implements UserDataStorage { * go out with the first flush, and the first read keeps them. */ private takeOver(page: string): void { - for (const write of readUnsentWrites( + restorePage( + this.writes, + this.deferred, this.local, this.instanceId, page - )) { - this.writes.queue( - write.namespace, - write.playlistId, - write.value, - undefined, - { tag: write.tag, steps: write.steps } - ); - } - for (const edit of readDeferredEdits( - this.local, - this.instanceId, - page - )) { - this.deferred.set(`${edit.namespace}|${edit.playlistId}`, edit); - } + ); // Saved under this page load before the old one is forgotten; if // storage refuses the copy, the old one stays. const saved = [this.saveDeferred(), this.saveUnsent()].every(Boolean); @@ -207,6 +202,22 @@ export class ServerUserDataStorage implements UserDataStorage { } } + /** + * Back from the back-forward cache, where its record went stale: false + * when another tab took its writes over meanwhile, which this page + * must then not send again (it reloads instead). + */ + resumed(): boolean { + if ( + this.recorded && + !hasRecord(this.local, this.instanceId, this.tabId) + ) { + return false; + } + this.publishUnconfirmed(); + return true; + } + /** Not read yet, or holding writes the server has not taken. */ needsSync(): boolean { return !this.hydrated || this.hasUnsentWrites(); @@ -448,17 +459,8 @@ export class ServerUserDataStorage implements UserDataStorage { this.saveDeferred(); this.publishUnconfirmed(); }; - for (const [playlistId, value] of Object.entries(next)) { - if ( - JSON.stringify(previous[playlistId]) !== JSON.stringify(value) - ) { - change(playlistId, value); - } - } - for (const playlistId of Object.keys(previous)) { - if (!(playlistId in next)) { - change(playlistId, undefined); - } + for (const [playlistId, value] of changedEntries(previous, next)) { + change(playlistId, value); } this.writes.scheduleFlush(); } @@ -505,9 +507,11 @@ export class ServerUserDataStorage implements UserDataStorage { } private publishUnconfirmed(): void { - markUnconfirmed(this.local, this.instanceId, this.tabId, [ - ...this.writes.unconfirmedKeys(), - ...this.deferred.keys(), - ]); + this.recorded = markUnconfirmed( + this.local, + this.instanceId, + this.tabId, + [...this.writes.unconfirmedKeys(), ...this.deferred.keys()] + ); } } diff --git a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts index 90f1595ea..cce264177 100644 --- a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts @@ -681,109 +681,6 @@ describe('ServerUserDataStorage with other devices', () => { expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); }); - it("sends a closed tab's saved write from an idle tab", async () => { - const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); - const shared = memoryStorage(); - const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); - // Read, and with nothing to sync it never reads again. - const idle = new ServerUserDataStorage( - server.api as never, - shared, - jest.fn(), - 'default', - memoryStorage() - ); - await idle.hydrate(); - const closed = new ServerUserDataStorage( - { - ...server.api, - sendJson: jest.fn(() => - Promise.reject(new ServerApiError(0, 'network', 'offline')) - ), - } as never, - shared, - jest.fn(), - 'default', - memoryStorage() - ); - await closed.hydrate(); - closed.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); - await closed.flush(); - closed.close(); - - clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); - expect(idle.needsSync()).toBe(false); - idle.touch(); - await idle.flush(); - await idle.flush(); - clock.mockRestore(); - - expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'X']); - }); - - it("keeps progress this tab saved over a closed tab's older edit", async () => { - const at = (seconds: number) => [ - { - contentType: 'vod', - contentXtreamId: 1, - positionSeconds: seconds, - }, - ]; - const server = fakeServer({ - 'xtream-playback-positions': { p1: at(10) }, - }); - const shared = memoryStorage(); - const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); - const closed = new ServerUserDataStorage( - { - ...server.api, - sendJson: jest.fn(() => - Promise.reject(new ServerApiError(0, 'network', 'offline')) - ), - } as never, - shared, - jest.fn(), - 'default', - memoryStorage() - ); - await closed.hydrate(); - closed.setItem( - 'xtream-playback-positions', - JSON.stringify({ p1: at(20) }) - ); - await closed.flush(); - closed.close(); - - // This tab saves newer progress before it adopts the closed one. - const tab = new ServerUserDataStorage( - server.api as never, - shared, - jest.fn(), - 'default', - memoryStorage() - ); - await tab.hydrate(); - tab.setItem( - 'xtream-playback-positions', - JSON.stringify({ p1: at(30) }) - ); - // Sent on the server's value: the closed tab's write is unconfirmed. - await tab.flush(); - await tab.flush(); - expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); - - clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); - await tab.hydrate(); - await tab.flush(); - await tab.flush(); - clock.mockRestore(); - - expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); - expect( - JSON.parse(tab.getItem('xtream-playback-positions') ?? '{}')['p1'] - ).toEqual(at(30)); - }); - it("keeps the last page load's saved writes when storage refuses the copy", async () => { const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); const local = memoryStorage();