From c4754a7aa4e201899ddd84084d897e955717635d Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 13:43:11 +0200 Subject: [PATCH] fix(settings): re-check lock removals at their durable commit points A lock removal that passed the unlocked check before its write is asked again right after the store write and, for Xtream, after the index stamps. A relock in between writes the previous store back or rolls the stamps back before anything is published. The stale-index bookkeeping, index stamping and store merge move into helpers to keep the lock store within the file size limit. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/parental-lock.md | 6 +- .../parental-lock-lock-store.service.spec.ts | 53 +++++++ .../parental-lock-lock-store.service.ts | 144 +++++++++--------- .../parental-lock-stale-index.ts | 34 +++++ .../parental-lock/parental-lock-store.util.ts | 70 +++++++-- 5 files changed, 223 insertions(+), 84 deletions(-) create mode 100644 libs/services/src/lib/parental-lock/parental-lock-stale-index.ts diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 594f30dea..c3ba7e46c 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -416,7 +416,11 @@ by the index alone: unlocked, checked inside the write queue at commit time (`ParentalLockLockStore.setRemovalGate`, set by `ParentalLockService`). An editor opened while unlocked may still be saving, or queued behind - another write, when the app relocks. Adding locks is always allowed. + another write, when the app relocks. The answer is asked again at the + 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. + Adding locks is always allowed. Playlist deletion and "Remove all playlists" are not edits and are not gated. - Every launch re-derives the index from the store for each playlist that 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 28060bf1e..8bd8b9e11 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 @@ -465,6 +465,59 @@ describe('ParentalLockLockStore', () => { await expect(store.setM3uLocks('pl-1', [])).resolves.toBe(true); expect(store.lockedGroupTitles('pl-1')).toEqual([]); }); + it('rolls back a removal when the app relocks while its store write is in flight', 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); + let finishWrite: (ok: boolean) => void = () => undefined; + storage.writeLocks.mockImplementationOnce( + () => new Promise((resolve) => (finishWrite = resolve)) + ); + const revision = store.revision(); + + const removing = store.setM3uLocks('pl-1', []); + await waitFor(() => storage.writeLocks.mock.calls.length > 0); + unlocked = false; + finishWrite(true); + + await expect(removing).resolves.toBe(false); + expect(store.lockedGroupTitles('pl-1')).toEqual(['Adults']); + expect(store.revision()).toBe(revision); + // The durable store is put back. + 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(); + let unlocked = true; + store.setRemovalGate(() => unlocked); + jest.spyOn(console, 'warn').mockImplementation(() => undefined); + await store.setXtreamLocks('pl-1', 'live', [7, 9]); + setCategoryLocks.mockImplementationOnce(async () => { + unlocked = false; + return true; + }); + + await expect(store.setXtreamLocks('pl-1', 'live', [7])).resolves.toBe( + false + ); + + expect(store.lockedXtreamIds('pl-1', 'live')).toEqual([7, 9]); + expect(setCategoryLocks).toHaveBeenLastCalledWith( + 'pl-1', + 'live', + [7, 9] + ); + expect(store.readable()).toBe(true); + }); }); async function waitFor(condition: () => boolean): Promise { 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 7f83f7247..aed7f7044 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 @@ -12,10 +12,13 @@ import { } from '@iptvnator/shared/interfaces'; import { DatabaseService } from '../database-electron.service'; import { RuntimeCapabilitiesService } from '../runtime-capabilities.service'; +import { ParentalLockStaleIndex } from './parental-lock-stale-index'; import { ParentalLockStorageService } from './parental-lock-storage'; import { - isLockRemovalAllowed, + lockRemovalGate, + stampXtreamIndex, withM3uLocks, + withPlaylistLocksInStore, withStalkerLocks, withXtreamLocks, } from './parental-lock-store.util'; @@ -74,13 +77,7 @@ export class ParentalLockLockStore { private writeQueue: Promise = Promise.resolve(); /** "Remove all playlists" could not clear the persisted store yet. */ private pendingClearAll = false; - /** - * Playlists whose SQLite index could not be brought in line with the - * store (a failed re-stamp whose rollback failed too); re-stamped on - * the next store access. - */ - private readonly staleIndexPlaylists = new Set(); - private readonly staleIndexCount = signal(0); + private readonly staleIndex = new ParentalLockStaleIndex(); /** The persisted store could not be read; see `ensureReadable()`. */ readonly unreadable = signal(false); @@ -96,7 +93,7 @@ export class ParentalLockLockStore { () => this.loadedState() && !this.unreadable() && - this.staleIndexCount() === 0 + this.staleIndex.isEmpty() ); /** Bumps whenever the lock set changes; consumers re-query. */ readonly revision = this.revisionState.asReadonly(); @@ -119,7 +116,7 @@ export class ParentalLockLockStore { } else { this.locks.set(locks); for (const playlistId of Object.keys(locks)) { - this.markIndexStale(playlistId); + this.staleIndex.mark(playlistId); } // Awaited: catalog reads issued before the index agrees // with the store would serve rows stamped unlocked, and @@ -156,7 +153,7 @@ export class ParentalLockLockStore { // the read did; re-derive it from the recovered store as load() // does. for (const playlistId of Object.keys(locks)) { - this.markIndexStale(playlistId); + this.staleIndex.mark(playlistId); } this.revisionState.update((value) => value + 1); } @@ -164,27 +161,17 @@ export class ParentalLockLockStore { return this.readable(); } - private markIndexStale(playlistId: string): void { - this.staleIndexPlaylists.add(playlistId); - this.staleIndexCount.set(this.staleIndexPlaylists.size); - } - - private unmarkIndexStale(playlistId: string): void { - this.staleIndexPlaylists.delete(playlistId); - this.staleIndexCount.set(this.staleIndexPlaylists.size); - } - /** Re-stamps every playlist whose index may lag behind the store. */ private async reconcileXtreamIndex(): Promise { if (!this.runtime.supportsXtreamSqliteDataSource) { - this.staleIndexPlaylists.clear(); - this.staleIndexCount.set(0); + this.staleIndex.clear(); return; } - for (const playlistId of [...this.staleIndexPlaylists]) { + const restamped: string[] = []; + for (const playlistId of this.staleIndex.ids()) { try { if (await this.stampXtreamLocks(playlistId)) { - this.staleIndexPlaylists.delete(playlistId); + restamped.push(playlistId); } } catch (error) { console.error( @@ -193,8 +180,8 @@ export class ParentalLockLockStore { ); } } - if (this.staleIndexCount() !== this.staleIndexPlaylists.size) { - this.staleIndexCount.set(this.staleIndexPlaylists.size); + if (restamped.length > 0) { + this.staleIndex.unmark(...restamped); this.revisionState.update((value) => value + 1); } } @@ -251,9 +238,6 @@ export class ParentalLockLockStore { this.lockedXtreamIds(playlistId, categoryType) ) ); - if (!isLockRemovalAllowed(previous, next, this.mayRemoveLocks)) { - return false; - } return this.commitLocks(playlistId, previous, next, [categoryType]); }); } @@ -276,10 +260,7 @@ export class ParentalLockLockStore { this.lockedStalkerIds(playlistId, categoryType) ) ); - return ( - isLockRemovalAllowed(previous, next, this.mayRemoveLocks) && - this.persistPlaylistLocks(playlistId, next) - ); + return this.persistEdit(playlistId, previous, next); }); } @@ -299,10 +280,7 @@ export class ParentalLockLockStore { this.lockedGroupTitles(playlistId) ) ); - return ( - isLockRemovalAllowed(previous, next, this.mayRemoveLocks) && - this.persistPlaylistLocks(playlistId, next) - ); + return this.persistEdit(playlistId, previous, next); }); } @@ -315,13 +293,9 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } - const previous = this.locksFor(playlistId); - if (!isLockRemovalAllowed(previous, locks, this.mayRemoveLocks)) { - return false; - } return this.commitLocks( playlistId, - previous, + this.locksFor(playlistId), locks, XTREAM_CATEGORY_TYPES ); @@ -357,14 +331,26 @@ export class ParentalLockLockStore { // rewrites the whole store from this empty copy. this.locks.set({}); this.unreadable.set(false); - this.staleIndexPlaylists.clear(); - this.staleIndexCount.set(0); + this.staleIndex.clear(); this.revisionState.update((value) => value + 1); this.pendingClearAll = !(await this.storage.writeLocks({})); return !this.pendingClearAll; }); } + /** Stalker and M3U edits: no index, only the store write. */ + private async persistEdit( + playlistId: string, + previous: ParentalLockPlaylistLocks, + next: ParentalLockPlaylistLocks + ): Promise { + const authorize = lockRemovalGate(previous, next, this.mayRemoveLocks); + return ( + authorize() && + this.persistPlaylistLocks(playlistId, next, { authorize }) + ); + } + private enqueue(task: () => Promise): Promise { const run = this.writeQueue.then(task, task); this.writeQueue = run.catch(() => undefined); @@ -387,6 +373,10 @@ export class ParentalLockLockStore { next: ParentalLockPlaylistLocks, categoryTypes: readonly ParentalLockXtreamCategoryType[] ): Promise { + const authorize = lockRemovalGate(previous, next, this.mayRemoveLocks); + if (!authorize()) { + return false; + } const normalizedNext = normalizeParentalLockPlaylistLocks(next); if (isParentalLockPlaylistLocksEmpty(normalizedNext)) { if (!(await this.ensureReadable())) { @@ -405,9 +395,10 @@ export class ParentalLockLockStore { )) && (await this.persistPlaylistLocks(playlistId, normalizedNext, { publish: false, + authorize, })) ) { - this.unmarkIndexStale(playlistId); + this.staleIndex.unmark(playlistId); this.revisionState.update((value) => value + 1); return true; } @@ -425,13 +416,19 @@ export class ParentalLockLockStore { if ( !(await this.persistPlaylistLocks(playlistId, next, { publish: false, + authorize, })) ) { return false; } this.markStaleWhileStamping(playlistId); - if (await this.stampXtreamLocks(playlistId, categoryTypes)) { - this.unmarkIndexStale(playlistId); + // Asked again once the stamps landed: a relock during them must not + // see the removal published. + if ( + (await this.stampXtreamLocks(playlistId, categoryTypes)) && + authorize() + ) { + this.staleIndex.unmark(playlistId); this.revisionState.update((value) => value + 1); return true; } @@ -464,9 +461,9 @@ export class ParentalLockLockStore { (await this.stampXtreamLocks(playlistId, categoryTypes)); if (!restored || !restamped) { console.error('Failed to roll back the parental lock index.'); - this.markIndexStale(playlistId); + this.staleIndex.mark(playlistId); } else { - this.unmarkIndexStale(playlistId); + this.staleIndex.unmark(playlistId); } this.revisionState.update((value) => value + 1); } @@ -479,7 +476,7 @@ export class ParentalLockLockStore { */ private markStaleWhileStamping(playlistId: string): void { if (this.runtime.supportsXtreamSqliteDataSource) { - this.markIndexStale(playlistId); + this.staleIndex.mark(playlistId); } } @@ -491,9 +488,9 @@ export class ParentalLockLockStore { ): Promise { if (!(await this.stampXtreamLocks(playlistId, categoryTypes, locks))) { console.error('Failed to restore the parental lock index.'); - this.markIndexStale(playlistId); + this.staleIndex.mark(playlistId); } else { - this.unmarkIndexStale(playlistId); + this.staleIndex.unmark(playlistId); } this.revisionState.update((value) => value + 1); } @@ -507,25 +504,22 @@ export class ParentalLockLockStore { categoryTypes: readonly ParentalLockXtreamCategoryType[] = XTREAM_CATEGORY_TYPES, locks: ParentalLockPlaylistLocks = this.locksFor(playlistId) ): Promise { - if (!this.runtime.supportsXtreamSqliteDataSource) { - return true; - } - let success = true; - for (const categoryType of categoryTypes) { - success = - (await this.databaseService.setCategoryLocks( - playlistId, - categoryType, - lockedXtreamCategoryIds(locks, categoryType) - )) && success; - } - return success; + return ( + !this.runtime.supportsXtreamSqliteDataSource || + stampXtreamIndex( + (id, type, ids) => + this.databaseService.setCategoryLocks(id, type, ids), + playlistId, + categoryTypes, + locks + ) + ); } private async persistPlaylistLocks( playlistId: string, locks: ParentalLockPlaylistLocks, - options: { publish?: boolean } = {} + options: { publish?: boolean; authorize?: () => boolean } = {} ): Promise { // Every public mutation runs `ensureReadable()` before it builds its // edit. Here only the STORE must be known: the index of the @@ -534,16 +528,18 @@ export class ParentalLockLockStore { if (this.unreadable()) { return false; } - const normalized = normalizeParentalLockPlaylistLocks(locks); - const next: ParentalLockStore = { ...this.locks() }; - if (isParentalLockPlaylistLocksEmpty(normalized)) { - delete next[playlistId]; - } else { - next[playlistId] = normalized; - } + const next = withPlaylistLocksInStore(this.locks(), playlistId, locks); if (!(await this.storage.writeLocks(next))) { return false; } + 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.'); + } + return false; + } this.locks.set(next); if (options.publish !== false) { this.revisionState.update((value) => value + 1); 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 new file mode 100644 index 000000000..b4a08a053 --- /dev/null +++ b/libs/services/src/lib/parental-lock/parental-lock-stale-index.ts @@ -0,0 +1,34 @@ +import { computed, signal } from '@angular/core'; + +/** + * Playlists whose SQLite `categories.locked` index may lag behind the lock + * store: a re-stamp in progress, or one that failed (with its rollback) and + * is retried on the next store access. The lock store is not `readable` + * while any playlist is listed. + */ +export class ParentalLockStaleIndex { + private readonly playlists = new Set(); + private readonly count = signal(0); + + readonly isEmpty = computed(() => this.count() === 0); + + ids(): string[] { + return [...this.playlists]; + } + + mark(playlistId: string): void { + this.playlists.add(playlistId); + this.count.set(this.playlists.size); + } + + unmark(...playlistIds: string[]): void { + for (const playlistId of playlistIds) { + this.playlists.delete(playlistId); + } + this.count.set(this.playlists.size); + } + + clear(): void { + this.unmark(...this.playlists); + } +} diff --git a/libs/services/src/lib/parental-lock/parental-lock-store.util.ts b/libs/services/src/lib/parental-lock/parental-lock-store.util.ts index 951ee0674..7265dbed8 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-store.util.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-store.util.ts @@ -1,6 +1,10 @@ import { + isParentalLockPlaylistLocksEmpty, + lockedXtreamCategoryIds, + normalizeParentalLockPlaylistLocks, ParentalLockPlaylistLocks, ParentalLockStalkerCategoryType, + ParentalLockStore, ParentalLockXtreamCategoryType, } from '@iptvnator/shared/interfaces'; @@ -79,17 +83,65 @@ export function removesParentalLocks( } /** - * Whether an edit from `previous` to `next` may commit: adding locks always - * may, removing one only while `mayRemove()` says so (logged when refused). + * Whether an edit from `previous` to `next` may commit, asked each time the + * returned function is called: adding locks always may, removing one only + * while `mayRemove()` says so (logged when refused). Asked before a write + * and again at its durable commit points, since a relock can land while + * the write is in flight. */ -export function isLockRemovalAllowed( +export function lockRemovalGate( previous: ParentalLockPlaylistLocks, next: ParentalLockPlaylistLocks, mayRemove: () => boolean -): boolean { - if (!removesParentalLocks(previous, next) || mayRemove()) { - return true; - } - console.warn('A parental lock edit was refused: the app is locked.'); - return false; +): () => boolean { + const removes = removesParentalLocks(previous, next); + return () => { + if (!removes || mayRemove()) { + return true; + } + console.warn('A parental lock edit was refused: the app is locked.'); + return false; + }; +} + +/** `store` with one playlist's locks replaced; an empty set drops its key. */ +export function withPlaylistLocksInStore( + store: ParentalLockStore, + playlistId: string, + locks: ParentalLockPlaylistLocks +): ParentalLockStore { + const normalized = normalizeParentalLockPlaylistLocks(locks); + const next: ParentalLockStore = { ...store }; + if (isParentalLockPlaylistLocksEmpty(normalized)) { + delete next[playlistId]; + } else { + next[playlistId] = normalized; + } + return next; +} + +/** + * Stamps the SQLite `categories.locked` index of one playlist from `locks`, + * type by type. Every type is attempted; false when any of them failed. + */ +export async function stampXtreamIndex( + setCategoryLocks: ( + playlistId: string, + categoryType: ParentalLockXtreamCategoryType, + lockedXtreamIds: number[] + ) => Promise, + playlistId: string, + categoryTypes: readonly ParentalLockXtreamCategoryType[], + locks: ParentalLockPlaylistLocks +): Promise { + let success = true; + for (const categoryType of categoryTypes) { + success = + (await setCategoryLocks( + playlistId, + categoryType, + lockedXtreamCategoryIds(locks, categoryType) + )) && success; + } + return success; }