diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index c3ba7e46c..0fe1aa880 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -420,6 +420,10 @@ by the index alone: durable commit points: right after the store write (a refusal writes the previous store back) and, for Xtream, after the index stamps (a refusal rolls back like a failed stamp), before the revision is published. + If writing the previous store back fails, the persisted copy may carry + the removal while memory does not: the store stays not `readable` + (fail-closed) and is rewritten from memory on the next store access, as + a failed "Remove all playlists" clear is. Adding locks is always allowed. Playlist deletion and "Remove all playlists" are not edits and are not gated. @@ -445,8 +449,8 @@ the store through the `PLAYLIST_DELETE_CLEANUP` hook re-stamp, the category rows go with the playlist), and "Remove all playlists" empties it through `ParentalLockService.clearAllLocks` once the deletion has succeeded — the in-memory store empties at once, and a failed -persisted clear is retried on the next store access and overwritten by the -next write. A backup restore that CREATES a playlist (not a merge) starts +persisted clear is retried on the next store access (the store is not +`readable` until it lands). A backup restore that CREATES a playlist (not a merge) starts that playlist from empty locks, so a reused id can never inherit entries a failed cleanup left behind; the restore retries a failed lock-store read first (`ensureLocksReadable`) and aborts while it still fails, so that check diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts index 8bd8b9e11..47a1d6207 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts @@ -494,6 +494,36 @@ describe('ParentalLockLockStore', () => { }); }); + it('fails closed and retries when the revert of a refused removal cannot be written', async () => { + storage.readLocks.mockResolvedValue({ + 'pl-1': { xtream: [], stalker: [], m3u: ['Adults'] }, + }); + await store.load(); + await store.ensureReadable(); + let unlocked = true; + store.setRemovalGate(() => unlocked); + jest.spyOn(console, 'warn').mockImplementation(() => undefined); + jest.spyOn(console, 'error').mockImplementation(() => undefined); + storage.writeLocks + .mockImplementationOnce(async () => { + unlocked = false; // relock while the removal is written + return true; + }) + .mockResolvedValueOnce(false) // the revert + .mockResolvedValueOnce(false); // the first retry + + await expect(store.setM3uLocks('pl-1', [])).resolves.toBe(false); + // The persisted store may carry the removal: not readable. + expect(store.readable()).toBe(false); + expect(store.lockedGroupTitles('pl-1')).toEqual(['Adults']); + await expect(store.ensureReadable()).resolves.toBe(false); + + await expect(store.ensureReadable()).resolves.toBe(true); + expect(storage.writeLocks).toHaveBeenLastCalledWith({ + 'pl-1': { xtream: [], stalker: [], m3u: ['Adults'] }, + }); + }); + it('rolls back an Xtream removal when the app relocks during the index stamps', async () => { await store.load(); await store.ensureReadable(); diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts index aed7f7044..5508fc8c5 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts @@ -75,8 +75,14 @@ export class ParentalLockLockStore { * same store and the later write would silently drop the earlier edit. */ private writeQueue: Promise = Promise.resolve(); - /** "Remove all playlists" could not clear the persisted store yet. */ - private pendingClearAll = false; + /** + * The persisted store differs from the in-memory one and must be + * rewritten from it: "Remove all playlists" could not clear it, or a + * refused edit could not be reverted. Retried on the next store access; + * the store is not `readable` meanwhile, since a restart would load the + * persisted copy. + */ + private readonly pendingRewrite = signal(false); private readonly staleIndex = new ParentalLockStaleIndex(); /** The persisted store could not be read; see `ensureReadable()`. */ @@ -93,6 +99,7 @@ export class ParentalLockLockStore { () => this.loadedState() && !this.unreadable() && + !this.pendingRewrite() && this.staleIndex.isEmpty() ); /** Bumps whenever the lock set changes; consumers re-query. */ @@ -136,11 +143,8 @@ export class ParentalLockLockStore { */ async ensureReadable(): Promise { await this.load(); - if (this.pendingClearAll) { - this.pendingClearAll = !(await this.storage.writeLocks({})); - if (this.pendingClearAll) { - return false; - } + if (this.pendingRewrite() && !(await this.rewritePersistedStore())) { + return false; } if (this.unreadable()) { const locks = await this.storage.readLocks(); @@ -167,21 +171,9 @@ export class ParentalLockLockStore { this.staleIndex.clear(); return; } - const restamped: string[] = []; - for (const playlistId of this.staleIndex.ids()) { - try { - if (await this.stampXtreamLocks(playlistId)) { - restamped.push(playlistId); - } - } catch (error) { - console.error( - 'Failed to reconcile the parental lock index.', - error - ); - } - } - if (restamped.length > 0) { - this.staleIndex.unmark(...restamped); + if ( + await this.staleIndex.reconcile((id) => this.stampXtreamLocks(id)) + ) { this.revisionState.update((value) => value + 1); } } @@ -333,8 +325,7 @@ export class ParentalLockLockStore { this.unreadable.set(false); this.staleIndex.clear(); this.revisionState.update((value) => value + 1); - this.pendingClearAll = !(await this.storage.writeLocks({})); - return !this.pendingClearAll; + return this.rewritePersistedStore(); }); } @@ -351,6 +342,19 @@ export class ParentalLockLockStore { ); } + /** Writes the in-memory store; a failure leaves `pendingRewrite` set. */ + private async rewritePersistedStore(): Promise { + const written = await this.storage.writeLocks(this.locks()); + if (!written) { + console.error('The parental lock store could not be rewritten.'); + } + if (written === this.pendingRewrite()) { + this.pendingRewrite.set(!written); + this.revisionState.update((value) => value + 1); + } + return written; + } + private enqueue(task: () => Promise): Promise { const run = this.writeQueue.then(task, task); this.writeQueue = run.catch(() => undefined); @@ -535,9 +539,7 @@ export class ParentalLockLockStore { if (options.authorize && !options.authorize()) { // Relocked while the write was in flight: the durable store goes // back to the in-memory one, which still holds the lock. - if (!(await this.storage.writeLocks(this.locks()))) { - console.error('Failed to revert a refused parental lock edit.'); - } + await this.rewritePersistedStore(); return false; } this.locks.set(next); diff --git a/libs/services/src/lib/parental-lock/parental-lock-stale-index.ts b/libs/services/src/lib/parental-lock/parental-lock-stale-index.ts index b4a08a053..0c3206d4d 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-stale-index.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-stale-index.ts @@ -31,4 +31,30 @@ export class ParentalLockStaleIndex { clear(): void { this.unmark(...this.playlists); } + + /** + * Re-stamps every listed playlist through `stamp`; the ones that + * succeed leave the list. True when any did. + */ + async reconcile( + stamp: (playlistId: string) => Promise + ): Promise { + const restamped: string[] = []; + for (const playlistId of this.ids()) { + try { + if (await stamp(playlistId)) { + restamped.push(playlistId); + } + } catch (error) { + console.error( + 'Failed to reconcile the parental lock index.', + error + ); + } + } + if (restamped.length > 0) { + this.unmark(...restamped); + } + return restamped.length > 0; + } }