diff --git a/docs/architecture/playlist-backup-restore.md b/docs/architecture/playlist-backup-restore.md index 52ac6db09..72d58abbe 100644 --- a/docs/architecture/playlist-backup-restore.md +++ b/docs/architecture/playlist-backup-restore.md @@ -252,9 +252,10 @@ self-hosted server storage is covered too. browser tracks neither. - An unreadable map (storage error, malformed JSON, a playlist entry that is not a list) fails the export instead of being written as an empty list. - So does server mode before its first read of the server's user data: - the export waits for a read in flight (`UserDataStorage.ready`) and fails - when none succeeded. A restore does the same and stays parked, since its + So does server mode when it cannot read the server's user data: the + export reads it again (`UserDataStorage.ready(true)`, as another device + may have changed it since this page's first read) and fails when that + read does not succeed. A restore does the same and stays parked, since its replacement lists would otherwise merge into the server's; it also stays parked until the server took that playlist's lists. - A pending restore snapshot still wins over the stored maps, as on desktop. diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index c3e7fab5b..5936f001a 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -264,8 +264,9 @@ settings and guards read them in every build. succeeded. The Xtream data source's favorites, favorite checks, history and position reads wait for it, so a new device does not show empty lists or start playback without its resume position. A backup export or - restore (also its replay when the playlist opens) fails while no read - succeeded: unread maps would export as empty lists, and a restore's lists + restore (also its replay when the playlist opens) reads the server again + first (`ready(true)`: another device may have changed the lists since) and + fails when that read does not succeed: unread maps would export as empty lists, and a restore's lists would merge into the server's instead of replacing them (the restore stays parked). A restore also counts only once the server took that playlist's lists (`UserDataStorage.flushed`); a refused one stays parked. Replayed edits keep recently viewed lists newest first, diff --git a/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.spec.ts b/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.spec.ts index 646359f1d..8bf976ea8 100644 --- a/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.spec.ts +++ b/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.spec.ts @@ -1021,7 +1021,7 @@ describe('PwaXtreamDataSource', () => { removeItem: (key: string) => { values.delete(key); }, - ready: async () => false, + ready: jest.fn(async () => false), }; TestBed.resetTestingModule(); TestBed.configureTestingModule({ @@ -1043,6 +1043,8 @@ describe('PwaXtreamDataSource', () => { playbackPositions: [], } as never) ).rejects.toThrow(/restore waits/); + // A fresh read: the restore replaces the current lists. + expect(storage.ready).toHaveBeenCalledWith(true); expect(values.get('xtream-favorites')).toBe( JSON.stringify({ p1: ['live:1'] }) ); diff --git a/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.ts b/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.ts index 0941c175b..6f9ddad36 100644 --- a/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.ts +++ b/libs/portal/xtream/data-access/src/lib/data-sources/pwa-xtream-data-source.ts @@ -951,10 +951,11 @@ export class PwaXtreamDataSource implements IXtreamDataSource { * Server mode: favorites, history and positions show the server's data * once its first read settled, which startup does not wait for beyond a * few seconds. A read before it would show a new device empty lists. - * Whether a read succeeded (always true outside server mode). + * Whether a read succeeded (always true outside server mode); `fresh` + * reads again first, for a restore that replaces the current lists. */ - private async userDataRead(): Promise { - return (await this.userDataStorage.ready?.()) ?? true; + private async userDataRead(fresh = false): Promise { + return (await this.userDataStorage.ready?.(fresh)) ?? true; } private getPlaybackPositionsFromStorage(): Record< @@ -1407,7 +1408,7 @@ export class PwaXtreamDataSource implements IXtreamDataSource { void options; // Its lists replace the server's: before the server's are read they // would merge into them instead. The restore stays parked. - if (!(await this.userDataRead())) { + if (!(await this.userDataRead(true))) { throw new Error( "The server's user data could not be read yet; the restore waits." ); diff --git a/libs/services/src/lib/playlist-backup.service.pwa-xtream.spec.ts b/libs/services/src/lib/playlist-backup.service.pwa-xtream.spec.ts index e02f6a0d8..d9c54858b 100644 --- a/libs/services/src/lib/playlist-backup.service.pwa-xtream.spec.ts +++ b/libs/services/src/lib/playlist-backup.service.pwa-xtream.spec.ts @@ -288,6 +288,8 @@ describe('PlaylistBackupService Xtream user state in the PWA', () => { const { manifest } = await service.exportBackup(); + // A fresh read: another device may have changed them since. + expect(ready).toHaveBeenCalledWith(true); expect(ready).toHaveBeenCalledTimes(1); expect(xtreamEntry(manifest).userState.favorites).not.toEqual([]); }); diff --git a/libs/services/src/lib/playlists.service.spec.ts b/libs/services/src/lib/playlists.service.spec.ts index 78c9723e4..95929ee24 100644 --- a/libs/services/src/lib/playlists.service.spec.ts +++ b/libs/services/src/lib/playlists.service.spec.ts @@ -1808,6 +1808,70 @@ describe('PlaylistsService', () => { expect(order).toEqual(['read', 'write', 'delete all', 'read', 'write']); }); + it('lets a repair holding its reservation write before Delete All', async () => { + let releaseRepair: () => void = () => undefined; + const repairHeld = new Promise( + (resolve) => (releaseRepair = resolve) + ); + const request = jest.fn( + async ( + name: string, + options: LockOptions, + callback: (lock: Lock | null) => Promise + ) => { + // The reset waits for the reservation a repair holds. + if (options.mode === 'exclusive') { + await repairHeld; + } + return callback({ + name, + mode: options.mode ?? 'exclusive', + } as Lock); + } + ); + Object.defineProperty(globalThis.navigator, 'locks', { + configurable: true, + value: { request }, + }); + const electron = { + dbGetAppPlaylist: jest.fn(async () => ({ + _id: 'p9', + title: 'P', + favorites: [], + })), + dbGetAppPlaylists: jest.fn(async () => []), + dbGetAppState: jest.fn(async (key: string) => + key === SQLITE_PLAYLIST_MIGRATION_FLAG ? '1' : null + ), + dbSetAppState: jest.fn(), + dbUpsertAppPlaylist: jest.fn(async () => undefined), + dbUpsertAppPlaylists: jest.fn(), + dbDeleteAllPlaylists: jest.fn(async () => undefined), + }; + testWindow.electron = electron; + const service = createService(); + const settle = async () => { + for (let i = 0; i < 50; i += 1) { + await Promise.resolve(); + } + }; + + const removed = firstValueFrom(service.removeAll()); + // The repair's conditional row write, under its reservation. + let written = false; + const repair = firstValueFrom( + service.transformPlaylistFavorites('p9', (f) => [...f, 'a']) + ).then(() => (written = true)); + await settle(); + + expect(written).toBe(true); + expect(electron.dbDeleteAllPlaylists).not.toHaveBeenCalled(); + releaseRepair(); + await repair; + await removed; + expect(electron.dbDeleteAllPlaylists).toHaveBeenCalled(); + }); + it('runs a mutation again on a fresh read when the server refused it', async () => { const versions = [ { _id: 'p9', title: 'P', favorites: ['a'] }, diff --git a/libs/services/src/lib/playlists.service.ts b/libs/services/src/lib/playlists.service.ts index 2f8943982..c987b310e 100644 --- a/libs/services/src/lib/playlists.service.ts +++ b/libs/services/src/lib/playlists.service.ts @@ -1342,34 +1342,39 @@ export class PlaylistsService { } removeAll(): Observable { - // Writes already queued land first; new ones wait for the delete. - return defer(() => { - const pending = [...this.playlistWriteQueues.values()]; - const run = Promise.all(pending).then(() => this.removeAllNow()); - this.deletingAll = run.then( - () => undefined, - () => undefined - ); - return run; - }); + // Inside the exclusive authority reset: a Stalker repair already + // holding its reservation finishes its write first (blocking it here + // would deadlock). Then writes queued so far land, and new ones wait + // for the delete. + return defer(() => + runWithPlaylistAuthorityReset(async () => { + const pending = [...this.playlistWriteQueues.values()]; + let deleted: () => void = () => undefined; + this.deletingAll = new Promise( + (resolve) => (deleted = resolve) + ); + try { + await Promise.all(pending); + await this.deleteAllRows(); + } finally { + deleted(); + } + }) + ); } - private removeAllNow(): Promise { - return runWithPlaylistAuthorityReset(async () => { - if (this.usesDocumentStore) { - await this.ensureElectronPlaylistMigrations(); - const store = this.documentStore; - if (store) { - this.startupMetas = null; - await store.dbDeleteAllPlaylists(); - this.healthEvidence?.connections.next({}); - } - return undefined; + private async deleteAllRows(): Promise { + if (this.usesDocumentStore) { + await this.ensureElectronPlaylistMigrations(); + const store = this.documentStore; + if (store) { + this.startupMetas = null; + await store.dbDeleteAllPlaylists(); + this.healthEvidence?.connections.next({}); } - - await firstValueFrom(this.dbService.clear(DbStores.Playlists)); - return undefined; - }); + return; + } + await firstValueFrom(this.dbService.clear(DbStores.Playlists)); } private normalizePortalRecentIdentity(value: unknown): string { diff --git a/libs/services/src/lib/pwa-xtream-backup-state.service.ts b/libs/services/src/lib/pwa-xtream-backup-state.service.ts index b6baeb322..c72b7810a 100644 --- a/libs/services/src/lib/pwa-xtream-backup-state.service.ts +++ b/libs/services/src/lib/pwa-xtream-backup-state.service.ts @@ -38,7 +38,9 @@ export class PwaXtreamBackupStateService { * restore's replacement lists would merge into the server's instead. */ async ready(): Promise { - if ((await this.storage.ready?.()) === false) { + // Read again: another device may have changed them since this + // page's first read, and a restore must replace the current lists. + if ((await this.storage.ready?.(true)) === false) { throw new Error( "The server's user data could not be read yet; try again once the server answers." ); 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 756f187ec..b596ca092 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 @@ -10,7 +10,12 @@ 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'; +import { + othersUnconfirmed, + readDeferredEdits, + readUnsentWrites, +} from './server-unsent-writes'; +import type { UserDataCache } from './server-user-data-cache'; /** * A closed tab's saved edits, ahead of this tab's own (which may have been @@ -128,3 +133,35 @@ export async function settleWrites( } return !unsent(); } + +/** + * Another page load's saved writes and early edits, shown on what this tab + * shows now and queued with its writes (also when the cache shows them). + */ +export function adoptPageEdits( + writes: UserDataWriteQueue, + cache: UserDataCache, + from: { + readonly local: UserDataStorage; + readonly instanceId: string; + readonly tabId: string; + readonly page: string; + } +): void { + const { local, instanceId, tabId, page } = from; + const foreign = othersUnconfirmed(local, instanceId, tabId); + for (const { namespace, playlistId, steps } of [ + ...readDeferredEdits(local, instanceId, page), + ...readUnsentWrites(local, instanceId, page), + ]) { + const value = adoptEdits( + writes, + namespace, + playlistId, + cache.map(namespace)[playlistId], + steps, + foreign.has(`${namespace}|${playlistId}`) + ); + cache.setEntry(namespace, playlistId, value); + } +} 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 46b797970..7f8a4b7ce 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 @@ -594,6 +594,22 @@ describe('ServerUserDataStorage', () => { expect(api.getJson).toHaveBeenCalledTimes(2); }); + it('reads the server again when a backup asks for fresh data', async () => { + const { api, storage } = setup(); + await storage.hydrate(); + api.getJson.mockClear(); + + // Another device may have changed it since: read again. + await expect(storage.ready(true)).resolves.toBe(true); + expect(api.getJson).toHaveBeenCalledTimes(1); + + api.getJson.mockRejectedValueOnce(new Error('offline')); + await expect(storage.ready(true)).resolves.toBe(false); + // Without `fresh` an earlier read is enough. + await expect(storage.ready()).resolves.toBe(true); + expect(api.getJson).toHaveBeenCalledTimes(2); + }); + it("never writes over another server instance's cache", async () => { const local = memoryStorage(); const tab = (instanceId: string, favorites: unknown) => 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 2a2c37a55..e4844dc49 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 @@ -14,7 +14,7 @@ import { UNKNOWN_TAG, } from './server-user-data-entries'; import { - adoptEdits, + adoptPageEdits, extendUnsent, restorePage, settleWrites, @@ -36,8 +36,6 @@ import { orphanedPages, pageIsLive, othersUnconfirmed, - readDeferredEdits, - readUnsentWrites, removePage, saveDeferredEdits, saveUnsentWrites, @@ -159,27 +157,12 @@ export class ServerUserDataStorage implements UserDataStorage { /** Its edits go ahead of this tab's own, which may be newer. */ private adoptPage(page: string): void { - const edits = [ - ...readDeferredEdits(this.local, this.instanceId, page), - ...readUnsentWrites(this.local, this.instanceId, page), - ]; - const foreign = othersUnconfirmed( - this.local, - this.instanceId, - this.tabId - ); - for (const { namespace, playlistId, steps } of edits) { - // Queued even when the cache shows it already. - const value = adoptEdits( - this.writes, - namespace, - playlistId, - this.cache.map(namespace)[playlistId], - steps, - foreign.has(`${namespace}|${playlistId}`) - ); - this.cache.setEntry(namespace, playlistId, value); - } + adoptPageEdits(this.writes, this.cache, { + local: this.local, + instanceId: this.instanceId, + tabId: this.tabId, + page, + }); this.adopted.add(page); // The other page's copy goes only once this tab saved its own. this.release(page, this.saveUnsent()); @@ -232,9 +215,16 @@ export class ServerUserDataStorage implements UserDataStorage { /** * Once a first read in flight settled: whether reads show the server's - * data. Starts no read, so an offline page is not held up. + * data. Starts no read, so an offline page is not held up; `fresh` + * (backups) reads the server again and tells whether that succeeded. */ - async ready(): Promise { + async ready(fresh = false): Promise { + if (fresh) { + return this.hydrate().then( + () => true, + () => false + ); + } await (this.hydrated ? undefined : this.hydration); return this.hydrated; } diff --git a/libs/services/src/lib/user-data-storage.ts b/libs/services/src/lib/user-data-storage.ts index d085fa7de..c36f7bfeb 100644 --- a/libs/services/src/lib/user-data-storage.ts +++ b/libs/services/src/lib/user-data-storage.ts @@ -4,7 +4,8 @@ import { InjectionToken } from '@angular/core'; * The slice of the Web Storage API that PWA user-data stores rely on, plus * the stored keys where the store can list them, and where writes reach a * source later (server mode): `ready`, which waits for a first read in - * flight and tells whether one succeeded, and `flushed`, which sends what + * flight and tells whether one succeeded (`fresh`: reads again, for + * backups that must see the source's current data), and `flushed`, which sends what * is queued and tells whether all of it (of one playlist) was taken. */ export type UserDataStorage = Pick< @@ -12,7 +13,7 @@ export type UserDataStorage = Pick< 'getItem' | 'setItem' | 'removeItem' > & { keys?(): string[]; - ready?(): Promise; + ready?(fresh?: boolean): Promise; flushed?(playlistId?: string): Promise; };