fix(portals): a failed pin read must not export as "no pins"

Three findings, all in code from this session.

Backup called the lenient `listForPlaylist`, which turns a failed read
into `[]`. Since `e0ebbeaf` made restore treat `sourcePins` as
authoritative — clearing the playlist's pins before applying it — an
export whose read failed produced a file that looks complete and wipes
every pin on restore. Losing them is bad; losing them through the one
feature meant to protect them is worse. Backup now uses a strict listing
that throws, so the export fails instead.

The diacritic map stopped at U+024F, which is tidy and leaves Vietnamese
out: `ố` is U+1ED1, the normalizer folds it to `o`, and the scan filtered
those rows out before confirmation. Latin Extended Additional is included
now; the filter decides what belongs, so the range only has to be wide.

And two panels spelling one language differently (`eng` vs `en`, or
`en-US`) raised a dub warning between identical tracks. Tags are
canonicalized before comparison — 639-2 collapses to 639-1, both German
forms meet at `de`, regions drop, and `und` becomes nothing. Anything
that survives longer than three characters is not a language code, so the
comparison is declined rather than guessed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-29 21:10:53 +02:00
1 parent 960a33940f
commit 76804ee1bc
10 files changed
+267 -38

No files matched your search

@@ -135,6 +135,19 @@ describeWithSqlite('caseInsensitiveGlobPattern against SQLite', () => {
expect(sqliteGlob('Ca', pattern)).toBe(true);
});
it('reaches beyond Latin Extended-B for the fold', () => {
// "Bố" (U+1ED1) is Vietnamese and normalizes to "bo" like any other
// accented o. A range that stops at Latin Extended-B looks tidy and
// leaves a whole language filtered out before confirmation.
const pattern = caseInsensitiveGlobPattern('bo', {
foldDiacritics: true,
}) as string;
expect(sqliteGlob('Bố', pattern)).toBe(true);
expect(sqliteGlob('bồ', pattern)).toBe(true);
expect(sqliteGlob('Bo', pattern)).toBe(true);
});
it('does not fold diacritics unless asked', () => {
const pattern = caseInsensitiveGlobPattern('ca') as string;
@@ -25,23 +25,39 @@ const GLOB_METACHARACTERS = /[*?[\]^-]/;
*/
const ACCENTED_BY_BASE = ((): Map<string, string[]> => {
const byBase = new Map<string, string[]>();
// Latin-1 Supplement through Latin Extended-B, which is where the accented
// forms of ASCII letters live.
for (let codePoint = 0xc0; codePoint <= 0x24f; codePoint += 1) {
const character = String.fromCodePoint(codePoint);
const base = character.normalize('NFD').replace(/\p{M}/gu, '');
if (base.length !== 1 || !/[a-z]/i.test(base)) {
continue;
// Latin-1 Supplement through Latin Extended-B, PLUS Latin Extended
// Additional — which is where Vietnamese lives (ố is U+1ED1). Stopping at
// Latin Extended-B looks tidy and silently leaves a whole language out,
// while the normalizer folds it perfectly well. The filter below decides
// what actually belongs, so a range only has to be wide enough.
const ranges: ReadonlyArray<[number, number]> = [
[0x00c0, 0x024f],
[0x1e00, 0x1eff],
];
for (const [first, last] of ranges) {
for (let codePoint = first; codePoint <= last; codePoint += 1) {
addAccentedForm(byBase, String.fromCodePoint(codePoint));
}
const key = base.toLowerCase();
const forms = byBase.get(key) ?? [];
forms.push(character);
byBase.set(key, forms);
}
return byBase;
})();
/** Files one character under the plain ASCII letter it decomposes to. */
function addAccentedForm(
byBase: Map<string, string[]>,
character: string
): void {
const base = character.normalize('NFD').replace(/\p{M}/gu, '');
if (base.length !== 1 || !/[a-z]/i.test(base)) {
return;
}
const key = base.toLowerCase();
const forms = byBase.get(key) ?? [];
forms.push(character);
byBase.set(key, forms);
}
/**
* A `*…*` containment pattern matching `token` in any case, or `null` when the
* token cannot be expressed safely.
@@ -164,6 +164,33 @@ describe('applyApiMetadata', () => {
).toEqual({ value: '576p', provenance: 'api' });
});
it.each([
['eng', 'en'],
['en', 'en'],
['en-US', 'en'],
// Both ISO 639-2 spellings of German have to land together.
['ger', 'de'],
['deu', 'de'],
// No two-letter code exists for these, so three is canonical.
['fil', 'fil'],
])('canonicalizes the language tag %s', (raw, expected) => {
expect(
applyApiMetadata(candidate(), { audioLanguage: raw }).audioLanguage
).toEqual({ value: expected, provenance: 'api' });
});
it.each([
// ffprobe's marker for "we do not know".
['und'],
['Russian'],
['en_US'],
[''],
])('states no language for %s', (raw) => {
expect(
applyApiMetadata(candidate(), { audioLanguage: raw }).audioLanguage
).toBeUndefined();
});
it('refuses a wide frame taller than the format its width names', () => {
// 1440x1080 is anamorphic 1080 and 1600x900 is 900p. Both fall in the
// 1200-1699 band, so both were published as "720p" — with `api`
@@ -319,6 +346,20 @@ describe('audioDiffersFactually', () => {
expect(audioDiffersFactually(null, known)).toBe(false);
});
it('stays silent when two panels spell one language differently', () => {
// ffprobe says `eng`, a panel hoists `en`, and both mean English. A
// literal comparison turns that into a dub change on every switch
// between those two portals.
const from = candidate(
applyApiMetadata(candidate(), { audioLanguage: 'eng' })
);
const to = candidate(
applyApiMetadata(candidate(), { audioLanguage: 'en-US' })
);
expect(audioDiffersFactually(from, to)).toBe(false);
});
it('stays silent when both sides state the same track', () => {
const from = candidate({
audioLanguage: { value: 'rus', provenance: 'api' },
@@ -144,12 +144,9 @@ export function applyApiMetadata(
next.audio = { value: audio, provenance: 'api' };
}
const language = cleanString(api.audioLanguage);
const language = canonicalLanguage(api.audioLanguage);
if (language) {
next.audioLanguage = {
value: language.toLowerCase(),
provenance: 'api',
};
next.audioLanguage = { value: language, provenance: 'api' };
}
const quality = qualityFromDimensions(api.width, api.height);
@@ -210,6 +207,52 @@ function keepFactual(field?: VodSourceField): VodSourceField | undefined {
return field && field.provenance !== 'parsed' ? field : undefined;
}
/**
* A language tag reduced to the one form two panels can be compared on.
*
* The same spoken language is written several ways in the wild — ffprobe emits
* ISO 639-2 (`eng`, and `ger` or `deu` for German), panels hoist 639-1 (`en`),
* and either may carry a region (`en-US`). Comparing those literally makes a
* dub "change" out of two spellings of English.
*
* `Intl.Locale` does the canonicalizing: 639-2 collapses to 639-1 where one
* exists, both German forms land on `de`, and regions drop away. `und` —
* ffprobe's marker for undetermined — canonicalizes to nothing, which is
* exactly right: it means the provider does not know either.
*
* Anything that survives with more than three characters is not a language
* code (`Russian` parses as the subtag `russian`), so the comparison is
* declined rather than guessed at.
*/
function canonicalLanguage(raw: string | null | undefined): string | null {
const value = cleanString(raw);
if (!value || !LocaleCtor) {
return null;
}
try {
const language = new LocaleCtor(value).language;
return language && language.length <= 3 ? language : null;
} catch {
// Not a well-formed tag at all; saying nothing beats comparing junk.
return null;
}
}
/**
* `Intl.Locale` is ES2020 and this workspace compiles against the es2018 lib,
* so it is reached through a narrow shim rather than by moving the target.
*
* Absent — which no supported runtime actually is — the comparison is declined
* rather than half-canonicalized, since `eng` measured against `en` by a
* partial fold is exactly the false warning this function exists to prevent.
*/
const LocaleCtor = (
Intl as unknown as {
Locale?: new (tag: string) => { language?: string };
}
).Locale;
function cleanString(raw: string | null | undefined): string | null {
const trimmed = typeof raw === 'string' ? raw.trim() : '';
return trimmed === '' ? null : trimmed;
@@ -28,6 +28,7 @@ function alternative(language: string): VodSourceCandidate {
rawTitle: 'Dune',
matchConfidence: 'exact',
year: null,
// Already canonical, as `applyApiMetadata` would have produced.
audioLanguage: { value: language, provenance: 'api' },
} as VodSourceCandidate;
}
@@ -80,8 +81,8 @@ describe('currentSourceRow', () => {
// route row made the warning structurally unreachable on the
// commonest switch there is — route to alternative. It only ever
// fired between two alternatives that had both been resolved.
expect(audioDiffersFactually(row, alternative('eng'))).toBe(true);
expect(audioDiffersFactually(row, alternative('rus'))).toBe(false);
expect(audioDiffersFactually(row, alternative('en'))).toBe(true);
expect(audioDiffersFactually(row, alternative('ru'))).toBe(false);
});
it('does not call a codec change a dub change', () => {
@@ -97,6 +98,6 @@ describe('currentSourceRow', () => {
// and warning anyway trains people to ignore the one that matters.
expect(row.audio?.value).toBe('ac3');
expect(row.audioLanguage).toBeUndefined();
expect(audioDiffersFactually(row, alternative('eng'))).toBe(false);
expect(audioDiffersFactually(row, alternative('en'))).toBe(false);
});
});
@@ -25,13 +25,34 @@ describe('PlaylistBackupService Xtream source pins', () => {
localStorage.clear();
});
it('refuses to export a backup whose pin read failed', async () => {
const collaborators = createRestoreCollaborators();
const service = createPlaylistBackupService({
playlistsService: collaborators.playlistsService,
databaseService: collaborators.databaseService,
vodSourcePinService: {
listForPlaylistOrThrow: jest
.fn()
.mockRejectedValue(new Error('database is locked')),
set: jest.fn().mockResolvedValue(true),
},
});
// `sourcePins` is authoritative and restore CLEARS the playlist's pins
// before applying it, so a read that failed and reported "none" would
// produce a file that looks complete and wipes every pin the user had
// — through the one feature meant to protect them. Failing the export
// is the only honest outcome.
await expect(service.exportBackup()).rejects.toThrow();
});
it('exports the pins that point at this playlist', async () => {
const collaborators = createRestoreCollaborators();
const service = createPlaylistBackupService({
playlistsService: collaborators.playlistsService,
databaseService: collaborators.databaseService,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([
listForPlaylistOrThrow: jest.fn().mockResolvedValue([
{
matchKey: 'tmdb:603',
playlistId: 'xtream-1',
@@ -65,7 +86,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
listForPlaylistOrThrow: jest.fn().mockResolvedValue([]),
set: setPin,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
@@ -94,7 +115,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
listForPlaylistOrThrow: jest.fn().mockResolvedValue([]),
// `set` reports failure rather than throwing, so ignoring the
// result would drop the preference while the summary claims
// the import succeeded.
@@ -120,7 +141,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([
listForPlaylistOrThrow: jest.fn().mockResolvedValue([
{
matchKey: 'tmdb:999',
playlistId: 'xtream-1',
@@ -149,7 +170,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
listForPlaylistOrThrow: jest.fn().mockResolvedValue([]),
set: jest.fn().mockResolvedValue(true),
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(false),
@@ -175,7 +196,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
vodSourcePinService: {
// A pin EXISTS, or an empty list would make this pass whether
// or not the clear was skipped.
listForPlaylist: jest.fn().mockResolvedValue([
listForPlaylistOrThrow: jest.fn().mockResolvedValue([
{
matchKey: 'tmdb:999',
playlistId: 'xtream-1',
@@ -202,7 +223,7 @@ describe('PlaylistBackupService Xtream source pins', () => {
const service = createPlaylistBackupService({
...collaborators,
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
listForPlaylistOrThrow: jest.fn().mockResolvedValue([]),
set: setPin,
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
@@ -63,7 +63,7 @@ export function createPlaylistBackupService(
savePlaybackPosition: jest.fn().mockResolvedValue(undefined),
},
vodSourcePinService: {
listForPlaylist: jest.fn().mockResolvedValue([]),
listForPlaylistOrThrow: jest.fn().mockResolvedValue([]),
set: jest.fn().mockResolvedValue(true),
clear: jest.fn().mockResolvedValue(true),
clearForPlaylist: jest.fn().mockResolvedValue(true),
@@ -240,7 +240,7 @@ export function createStatefulBackupCollaborators(
},
},
vodSourcePinService: {
listForPlaylist: async (playlistId: string) =>
listForPlaylistOrThrow: async (playlistId: string) =>
state.sourcePins
.filter((pin) => pin.playlistId === playlistId)
.map((pin) => ({ ...pin })),
@@ -274,7 +274,11 @@ export class PlaylistBackupService {
this.databaseService.getFavorites(playlist._id),
this.databaseService.getRecentItems(playlist._id),
this.playbackPositionService.getAllPlaybackPositions(playlist._id),
this.vodSourcePinService.listForPlaylist(playlist._id),
// Throws rather than reading a failure as "no pins": restore
// treats this collection as authoritative and clears the
// playlist's pins before applying it, so an empty list born of a
// failed read would wipe them.
this.vodSourcePinService.listForPlaylistOrThrow(playlist._id),
]);
return {
@@ -0,0 +1,71 @@
import { VodSourcePinService } from './vod-source-pin.service';
/**
* The two listing methods differ in ONE way that matters: what a failed read
* means. Backup depends on that difference — its `sourcePins` collection is
* authoritative and restore clears the playlist's pins before applying it, so
* a failure reported as "no pins" writes a backup that silently wipes them.
*/
const electronWindow = window as unknown as {
electron?: Record<string, unknown>;
};
function withBridge(dbListVodSourcePins: unknown) {
electronWindow.electron = {
// `isAvailable` probes for this one.
dbGetVodSourcePin: jest.fn(),
dbListVodSourcePins,
};
}
describe('VodSourcePinService listing', () => {
let service: VodSourcePinService;
let warn: jest.SpyInstance;
beforeEach(() => {
service = new VodSourcePinService();
warn = jest.spyOn(console, 'warn').mockImplementation(() => undefined);
});
afterEach(() => {
delete electronWindow.electron;
warn.mockRestore();
});
it('returns the pins the bridge reports', async () => {
const pin = {
matchKey: 'tmdb:603',
playlistId: 'p1',
contentId: 501,
portalType: 'xtream' as const,
};
withBridge(jest.fn().mockResolvedValue([pin]));
await expect(service.listForPlaylist('p1')).resolves.toEqual([pin]);
await expect(service.listForPlaylistOrThrow('p1')).resolves.toEqual([
pin,
]);
});
it('reads a failure as "no pins" only for the lenient caller', async () => {
withBridge(
jest.fn().mockRejectedValue(new Error('database is locked'))
);
await expect(service.listForPlaylist('p1')).resolves.toEqual([]);
// The whole point of the second method: backup must be able to tell a
// failed read from an empty one, or it exports a wipe.
await expect(service.listForPlaylistOrThrow('p1')).rejects.toThrow(
'database is locked'
);
});
it('reports no pins, without throwing, outside Electron', async () => {
delete electronWindow.electron;
// Not a failure — there is no pin store in the PWA at all, so an empty
// list is the true answer and a backup built there is complete.
await expect(service.listForPlaylistOrThrow('p1')).resolves.toEqual([]);
});
});
@@ -67,16 +67,13 @@ export class VodSourcePinService {
}
}
/** Every pin pointing at this playlist. Used by playlist backup. */
/**
* Every pin pointing at this playlist, empty when there are none OR when
* the read failed. Only for callers that treat both the same way.
*/
async listForPlaylist(playlistId: string): Promise<VodSourcePin[]> {
if (!this.isAvailable || !playlistId) {
return [];
}
try {
return (
(await window.electron.dbListVodSourcePins(playlistId)) ?? []
);
return await this.readForPlaylist(playlistId);
} catch (error) {
console.warn(
'Listing pinned VOD sources failed:',
@@ -86,6 +83,28 @@ export class VodSourcePinService {
}
}
/**
* As above, but a failed read THROWS instead of reading as "none".
*
* Backup needs this distinction and nothing else does. An export writes
* `sourcePins` as the authoritative set for the playlist, and restore
* clears whatever is there before applying it — so a read that failed and
* returned `[]` produces a backup that looks complete and silently wipes
* every pin the user had. Losing the preferences is bad; losing them via a
* file created to protect them is worse.
*/
async listForPlaylistOrThrow(playlistId: string): Promise<VodSourcePin[]> {
return this.readForPlaylist(playlistId);
}
private async readForPlaylist(playlistId: string): Promise<VodSourcePin[]> {
if (!this.isAvailable || !playlistId) {
return [];
}
return (await window.electron.dbListVodSourcePins(playlistId)) ?? [];
}
/**
* Store the pin, also under `aliasKeys`, retiring `retireKeys` — all in
* the same transaction, because a half-applied change has no honest