fix(portals): clear a playlist's pins by playlist, not by key list

Restoring over a playlist reused the keyed clear, which caps its input at
MAX_KEYS_PER_LOOKUP to bound an IN clause. A playlist with more than eight
pinned movies therefore kept the surplus while the call still reported
success, and the restore then wrote the archive's pins on top — leaving the
union of two states, which is neither the one the user asked for.

Clearing is now a dedicated delete-by-playlist operation with no key list to
truncate, and it refuses a blank playlist id rather than deleting everything.
A failure fails the entry instead of being swallowed: `listForPlaylist`
returns `[]` on error and `clear` returns `false`, so ignoring the result made
a failed read indistinguishable from "there was nothing to clear".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-29 09:03:40 +02:00
1 parent 3fa7007592
commit e0ebbeafed
14 files changed
+162 -9

No files matched your search

@@ -418,6 +418,12 @@ export const dbPreloadCases: PreloadInvokeCase[] = [
channel: 'DB_LIST_VOD_SOURCE_PINS',
forwardedArgs: ['playlist-1'],
},
{
method: 'dbClearVodSourcePinsForPlaylist',
args: ['playlist-1'],
channel: 'DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST',
forwardedArgs: ['playlist-1'],
},
{
method: 'dbSetVodSourcePin',
args: [vodSourcePin, ['title:dune:']],
@@ -903,6 +903,8 @@ const electronApi: ElectronBridgeApi = {
ipcRenderer.invoke('DB_GET_VOD_SOURCE_PIN', matchKeys),
dbListVodSourcePins: (playlistId: string) =>
ipcRenderer.invoke('DB_LIST_VOD_SOURCE_PINS', playlistId),
dbClearVodSourcePinsForPlaylist: (playlistId: string) =>
ipcRenderer.invoke('DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST', playlistId),
dbSetVodSourcePin: (pin: VodSourcePin, retireKeys?: string[]) =>
ipcRenderer.invoke('DB_SET_VOD_SOURCE_PIN', pin, retireKeys ?? []),
dbClearVodSourcePin: (matchKeys: string[]) =>
@@ -13,6 +13,7 @@ import {
clearVodSourcePin,
getVodSourcePin,
setVodSourcePin,
clearVodSourcePinsForPlaylist,
} from './vod-source-pin.operations';
const tmdbRow = {
@@ -245,4 +246,30 @@ describe('vod-source-pin.operations', () => {
);
});
});
describe('clearVodSourcePinsForPlaylist', () => {
it('deletes by playlist, with no key cap to truncate', async () => {
const { db, deleteFn, deleteWhere } = createDbMock();
await expect(
clearVodSourcePinsForPlaylist(db, 'playlist-1')
).resolves.toEqual({ success: true });
// The keyed clear caps its IN list at MAX_KEYS_PER_LOOKUP, so a
// playlist with more pinned movies than that kept the surplus —
// and still reported success.
expect(deleteFn).toHaveBeenCalledWith(schema.vodSourcePins);
expect(deleteWhere).toHaveBeenCalled();
});
it('refuses a blank playlist id rather than clearing everything', async () => {
const { db, deleteFn } = createDbMock();
await expect(
clearVodSourcePinsForPlaylist(db, '')
).resolves.toEqual({ success: false });
expect(deleteFn).not.toHaveBeenCalled();
});
});
});
@@ -148,6 +148,29 @@ export async function clearVodSourcePin(
return { success: true };
}
/**
* Drop every pin pointing at one playlist.
*
* NOT expressible through `clearVodSourcePin`: that takes match keys and caps
* them at `MAX_KEYS_PER_LOOKUP` to bound an IN list, so a playlist with more
* pinned movies than the cap would have had the rest silently survive — while
* the call still reported success.
*/
export async function clearVodSourcePinsForPlaylist(
db: AppDatabase,
playlistId: string
): Promise<{ success: boolean }> {
if (typeof playlistId !== 'string' || playlistId === '') {
return { success: false };
}
await db
.delete(schema.vodSourcePins)
.where(eq(schema.vodSourcePins.playlistId, playlistId));
return { success: true };
}
function toPin(row: schema.VodSourcePinRow): VodSourcePin {
return {
matchKey: row.matchKey,
@@ -20,6 +20,11 @@ handleWorkerRequest('DB_LIST_VOD_SOURCE_PINS', (playlistId: string) => ({
playlistId,
}));
handleWorkerRequest(
'DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST',
(playlistId: string) => ({ playlistId })
);
handleWorkerRequest(
'DB_SET_VOD_SOURCE_PIN',
(pin: VodSourcePin, retireKeys: string[] = []) => ({ pin, retireKeys })
@@ -365,6 +365,11 @@ export const workerIpcContractCases: WorkerIpcContractCase[] = [
args: ['playlist-1'],
payload: { playlistId: 'playlist-1' },
},
{
operation: 'DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST',
args: ['playlist-1'],
payload: { playlistId: 'playlist-1' },
},
{
operation: 'DB_SET_VOD_SOURCE_PIN',
args: [vodSourcePin, ['title:dune:']],
@@ -59,6 +59,7 @@ export const DB_WORKER_OPERATIONS = [
'DB_FIND_TITLE_SOURCES',
'DB_GET_VOD_SOURCE_PIN',
'DB_LIST_VOD_SOURCE_PINS',
'DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST',
'DB_SET_VOD_SOURCE_PIN',
'DB_CLEAR_VOD_SOURCE_PIN',
] as const;
@@ -94,6 +94,7 @@ import {
import {
clearVodSourcePin,
getVodSourcePin,
clearVodSourcePinsForPlaylist,
listVodSourcePinsForPlaylist,
setVodSourcePin,
} from '../database/operations/vod-source-pin.operations';
@@ -805,6 +806,11 @@ async function executeRequest(
return getVodSourcePin(db, payload.matchKeys);
}
case 'DB_CLEAR_VOD_SOURCE_PINS_FOR_PLAYLIST': {
const payload = message.payload as { playlistId: string };
return clearVodSourcePinsForPlaylist(db, payload.playlistId);
}
case 'DB_LIST_VOD_SOURCE_PINS': {
const payload = message.payload as { playlistId: string };
return listVodSourcePinsForPlaylist(db, payload.playlistId);
@@ -162,6 +162,12 @@ worker IPC boundary in the snake_case wire shape declared by
`XCategoryFromDb`/`XtreamCategoryFromDb`; the category operations project
their Drizzle rows explicitly to keep that contract true.
Clearing the playlist's existing pins goes through a dedicated
delete-by-playlist operation, not the keyed clear: that one caps its key list
to bound an IN clause, so a playlist with more pinned movies than the cap kept
the surplus while still reporting success. A failure now fails the entry
rather than leaving the union of old and archived pins.
`sourcePins` (VOD multi-source) is the one **optional** collection, and the
normalizer preserves that: an absent field stays absent rather than becoming
`[]`, because restore treats a PRESENT collection as authoritative and clears
@@ -68,6 +68,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
listForPlaylist: jest.fn().mockResolvedValue([]),
set: setPin,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
},
});
@@ -128,7 +129,8 @@ describe('PlaylistBackupService Xtream source pins', () => {
},
]),
set: jest.fn().mockResolvedValue(true),
clear: clearPins,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: clearPins,
},
});
@@ -137,7 +139,32 @@ describe('PlaylistBackupService Xtream source pins', () => {
// archive deliberately does not contain.
await service.importBackup(JSON.stringify(createXtreamManifest([], [])));
expect(clearPins).toHaveBeenCalledWith(['tmdb:999']);
// By playlist, not by key list: the keyed clear caps its input and
// would have left the surplus behind while reporting success.
expect(clearPins).toHaveBeenCalledWith('xtream-1');
});
it('fails the entry when the existing pins cannot be cleared', async () => {
const collaborators = createRestoreCollaborators();
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
set: jest.fn().mockResolvedValue(true),
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(false),
},
});
const summary = await service.importBackup(
JSON.stringify(createXtreamManifest([], []))
);
// Writing the archive's pins over pins that are still there produces a
// union of both, which is neither state the user asked for.
expect(summary).toEqual(
expect.objectContaining({ merged: 0, failed: 1 })
);
});
it('leaves pins alone for an archive that has no opinion', async () => {
@@ -157,7 +184,8 @@ describe('PlaylistBackupService Xtream source pins', () => {
},
]),
set: jest.fn().mockResolvedValue(true),
clear: clearPins,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: clearPins,
},
});
@@ -177,6 +205,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
listForPlaylist: jest.fn().mockResolvedValue([]),
set: setPin,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
},
});
@@ -66,6 +66,7 @@ export function createPlaylistBackupService(
listForPlaylist: jest.fn().mockResolvedValue([]),
set: jest.fn().mockResolvedValue(true),
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
},
pendingRestoreService: {
set: jest.fn(),
@@ -258,6 +259,12 @@ export function createStatefulBackupCollaborators(
);
return true;
},
clearForPlaylist: async (playlistId: string) => {
state.sourcePins = state.sourcePins.filter(
(pin) => pin.playlistId !== playlistId
);
return true;
},
},
pendingRestoreService: new XtreamPendingRestoreService(),
};
@@ -774,13 +774,19 @@ export class PlaylistBackupService {
});
}
/** Drops every pin currently pointing at this playlist. */
/**
* Drops every pin currently pointing at this playlist.
*
* One statement rather than list-then-clear-by-key: the keyed clear caps
* its input, so a playlist with more pinned movies than the cap kept the
* surplus while reporting success — and a failed list read silently
* became "there was nothing to clear".
*/
private async clearPinsForPlaylist(playlistId: string): Promise<void> {
const existing =
await this.vodSourcePinService.listForPlaylist(playlistId);
const matchKeys = existing.map((pin) => pin.matchKey).filter(Boolean);
if (matchKeys.length > 0) {
await this.vodSourcePinService.clear(matchKeys);
if (!(await this.vodSourcePinService.clearForPlaylist(playlistId))) {
throw new PlaylistBackupError(
`Clearing the existing pinned sources for "${playlistId}" failed.`
);
}
}
@@ -41,6 +41,32 @@ export class VodSourcePinService {
}
}
/**
* Drop every pin pointing at this playlist, in one statement.
*
* `clear()` caps its key list, so clearing a playlist through it would
* silently leave the surplus behind while reporting success.
*/
async clearForPlaylist(playlistId: string): Promise<boolean> {
if (!this.isAvailable || !playlistId) {
return false;
}
try {
const result =
await window.electron.dbClearVodSourcePinsForPlaylist(
playlistId
);
return result?.success === true;
} catch (error) {
console.warn(
'Clearing pinned VOD sources failed:',
redactSensitiveData(error)
);
return false;
}
}
/** Every pin pointing at this playlist. Used by playlist backup. */
async listForPlaylist(playlistId: string): Promise<VodSourcePin[]> {
if (!this.isAvailable || !playlistId) {
@@ -909,6 +909,10 @@ export interface ElectronBridgeApi {
dbGetVodSourcePin: (matchKeys: string[]) => Promise<VodSourcePin | null>;
/** Every pin pointing at this playlist — used by playlist backup. */
dbListVodSourcePins: (playlistId: string) => Promise<VodSourcePin[]>;
/** Bulk clear: not keyed, so no `MAX_KEYS_PER_LOOKUP` truncation. */
dbClearVodSourcePinsForPlaylist: (
playlistId: string
) => Promise<ElectronBridgeResult>;
/** `retireKeys` are removed in the SAME transaction as the write. */
dbSetVodSourcePin: (
pin: VodSourcePin,