diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 0fe1aa880..2d911ea21 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -283,8 +283,9 @@ on either side. locked rows is not mistaken for a stalled portal. - **Stalker search relock:** a lock change drops the withheld rows already on screen at once (`applyRelockToResults`, page 1 included) and closes an - open detail of a now-withheld genre, before the replacement page is - awaited — otherwise they stay clickable while that request is pending. + open detail of a now-withheld genre (in fail-closed mode also one + without a genre), before the replacement page is awaited — otherwise + they stay clickable while that request is pending. - **Stalker search staleness:** portal requests are not aborted, so a search page issued before a relock can finish after it, filtered with the pre-relock withheld set; `isStalkerSearchRequestCurrent` keys the @@ -416,15 +417,13 @@ 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. 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. - 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. + another write, when the app relocks. The gate is asked right before the + edit's first write is issued; a relock that lands after that is ordered + after the write, which completes: the parent authorized that removal, + and a post-write rollback could itself fail and leave the persisted + store diverged from memory across a restart. (During the Xtream stamps + the playlist is stale anyway, so the relock reloads fail-closed until + they land.) 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/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.spec.ts b/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.spec.ts index 51b4b20c8..ea51992c5 100644 --- a/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.spec.ts +++ b/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.spec.ts @@ -29,7 +29,10 @@ import { ParentalLockService, PlaylistsService, } from '@iptvnator/services'; -import { CONNECTIVITY_GUARD_RESET } from '@iptvnator/shared/interfaces'; +import { + ALL_CATEGORIES_WITHHELD, + CONNECTIVITY_GUARD_RESET, +} from '@iptvnator/shared/interfaces'; import type { ResolvedPortalPlayback } from '@iptvnator/shared/interfaces'; import { StalkerSearchComponent } from './stalker-search.component'; @@ -492,6 +495,17 @@ describe('StalkerSearchComponent result paging', () => { expect(stalkerStoreMock.setSelectedItem).toHaveBeenLastCalledWith(null); }); + it('closes a genre-less detail on relock only in fail-closed mode', () => { + component.selectItem({ id: 'no-genre', name: 'N' }); + expect(component.itemDetails()).not.toBeNull(); + + component.closeWithheldDetail(new Set(['9'])); + expect(component.itemDetails()).not.toBeNull(); + + component.closeWithheldDetail(ALL_CATEGORIES_WITHHELD); + expect(component.itemDetails()).toBeNull(); + }); + it('keeps paging past a page whose rows were all withheld by the parental lock', () => { component.applySearchPageSuccess(1, searchItems('page1', 3), 10); expect(component.searchHasMore()).toBe(true); diff --git a/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.ts b/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.ts index 4d71c418e..a49409256 100644 --- a/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.ts +++ b/libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.ts @@ -757,13 +757,18 @@ export class StalkerSearchComponent { closeWithheldDetail(withheldCategoryIds: ReadonlySet): void { const details = this.itemDetails(); - const categoryId = details?.category_id; - if ( - !details || - categoryId === undefined || - categoryId === null || - !withheldCategoryIds.has(String(categoryId)) - ) { + if (!details) { + return; + } + // A detail without a genre is withheld only in fail-closed mode + // (`ALL_CATEGORIES_WITHHELD`): "unknown genre" is not "no locked + // genre" while the locks themselves are unknown. + const categoryId = details.category_id; + const withheld = + categoryId === undefined || categoryId === null || categoryId === '' + ? withheldCategoryIds === ALL_CATEGORIES_WITHHELD + : withheldCategoryIds.has(String(categoryId)); + if (!withheld) { return; } const cleared = clearStalkerDetailViewState(); 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 47a1d6207..dce3e6f92 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,7 +465,10 @@ 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 () => { + it('completes a removal whose write was issued before the app relocked', async () => { + // The parent authorized the removal; a relock that lands once its + // write is issued is ordered after it (no post-write rollback, which + // could itself fail and leave the persisted store diverged). storage.readLocks.mockResolvedValue({ 'pl-1': { xtream: [], stalker: [], m3u: ['Adults'] }, }); @@ -473,79 +476,15 @@ describe('ParentalLockLockStore', () => { 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('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(); - let unlocked = true; - store.setRemovalGate(() => unlocked); - jest.spyOn(console, 'warn').mockImplementation(() => undefined); - await store.setXtreamLocks('pl-1', 'live', [7, 9]); - setCategoryLocks.mockImplementationOnce(async () => { + storage.writeLocks.mockImplementationOnce(async () => { unlocked = false; return true; }); - await expect(store.setXtreamLocks('pl-1', 'live', [7])).resolves.toBe( - false - ); + await expect(store.setM3uLocks('pl-1', [])).resolves.toBe(true); - expect(store.lockedXtreamIds('pl-1', 'live')).toEqual([7, 9]); - expect(setCategoryLocks).toHaveBeenLastCalledWith( - 'pl-1', - 'live', - [7, 9] - ); + expect(store.lockedGroupTitles('pl-1')).toEqual([]); + expect(storage.writeLocks).toHaveBeenCalledTimes(1); expect(store.readable()).toBe(true); }); }); 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 5508fc8c5..73631a6fc 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 @@ -65,8 +65,10 @@ export class ParentalLockLockStore { /** * Whether an edit may REMOVE a lock right now; set by * `ParentalLockService` to "the session is not locked". Asked inside - * the write queue, at commit time: an editor opened while unlocked may - * still be saving (or queued) when the app relocks. + * the write queue right before the edit's first write: an editor opened + * while unlocked may still be saving (or queued) when the app relocks. + * A relock that lands once the write is issued is ordered after it: the + * parent authorized that removal, so it completes. */ private mayRemoveLocks: () => boolean = () => true; /** @@ -77,8 +79,8 @@ export class ParentalLockLockStore { private writeQueue: Promise = Promise.resolve(); /** * 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; + * rewritten from it: "Remove all playlists" could not clear it. Retried + * on the next store access; * the store is not `readable` meanwhile, since a restart would load the * persisted copy. */ @@ -335,10 +337,9 @@ export class ParentalLockLockStore { previous: ParentalLockPlaylistLocks, next: ParentalLockPlaylistLocks ): Promise { - const authorize = lockRemovalGate(previous, next, this.mayRemoveLocks); return ( - authorize() && - this.persistPlaylistLocks(playlistId, next, { authorize }) + lockRemovalGate(previous, next, this.mayRemoveLocks)() && + this.persistPlaylistLocks(playlistId, next) ); } @@ -377,13 +378,10 @@ export class ParentalLockLockStore { next: ParentalLockPlaylistLocks, categoryTypes: readonly ParentalLockXtreamCategoryType[] ): Promise { - const authorize = lockRemovalGate(previous, next, this.mayRemoveLocks); - if (!authorize()) { - return false; - } + const authorized = lockRemovalGate(previous, next, this.mayRemoveLocks); const normalizedNext = normalizeParentalLockPlaylistLocks(next); if (isParentalLockPlaylistLocksEmpty(normalizedNext)) { - if (!(await this.ensureReadable())) { + if (!(await this.ensureReadable()) || !authorized()) { return false; } // Stale until the STORE write has landed too: while it is @@ -399,7 +397,6 @@ export class ParentalLockLockStore { )) && (await this.persistPlaylistLocks(playlistId, normalizedNext, { publish: false, - authorize, })) ) { this.staleIndex.unmark(playlistId); @@ -418,20 +415,15 @@ export class ParentalLockLockStore { // running would read a later type through its old stamps, and a // successful stamp emits nothing afterwards to reload it. if ( + !authorized() || !(await this.persistPlaylistLocks(playlistId, next, { publish: false, - authorize, })) ) { return false; } this.markStaleWhileStamping(playlistId); - // Asked again once the stamps landed: a relock during them must not - // see the removal published. - if ( - (await this.stampXtreamLocks(playlistId, categoryTypes)) && - authorize() - ) { + if (await this.stampXtreamLocks(playlistId, categoryTypes)) { this.staleIndex.unmark(playlistId); this.revisionState.update((value) => value + 1); return true; @@ -523,7 +515,7 @@ export class ParentalLockLockStore { private async persistPlaylistLocks( playlistId: string, locks: ParentalLockPlaylistLocks, - options: { publish?: boolean; authorize?: () => boolean } = {} + options: { publish?: boolean } = {} ): Promise { // Every public mutation runs `ensureReadable()` before it builds its // edit. Here only the STORE must be known: the index of the @@ -536,12 +528,6 @@ export class ParentalLockLockStore { 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. - await this.rewritePersistedStore(); - 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-store.util.ts b/libs/services/src/lib/parental-lock/parental-lock-store.util.ts index 7265dbed8..0dd84dcea 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 @@ -83,11 +83,10 @@ export function removesParentalLocks( } /** - * 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. + * Whether an edit from `previous` to `next` may be issued: adding locks + * always may, removing one only while `mayRemove()` says so (logged when + * refused). Returned as a function so the answer is taken at the moment + * the edit's first write is issued. */ export function lockRemovalGate( previous: ParentalLockPlaylistLocks,