diff --git a/apps/electron-backend/src/app/api/main.preload.spec-data.ts b/apps/electron-backend/src/app/api/main.preload.spec-data.ts index 7d584856d..44c2c0d68 100644 --- a/apps/electron-backend/src/app/api/main.preload.spec-data.ts +++ b/apps/electron-backend/src/app/api/main.preload.spec-data.ts @@ -420,9 +420,11 @@ export const dbPreloadCases: PreloadInvokeCase[] = [ }, { method: 'dbSetVodSourcePin', - args: [vodSourcePin], + args: [vodSourcePin, ['title:dune:']], channel: 'DB_SET_VOD_SOURCE_PIN', - forwardedArgs: [vodSourcePin], + // The aliases to retire ride along, so the write and the retirement + // are one transaction rather than two calls that can half-apply. + forwardedArgs: [vodSourcePin, ['title:dune:']], }, { method: 'dbClearVodSourcePin', diff --git a/apps/electron-backend/src/app/api/main.preload.ts b/apps/electron-backend/src/app/api/main.preload.ts index be49ac46b..417607e41 100644 --- a/apps/electron-backend/src/app/api/main.preload.ts +++ b/apps/electron-backend/src/app/api/main.preload.ts @@ -885,8 +885,8 @@ const electronApi: ElectronBridgeApi = { ipcRenderer.invoke('DB_GET_VOD_SOURCE_PIN', matchKeys), dbListVodSourcePins: (playlistId: string) => ipcRenderer.invoke('DB_LIST_VOD_SOURCE_PINS', playlistId), - dbSetVodSourcePin: (pin: VodSourcePin) => - ipcRenderer.invoke('DB_SET_VOD_SOURCE_PIN', pin), + dbSetVodSourcePin: (pin: VodSourcePin, retireKeys?: string[]) => + ipcRenderer.invoke('DB_SET_VOD_SOURCE_PIN', pin, retireKeys ?? []), dbClearVodSourcePin: (matchKeys: string[]) => ipcRenderer.invoke('DB_CLEAR_VOD_SOURCE_PIN', matchKeys), // Playback Positions diff --git a/apps/electron-backend/src/app/database/operations/operations.test-helpers.ts b/apps/electron-backend/src/app/database/operations/operations.test-helpers.ts index 6a9c6904f..3c0ef5af8 100644 --- a/apps/electron-backend/src/app/database/operations/operations.test-helpers.ts +++ b/apps/electron-backend/src/app/database/operations/operations.test-helpers.ts @@ -109,7 +109,18 @@ export function createDbMock(selectResultsByCall: unknown[][] = []) { return query; }); - const insertValues = jest.fn().mockResolvedValue(undefined); + // Same `.run()` requirement as the update below: an insert executed inside + // a synchronous transaction callback has to be the synchronous form. + const insertRun = jest.fn(); + const insertChain: Record = { + run: insertRun, + then: ( + resolve: (value: unknown) => void, + reject: (reason: unknown) => void + ) => Promise.resolve(undefined).then(resolve, reject), + }; + insertChain.onConflictDoUpdate = jest.fn().mockReturnValue(insertChain); + const insertValues = jest.fn().mockReturnValue(insertChain); const insert = jest.fn().mockReturnValue({ values: insertValues }); // Prepared-statement writes MUST use `.run()` (synchronous) rather than @@ -138,6 +149,8 @@ export function createDbMock(selectResultsByCall: unknown[][] = []) { .fn() .mockReturnValue({ execute: deleteExecute, run: deleteRun }); const deleteResult = { + // Inside a transaction the delete must be the synchronous form too. + run: deleteRun, prepare: deletePrepare, then: ( resolve: (value: unknown) => void, @@ -170,6 +183,7 @@ export function createDbMock(selectResultsByCall: unknown[][] = []) { deleteFn, deletePrepare, deleteRun, + insertRun, deleteWhere, insert, insertValues, diff --git a/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.spec.ts b/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.spec.ts index d47924ba9..7b7bfd3de 100644 --- a/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.spec.ts +++ b/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.spec.ts @@ -33,12 +33,16 @@ const titleRow = { updatedAt: '2026-07-01T10:00:00.000Z', }; -/** `createDbMock` resolves `.values()` directly; pins upsert on top of it. */ +/** + * Pins upsert, and run inside a transaction — so the chain has to end in a + * synchronous `.run()` rather than resolving, exactly as the driver requires. + */ function createUpsertDbMock() { const mock = createDbMock(); - const onConflictDoUpdate = jest.fn().mockResolvedValue(undefined); + const insertRun = jest.fn(); + const onConflictDoUpdate = jest.fn().mockReturnValue({ run: insertRun }); mock.insertValues.mockReturnValue({ onConflictDoUpdate }); - return { ...mock, onConflictDoUpdate }; + return { ...mock, onConflictDoUpdate, insertRun }; } describe('vod-source-pin.operations', () => { @@ -162,6 +166,40 @@ describe('vod-source-pin.operations', () => { ); }); + it('retires the aliases in the same transaction as the write', async () => { + const { db, transaction, insertRun, deleteRun, deleteWhere } = + createUpsertDbMock(); + + await setVodSourcePin(db, pin, [ + 'title:the matrix:1999', + 'title:the matrix:', + ]); + + // Two calls could half-apply, and neither half-outcome has an + // honest answer: a surviving alias wins the next lookup, while a + // rolled-back-looking failure leaves the canonical row durable. + expect(transaction).toHaveBeenCalledTimes(1); + expect(insertRun).toHaveBeenCalledTimes(1); + expect(deleteRun).toHaveBeenCalledTimes(1); + expect(deleteWhere).toHaveBeenCalled(); + }); + + it('never retires the key it just wrote', async () => { + const { db, deleteRun } = createUpsertDbMock(); + + await setVodSourcePin(db, pin, ['tmdb:603']); + + expect(deleteRun).not.toHaveBeenCalled(); + }); + + it('touches nothing else when there is no alias to retire', async () => { + const { db, deleteRun } = createUpsertDbMock(); + + await setVodSourcePin(db, pin); + + expect(deleteRun).not.toHaveBeenCalled(); + }); + it('writes an ISO timestamp on both the insert and the update', async () => { const { db, insertValues, onConflictDoUpdate } = createUpsertDbMock(); diff --git a/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.ts b/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.ts index 82745a55b..77888f924 100644 --- a/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.ts +++ b/apps/electron-backend/src/app/database/operations/vod-source-pin.operations.ts @@ -71,13 +71,28 @@ export async function listVodSourcePinsForPlaylist( return rows.map(toPin); } +/** + * Store the pin and retire its old aliases as ONE unit. + * + * Split across two calls there is no honest answer to a half-failure: report + * success and a surviving alias silently wins the next lookup; report failure + * and the canonical row is already durable, so a later reopen selects the + * source the UI said was rejected. A transaction removes the question. + */ export async function setVodSourcePin( db: AppDatabase, - pin: VodSourcePin + pin: VodSourcePin, + retireKeys: string[] = [] ): Promise<{ success: boolean }> { const updatedAt = new Date().toISOString(); + const retire = (retireKeys ?? []) + .filter( + (key) => + typeof key === 'string' && key !== '' && key !== pin.matchKey + ) + .slice(0, MAX_KEYS_PER_LOOKUP); - await db + const insert = db .insert(schema.vodSourcePins) .values({ matchKey: pin.matchKey, @@ -96,6 +111,19 @@ export async function setVodSourcePin( }, }); + await db.transaction(() => { + // `.run()`, not `.execute()`: on better-sqlite3 the latter defers the + // write to a promise that never settles inside this synchronous + // callback, so the statement would be a silent no-op. + insert.run(); + + if (retire.length > 0) { + db.delete(schema.vodSourcePins) + .where(inArray(schema.vodSourcePins.matchKey, retire)) + .run(); + } + }); + return { success: true }; } diff --git a/apps/electron-backend/src/app/events/database/vod-sources.events.ts b/apps/electron-backend/src/app/events/database/vod-sources.events.ts index 5846e2504..be5d73004 100644 --- a/apps/electron-backend/src/app/events/database/vod-sources.events.ts +++ b/apps/electron-backend/src/app/events/database/vod-sources.events.ts @@ -20,7 +20,10 @@ handleWorkerRequest('DB_LIST_VOD_SOURCE_PINS', (playlistId: string) => ({ playlistId, })); -handleWorkerRequest('DB_SET_VOD_SOURCE_PIN', (pin: VodSourcePin) => ({ pin })); +handleWorkerRequest( + 'DB_SET_VOD_SOURCE_PIN', + (pin: VodSourcePin, retireKeys: string[] = []) => ({ pin, retireKeys }) +); handleWorkerRequest('DB_CLEAR_VOD_SOURCE_PIN', (matchKeys: string[]) => ({ matchKeys, diff --git a/apps/electron-backend/src/app/events/database/worker-ipc-contract.spec-data.ts b/apps/electron-backend/src/app/events/database/worker-ipc-contract.spec-data.ts index fb7eb54cc..c9a749b2b 100644 --- a/apps/electron-backend/src/app/events/database/worker-ipc-contract.spec-data.ts +++ b/apps/electron-backend/src/app/events/database/worker-ipc-contract.spec-data.ts @@ -367,8 +367,8 @@ export const workerIpcContractCases: WorkerIpcContractCase[] = [ }, { operation: 'DB_SET_VOD_SOURCE_PIN', - args: [vodSourcePin], - payload: { pin: vodSourcePin }, + args: [vodSourcePin, ['title:dune:']], + payload: { pin: vodSourcePin, retireKeys: ['title:dune:'] }, }, { operation: 'DB_CLEAR_VOD_SOURCE_PIN', diff --git a/apps/electron-backend/src/app/workers/database.worker.ts b/apps/electron-backend/src/app/workers/database.worker.ts index d0ac87095..5c7d435ff 100644 --- a/apps/electron-backend/src/app/workers/database.worker.ts +++ b/apps/electron-backend/src/app/workers/database.worker.ts @@ -811,8 +811,11 @@ async function executeRequest( } case 'DB_SET_VOD_SOURCE_PIN': { - const payload = message.payload as { pin: VodSourcePin }; - return setVodSourcePin(db, payload.pin); + const payload = message.payload as { + pin: VodSourcePin; + retireKeys?: string[]; + }; + return setVodSourcePin(db, payload.pin, payload.retireKeys ?? []); } case 'DB_CLEAR_VOD_SOURCE_PIN': { diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index c10d835c9..c7cf3e596 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -235,8 +235,11 @@ one that does not exist — the chip simply does not appear. So: silently decide which valid sources the user may see, and it would not even buy anything: the GLOB cannot use an index, so SQLite reads every row either way and a `LIMIT` saves transfer, not work. What bounds the scan instead is - its predicate — reaching it means the movie's entire title is one or two - characters, and only films carrying that exact word come back. + its predicate: reaching it means NO token cleared the trigram minimum — a + one-or-two-character title like "It", but also an all-short multiword one + like "I Am" — and EVERY token must then appear as a word. Matching on the + first token alone would return most of a large catalog for the confirmation + pass to discard, which on the single database worker is real blocking. Like the `LIKE` it replaces it compares ASCII-lowercased text, so a non-ASCII short title is no better and no worse served than before. @@ -496,6 +499,13 @@ downloads sit in their own component-provided services beside it. ## Backup +A pin is written and its old aliases retired in ONE transaction +(`setVodSourcePin(db, pin, retireKeys)`). Split in two there is no honest +outcome for a half-failure: a surviving alias is read before the canonical key +on the next open, so reporting success starts the source the user just +replaced — while reporting failure leaves the canonical row durable and the UI +showing a pin that is no longer the stored one. + Pins ride along with playlist backup, under the playlist they point at, as the optional `sourcePins` collection. See `docs/architecture/playlist-backup-restore.md` for the remapping and diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin-writes.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin-writes.spec.ts index 582a8ff9b..69ddcb9c7 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin-writes.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin-writes.spec.ts @@ -142,9 +142,11 @@ describe('VodMultiSourceHostService — pin persistence', () => { await service.togglePin(ALT_TWO.id); expect(pins.set).toHaveBeenCalledWith( - expect.objectContaining({ matchKey: 'tmdb:603' }) + expect.objectContaining({ matchKey: 'tmdb:603' }), + expect.any(Array) ); - const [retired] = pins.clear.mock.calls[0] as [string[]]; + // The aliases ride along with the write, so the two cannot half-apply. + const [, retired] = pins.set.mock.calls[0] as [unknown, string[]]; // This film's other key goes, so a reopen before enrichment cannot // read a row still pointing at the source just replaced. expect(retired).toContain('title:the matrix:1999'); @@ -161,8 +163,7 @@ describe('VodMultiSourceHostService — pin persistence', () => { await service.togglePin(ALT_TWO.id); - // Retiring first would destroy the stored preference and then fail to - // replace it: nothing persisted, while the row still shows the old pin. + // One transaction: a refused write retires nothing either. expect(pins.clear).not.toHaveBeenCalled(); expect(rowFor(ALT_TWO.id)?.isPinned).toBe(false); }); @@ -196,21 +197,19 @@ describe('VodMultiSourceHostService — pin persistence', () => { expect(rowFor(ALT_TWO.id)?.isPinned).toBe(false); }); - it('does not claim a pin whose stale alias survived', async () => { - // A tmdb id AND a title/year, so the canonical key is written and the - // title alias is left to retire — the case where a failed clear can - // actually strand a stale row. + it('retires the aliases in the same call that writes the pin', async () => { await loadMovie([ALT_TWO], { ...MOVIE_A, tmdbId: 603 }); - pins.set.mockResolvedValue(true); - // The canonical key is written, but retiring the alias fails. - pins.clear.mockResolvedValue(false); await service.togglePin(ALT_TWO.id); - // Lookups read the alias BEFORE the key just written, so reopening - // this movie before enrichment lands would start the OLD source. The - // icon must not promise otherwise. - expect(rowFor(ALT_TWO.id)?.isPinned).toBe(false); + // Previously a write followed by a clear: if the clear failed, the + // surviving alias was read BEFORE the canonical key on the next open + // and started the source the user had just replaced. Neither + // half-outcome had an honest answer, so there is only one call now. + expect(pins.set).toHaveBeenCalledTimes(1); + expect(pins.clear).not.toHaveBeenCalled(); + const [, retired] = pins.set.mock.calls[0] as [unknown, string[]]; + expect(retired).toContain('title:the matrix:1999'); }); it('keeps the pin when clearing it fails', async () => { diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts index cb26145a2..90d52ddad 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts @@ -357,7 +357,8 @@ describe('VodMultiSourceHostService — pinning', () => { await service.togglePin(ALT_TWO.id); - expect(pins.set).toHaveBeenCalledWith(pin); + // The aliases to retire travel with the write, in one transaction. + expect(pins.set).toHaveBeenCalledWith(pin, []); expect(rowFor(ALT_TWO.id)?.isPinned).toBe(true); await service.togglePin(ALT_TWO.id); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts index 119f8a11f..67348ae7e 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts @@ -94,27 +94,21 @@ export async function writePin( return false; } - // The write goes FIRST, and the old rows only go once it landed. The - // other order destroys the stored preference and then fails to replace it - // — leaving nothing persisted while the UI still shows the old pin. - // Lookups are most-trusted-first, so a leftover alias never outranks the - // key just written. - const stored = await pins.set({ - matchKey, - playlistId: candidate.playlistId, - contentId: candidate.contentId, - portalType: candidate.portalType, - }); - if (!stored) { - return false; - } - - const retire = retirablePinKeys(keys).filter((key) => key !== matchKey); - // A surviving alias is read BEFORE the canonical key on the next open, so - // a failed retire means reopening the movie picks the OLD source until - // enrichment supplies the tmdb id again. Report that rather than claiming - // the change stuck. - return retire.length === 0 || (await erasePin(pins, retire)); + // ONE transaction, not a write followed by a clear. Split in two there is + // no honest outcome for a half-failure: a surviving alias is read BEFORE + // the canonical key on the next open, so reporting success starts the + // source the user just replaced — while reporting failure leaves the + // canonical row durable and the UI showing a pin that is no longer the + // stored one. + return pins.set( + { + matchKey, + playlistId: candidate.playlistId, + contentId: candidate.contentId, + portalType: candidate.portalType, + }, + retirablePinKeys(keys).filter((key) => key !== matchKey) + ); } /** Clears every alias, so unpinning is not undone by a stale row. */ diff --git a/libs/services/src/lib/vod-source-pin.service.ts b/libs/services/src/lib/vod-source-pin.service.ts index bcb586bc3..cc23fc36d 100644 --- a/libs/services/src/lib/vod-source-pin.service.ts +++ b/libs/services/src/lib/vod-source-pin.service.ts @@ -60,13 +60,20 @@ export class VodSourcePinService { } } - async set(pin: VodSourcePin): Promise { + /** + * Store the pin, retiring `retireKeys` in the same transaction — a + * half-applied change has no honest outcome to report. + */ + async set(pin: VodSourcePin, retireKeys: string[] = []): Promise { if (!this.isAvailable) { return false; } try { - const result = await window.electron.dbSetVodSourcePin(pin); + const result = await window.electron.dbSetVodSourcePin( + pin, + retireKeys + ); return result?.success === true; } catch (error) { console.warn( diff --git a/libs/shared/interfaces/src/lib/electron-api.interface.ts b/libs/shared/interfaces/src/lib/electron-api.interface.ts index 962cec3f4..34e7b0449 100644 --- a/libs/shared/interfaces/src/lib/electron-api.interface.ts +++ b/libs/shared/interfaces/src/lib/electron-api.interface.ts @@ -882,7 +882,11 @@ export interface ElectronBridgeApi { dbGetVodSourcePin: (matchKeys: string[]) => Promise; /** Every pin pointing at this playlist — used by playlist backup. */ dbListVodSourcePins: (playlistId: string) => Promise; - dbSetVodSourcePin: (pin: VodSourcePin) => Promise; + /** `retireKeys` are removed in the SAME transaction as the write. */ + dbSetVodSourcePin: ( + pin: VodSourcePin, + retireKeys?: string[] + ) => Promise; dbClearVodSourcePin: ( matchKeys: string[] ) => Promise;