From 434cbf3681f460ab60c2cae5b512ba7d705cf32b Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Mon, 12 Oct 2026 23:08:35 +0200 Subject: [PATCH] fix(playlist): keep cleared optional fields cleared on desktop (#1904) Clearing an optional playlist field (User-Agent, Referrer, Origin, credentials, EPG URL lists, file path) did not persist in Electron: the codec maps an empty value to `undefined`, `ON CONFLICT DO UPDATE` skips undefined keys, and `parseAppPlaylist` prefers the stale column over the updated payload, so the old value came back on the next read. `playlistConflictUpdate` now sets every optional column the document leaves empty to NULL, so both desktop upserts (`DB_UPSERT_APP_PLAYLIST` and `DB_UPSERT_APP_PLAYLISTS`) replace the stored row. The cleared-columns list moves from the web backend's playlist repository into the shared conflict module, and the server now uses the shared version. `date_created`, `name`, `type`, `autoRefresh`, `payload` and `last_usage` are never cleared. Co-Authored-By: Claude Opus 5.5 --- .changes/playlist-clear-optional-fields.md | 6 ++ .../operations/playlist.operations.spec.ts | 87 ++++++++++++++++++- .../app/storage/server-playlist-repository.ts | 50 +---------- docs/architecture/sqlite-db-worker.md | 11 +++ .../src/lib/app-playlist-conflict.spec.ts | 80 +++++++++++++++++ .../database/src/lib/app-playlist-conflict.ts | 70 ++++++++++++--- 6 files changed, 244 insertions(+), 60 deletions(-) create mode 100644 .changes/playlist-clear-optional-fields.md create mode 100644 libs/shared/database/src/lib/app-playlist-conflict.spec.ts diff --git a/.changes/playlist-clear-optional-fields.md b/.changes/playlist-clear-optional-fields.md new file mode 100644 index 000000000..237e86795 --- /dev/null +++ b/.changes/playlist-clear-optional-fields.md @@ -0,0 +1,6 @@ +--- +type: fix +area: playlist +--- + +In the desktop app, emptying an optional playlist field such as User-Agent, Referrer, Origin, the login details, the EPG sources or the file path now stays empty after saving instead of bringing the old value back. diff --git a/apps/electron-backend/src/app/database/operations/playlist.operations.spec.ts b/apps/electron-backend/src/app/database/operations/playlist.operations.spec.ts index 3370d633e..c827d4f8f 100644 --- a/apps/electron-backend/src/app/database/operations/playlist.operations.spec.ts +++ b/apps/electron-backend/src/app/database/operations/playlist.operations.spec.ts @@ -46,7 +46,7 @@ function runPlaylistUpdateScenario(scenario: string): unknown { const { default: Database } = await import('better-sqlite3'); const { drizzle } = await import('drizzle-orm/better-sqlite3'); const schema = await import('@iptvnator/shared/database/schema'); - const { updatePlaylist, upsertAppPlaylist, upsertAppPlaylists } = await import(${JSON.stringify(operationsUrl)}); + const { getAppPlaylist, getAppPlaylistMetas, updatePlaylist, upsertAppPlaylist, upsertAppPlaylists } = await import(${JSON.stringify(operationsUrl)}); const { setPlaylistServerTimezone } = await import(${JSON.stringify(timezoneOperationsUrl)}); const { __databaseConnectionTestHooks } = await import(${JSON.stringify(connectionUrl)}); const sqlite = new Database(':memory:'); @@ -317,6 +317,91 @@ describe('playlist.operations', () => { expect(result['moved'].payload).not.toHaveProperty('serverTimezone'); }); + it('clears the optional columns a full upsert leaves empty, so a cleared field stays cleared', () => { + // The codec maps an empty value to `undefined`, which ON CONFLICT DO + // UPDATE skips; reads prefer the column, so the old value would come + // back. Covers the single and batch writers and both conflict-update + // shapes (a clockless document and one that carries its own clock). + const ids = ['single', 'batch', 'with-clock']; + const result = runPlaylistUpdateScenario(` + const source = (id, extra = {}) => ({ + _id: id, + title: id, + type: 'm3u-url', + url: 'http://example.com/list.m3u', + userAgent: 'Agent/1', + referrer: 'http://ref.example', + origin: 'http://origin.example', + username: 'demo', + password: 'secret', + filePath: '/playlists/list.m3u', + manualEpgUrls: ['http://epg.example/manual.xml'], + favorites: ['1'], + ...(id === 'with-clock' ? { serverTimezone: 'UTC' } : {}), + ...extra, + }); + const cleared = { + userAgent: '', + referrer: ' ', + origin: '', + username: '', + password: '', + filePath: undefined, + manualEpgUrls: undefined, + }; + for (const id of ${JSON.stringify(ids)}) { + await upsertAppPlaylist(db, source(id)); + } + await upsertAppPlaylist(db, source('single', cleared)); + await upsertAppPlaylists(db, [source('batch', cleared)]); + await upsertAppPlaylist(db, source('with-clock', cleared)); + const docs = {}; + for (const id of ${JSON.stringify(ids)}) { + docs[id] = await getAppPlaylist(db, id); + } + const metas = Object.fromEntries( + (await getAppPlaylistMetas(db)).map((meta) => [meta._id, meta]) + ); + const rows = Object.fromEntries( + sqlite.prepare('SELECT id, date_created FROM playlists').all() + .map((row) => [row.id, row.date_created]) + ); + process.stdout.write(JSON.stringify({ docs, metas, rows })); + `) as { + docs: Record>; + metas: Record>; + rows: Record; + }; + + for (const id of ids) { + // Metadata reads see only the columns; the full read merges the + // payload in. Both must lose every cleared field. + for (const playlist of [result.docs[id], result.metas[id]]) { + expect(playlist).toEqual( + expect.objectContaining({ + _id: id, + title: id, + url: 'http://example.com/list.m3u', + favorites: ['1'], + manualEpgUrls: [], + }) + ); + for (const field of [ + 'userAgent', + 'referrer', + 'origin', + 'username', + 'password', + 'filePath', + ]) { + expect(playlist).not.toHaveProperty(field); + } + } + expect(result.rows[id]).toEqual(expect.any(String)); + } + expect(result.docs['with-clock']['serverTimezone']).toBe('UTC'); + }); + it('loads app playlist metadata without selecting the large payload column', async () => { const from = jest.fn().mockResolvedValue([ { diff --git a/apps/web-backend/src/app/storage/server-playlist-repository.ts b/apps/web-backend/src/app/storage/server-playlist-repository.ts index 0f7f18d9d..6d27ba3c8 100644 --- a/apps/web-backend/src/app/storage/server-playlist-repository.ts +++ b/apps/web-backend/src/app/storage/server-playlist-repository.ts @@ -6,10 +6,7 @@ import { playlistConflictUpdate, } from '@iptvnator/shared/database/app-playlist'; import { appState, playlists } from '@iptvnator/shared/database/schema'; -import type { - NewPlaylist, - Playlist as PlaylistRow, -} from '@iptvnator/shared/database/schema'; +import type { Playlist as PlaylistRow } from '@iptvnator/shared/database/schema'; import { SERVER_PAYLOAD_FIELDS } from '@iptvnator/shared/interfaces'; import { metadataDocument, @@ -42,46 +39,6 @@ function metadataColumns() { const metaColumns = metadataColumns(); -/** - * Columns a whole-document upsert must rewrite even when the document no - * longer has a value: the codec maps empty values to `undefined`, which an - * `ON CONFLICT DO UPDATE` skips, so a cleared field would keep its old - * column value and `parseAppPlaylist` would bring it back. - */ -const REPLACEABLE_COLUMNS = [ - 'serverUrl', - 'username', - 'password', - 'lastUpdated', - 'userAgent', - 'origin', - 'referrer', - 'filePath', - 'epgUrls', - 'detectedEpgUrls', - 'manualEpgUrls', - 'disabledEpgUrls', - 'macAddress', - 'url', - 'portalUrl', - 'count', - 'importDate', - 'updateDate', - 'position', - 'favorites', - 'recentlyViewed', -] as const satisfies readonly (keyof NewPlaylist)[]; - -function clearedColumns(row: NewPlaylist): Partial { - const cleared: Partial> = {}; - for (const column of REPLACEABLE_COLUMNS) { - if (row[column] === undefined) { - cleared[column] = null; - } - } - return cleared as Partial; -} - /** Playlist documents in the shared `playlists` table, same rows as desktop. */ export class ServerPlaylistRepository { constructor(private readonly db: DatabaseInstance) {} @@ -146,10 +103,7 @@ export class ServerPlaylistRepository { throw new InvalidPlaylistDocumentError(); } - const update = { - ...playlistConflictUpdate(row, next), - ...clearedColumns(row), - }; + const update = playlistConflictUpdate(row, next); this.db.transaction((tx) => { tx.insert(playlists) .values(row) diff --git a/docs/architecture/sqlite-db-worker.md b/docs/architecture/sqlite-db-worker.md index 582a0b429..e0d213da2 100644 --- a/docs/architecture/sqlite-db-worker.md +++ b/docs/architecture/sqlite-db-worker.md @@ -618,6 +618,17 @@ Xtream-only row shape: 12. `DB_GET_APP_STATE` 13. `DB_SET_APP_STATE` +`DB_UPSERT_APP_PLAYLIST(S)` write whole documents: the row built by the shared +codec (`buildPlaylistRow` in `libs/shared/database`) replaces the stored one. +`playlistConflictUpdate` sets every optional column the document leaves empty +(an empty string or a missing key) to NULL instead of skipping it, because +`parseAppPlaylist` prefers a column over the payload and would otherwise bring +a cleared User-Agent, Referrer, Origin, credential, EPG list or file path back. +Only `date_created` is kept from the first insert. Callers must send the full +document (`PlaylistsService` reads, patches and writes the whole row); a +column-only change goes through `DB_UPDATE_PLAYLIST`. The self-hosted web +backend's playlist repository uses the same conflict update. + `DB_GET_APP_PLAYLIST_METAS` is the preferred path for summary surfaces such as the workspace sidebar and dashboard source rail. It selects playlist metadata columns only and deliberately skips the large `payload` column, which can diff --git a/libs/shared/database/src/lib/app-playlist-conflict.spec.ts b/libs/shared/database/src/lib/app-playlist-conflict.spec.ts new file mode 100644 index 000000000..631e06841 --- /dev/null +++ b/libs/shared/database/src/lib/app-playlist-conflict.spec.ts @@ -0,0 +1,80 @@ +import { SQL } from 'drizzle-orm'; +import { buildPlaylistRow } from './app-playlist-codec'; +import { playlistConflictUpdate } from './app-playlist-conflict'; +import type { NewPlaylist } from './schema'; + +function rowOf(document: Record): NewPlaylist { + const row = buildPlaylistRow(document); + if (!row) { + throw new Error('Expected the document to produce a row'); + } + return row; +} + +/** Every optional column of a document that only has an id, title and URL. */ +const CLEARED_COLUMNS = { + serverUrl: null, + username: null, + password: null, + lastUpdated: null, + userAgent: null, + origin: null, + referrer: null, + filePath: null, + epgUrls: null, + detectedEpgUrls: null, + manualEpgUrls: null, + disabledEpgUrls: null, + macAddress: null, + portalUrl: null, + count: null, + importDate: null, + updateDate: null, + position: null, + favorites: null, + recentlyViewed: null, +}; + +describe('playlistConflictUpdate', () => { + const document = { + _id: 'm3u-1', + title: 'Family', + url: 'https://example.test/list.m3u', + userAgent: '', + referrer: ' ', + }; + + it('sets every optional column the document leaves empty to NULL', () => { + const row = rowOf(document); + + expect(playlistConflictUpdate(row, document)).toMatchObject({ + ...CLEARED_COLUMNS, + url: 'https://example.test/list.m3u', + }); + }); + + it('never clears the creation date or the columns every row carries', () => { + const row = rowOf(document); + const update = playlistConflictUpdate(row, document); + + expect(update).not.toHaveProperty('dateCreated'); + expect(update).toMatchObject({ + id: 'm3u-1', + name: 'Family', + type: 'm3u-url', + autoRefresh: false, + lastUsage: row.lastUsage, + }); + // A clockless document keeps a stored panel clock through SQL. + expect(update.payload).toBeInstanceOf(SQL); + }); + + it('clears the same columns when the document carries its own clock', () => { + const withClock = { ...document, serverTimezone: 'UTC' }; + const row = rowOf(withClock); + const update = playlistConflictUpdate(row, withClock); + + expect(update).toMatchObject(CLEARED_COLUMNS); + expect(update.payload).toBe(row.payload); + }); +}); diff --git a/libs/shared/database/src/lib/app-playlist-conflict.ts b/libs/shared/database/src/lib/app-playlist-conflict.ts index e972cc81a..629b9a249 100644 --- a/libs/shared/database/src/lib/app-playlist-conflict.ts +++ b/libs/shared/database/src/lib/app-playlist-conflict.ts @@ -14,22 +14,70 @@ export type PlaylistConflictUpdate = { const stored = schema.playlists; /** - * The `DO UPDATE` half of an app-playlist upsert. A full upsert is built - * from a snapshot the caller read earlier; when that snapshot carries no - * panel clock while the stored row does — `setPlaylistServerTimezone` - * landed in between — the stored clock is carried over as long as the row - * still points at the same panel, so a favorites, recent-items or metadata - * write cannot hand a clockless payload back to the past. A snapshot that - * carries its own clock, or moves the source, wins as is. Nested CASE, not - * AND: SQLite may reorder AND terms, and the json_* readers raise on a - * malformed payload unless json_valid ran first. + * Columns a whole-document upsert must rewrite even when the document no + * longer has a value: the codec maps empty values to `undefined`, which an + * `ON CONFLICT DO UPDATE` skips, so a cleared field would keep its old + * column value and `parseAppPlaylist` would bring it back. `dateCreated` + * belongs to the first insert, and `name`, `type`, `autoRefresh`, + * `payload` and `lastUsage` always carry a value. + */ +const REPLACEABLE_COLUMNS = [ + 'serverUrl', + 'username', + 'password', + 'lastUpdated', + 'userAgent', + 'origin', + 'referrer', + 'filePath', + 'epgUrls', + 'detectedEpgUrls', + 'manualEpgUrls', + 'disabledEpgUrls', + 'macAddress', + 'url', + 'portalUrl', + 'count', + 'importDate', + 'updateDate', + 'position', + 'favorites', + 'recentlyViewed', +] as const satisfies readonly (keyof schema.NewPlaylist)[]; + +function withClearedColumns(row: schema.NewPlaylist): schema.NewPlaylist { + const cleared: Partial> = + {}; + for (const column of REPLACEABLE_COLUMNS) { + if (row[column] === undefined) { + cleared[column] = null; + } + } + return { ...row, ...cleared }; +} + +/** + * The `DO UPDATE` half of an app-playlist upsert. The document replaces + * the stored one, so every optional column it leaves empty is set to NULL + * rather than skipped (see REPLACEABLE_COLUMNS). + * + * A full upsert is built from a snapshot the caller read earlier; when + * that snapshot carries no panel clock while the stored row does — + * `setPlaylistServerTimezone` landed in between — the stored clock is + * carried over as long as the row still points at the same panel, so a + * favorites, recent-items or metadata write cannot hand a clockless + * payload back to the past. A snapshot that carries its own clock, or + * moves the source, wins as is. Nested CASE, not AND: SQLite may reorder + * AND terms, and the json_* readers raise on a malformed payload unless + * json_valid ran first. */ export function playlistConflictUpdate( row: schema.NewPlaylist, playlist: Record ): PlaylistConflictUpdate { + const replacement = withClearedColumns(row); if (readString(playlist['serverTimezone'])) { - return row; + return replacement; } const excludedPayload = sql.raw(`excluded.${stored.payload.name}`); const payload = sql`CASE @@ -48,7 +96,7 @@ export function playlistConflictUpdate( END ELSE ${excludedPayload} END`; - return { ...row, payload }; + return { ...replacement, payload }; } /**