fix(portals): find short Unicode titles, and stop two pickers racing

Five from review.

Greptile's P1: a short non-ASCII title was undiscoverable. SQLite's `LOWER()`
and GLOB classes are ASCII-only, so "он" never matched a stored "Он" and the
source simply never appeared. ASCII tokens keep the word-boundary GLOB; a
non-ASCII token falls back to a substring test against both the folded and the
as-typed form, which the normalized confirmation afterwards makes safe.

A probe now retries the ranged GET for 400 and 403, not just 405/501 — those
are what a WAF returns for an unexpected HEAD on a URL it serves happily over
GET, and calling that source dead also ranked it below worse ones.

Three races, all the same shape as ones fixed earlier in this branch:
- a pinned play awaiting its resume lookup did not notice a source picked
  across it, and finished last, replacing the user's choice;
- two overlapping switches both saw the same external session, both awaited
  its close, and both launched — two detached players again;
- the primary button showed the ROUTE copy's Resume while the pinned copy's
  row was still loading, so a click started somewhere else entirely.

Also puts the races spec on the shared host harness, which is what keeps it
inside the file-size rule now that it carries two more cases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-29 01:04:43 +02:00
1 parent e1c150e7bb
commit 270e54a0b5
11 files changed
+230 -93

No files matched your search

@@ -95,6 +95,32 @@ describe('title-sources.operations', () => {
expect(compiledQuery(fts.all).sql).toContain('LIMIT');
});
it('can still find a short non-ASCII title', async () => {
// SQLite's LOWER() and GLOB classes are ASCII-only, so folding
// "Он" to "он" never happened and the film stayed invisible in
// the Sources chip entirely.
const scan = createDbMock([]);
await findTitleSources(scan.db, { title: 'Он' });
const scanQuery = compiledQuery(scan.all);
expect(scanQuery.sql).toContain('instr');
// Both the folded and the as-typed form, since neither alone
// matches a title stored in the other case.
expect(scanQuery.params).toContain('он');
expect(scanQuery.params).toContain('Он');
});
it('keeps the word boundary for ASCII tokens', async () => {
// The looser substring test must not leak into the ASCII path,
// where it would let "it" match "Titanic".
const scan = createDbMock([]);
await findTitleSources(scan.db, { title: 'It' });
const scanQuery = compiledQuery(scan.all);
expect(scanQuery.sql).toContain('GLOB ?');
expect(scanQuery.sql).not.toContain('instr');
});
it('requires every token of an all-short multiword title', async () => {
// "I Am" reaches the scan too — no token survives the trigram
// minimum. Matching on the first token alone returns every catalog
@@ -116,21 +116,48 @@ function excludePlaylistClause(
* ("IT (2017) 1080p").
*
* Still a necessary-not-sufficient filter — `normalizeTitleKeys` below is what
* confirms a match. 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.
* confirms a match.
*/
function scanCandidateQuery(base: string, excludePlaylist: SQL) {
/**
* One token's presence test.
*
* SQLite's `LOWER()` and GLOB character classes are ASCII-only, so a Cyrillic
* or Greek token cannot be folded or word-bounded in stock SQLite — matching
* "он" against a stored "Он" would simply fail, and the film stayed invisible
* in the Sources chip.
*
* ASCII tokens keep the word-boundary GLOB, which is what stops "it" matching
* "Titanic". A non-ASCII token falls back to a substring test tried against
* both the folded and the as-typed form: that covers a title stored in the
* same case as the request or in lower case, which is the realistic pair, and
* it deliberately does NOT claim full Unicode case folding. It is looser than
* the ASCII branch, and the normalized confirmation afterwards is what makes
* that safe — a looser filter here costs transfer, never a wrong match.
*/
function tokenPredicate(token: string, rawToken: string): SQL {
// Normalized, so an ASCII token holds letters and digits only — nothing
// GLOB would read as a metacharacter.
if (/^[a-z0-9]+$/.test(token)) {
return sql`' ' || LOWER(c.title) || ' ' GLOB ${`*[^a-z0-9]${token}[^a-z0-9]*`}`;
}
return sql`(instr(LOWER(c.title), ${token}) > 0 OR instr(c.title, ${rawToken}) > 0)`;
}
function scanCandidateQuery(
base: string,
rawTitle: string,
excludePlaylist: SQL
) {
// EVERY token has to appear, not just the first. This path is reached
// whenever no token survives FTS's minimum length, which includes
// multiword titles like "I Am" — matching on "i" alone would return most
// of the catalog and hand it all to the TypeScript pass to throw away.
const tokens = base.split(' ').filter(Boolean);
// Normalized, so they hold letters, digits and spaces only — nothing GLOB
// would read as a metacharacter.
const rawTokens = rawTitle.split(/\s+/).filter(Boolean);
const wordMatches = sql.join(
tokens.map(
(token) =>
sql`' ' || LOWER(c.title) || ' ' GLOB ${`*[^a-z0-9]${token}[^a-z0-9]*`}`
tokens.map((token, index) =>
tokenPredicate(token, rawTokens[index] ?? token)
),
sql` AND `
);
@@ -224,7 +251,7 @@ export async function findTitleSources(
ftsCandidateQuery(matchQuery, excludePlaylist)
)) as TitleSourceRow[])
: ((await db.all(
scanCandidateQuery(wanted.base, excludePlaylist)
scanCandidateQuery(wanted.base, rawTitle, excludePlaylist)
)) as TitleSourceRow[]);
} catch {
// Malformed FTS query for exotic titles — no sources, not a crash
+11
View File
@@ -478,6 +478,17 @@ session first. It REPLACES what is playing — with MPV or VLC and instance
reuse off, the backend would otherwise spawn a second detached player, leaving
both sources running and Stop owning only the newer one.
## Short titles and Unicode
SQLite's `LOWER()` and GLOB character classes are ASCII-only, so a short
non-ASCII title could not be folded or word-bounded and simply never matched —
the film stayed absent from the chip. ASCII tokens keep the word-boundary GLOB
(what stops "it" matching "Titanic"); a non-ASCII token falls back to a
substring test against both the folded and the as-typed form. That covers a
title stored in the same case as the request or in lower case, and deliberately
does not claim full Unicode case folding. It is looser, and the normalized
confirmation afterwards is what makes looser safe.
## Claims about the present
`isActive` means "the source a switch or Play would use" — selection, not
@@ -30,6 +30,7 @@ describe('VodDetailsPlaybackService — external session ownership', () => {
const addRecentItem = jest.fn();
const activeSession = signal<unknown>(null);
const closeSession = jest.fn().mockResolvedValue(undefined);
const openResolvedPlayback = jest.fn();
const activeSource = signal<PlayerContentInfo | null>(null);
function sessionFor(playlistId: string, contentXtreamId: number) {
@@ -75,7 +76,7 @@ describe('VodDetailsPlaybackService — external session ownership', () => {
provide: PORTAL_PLAYER,
useValue: {
isEmbeddedPlayer: jest.fn().mockReturnValue(false),
openResolvedPlayback: jest.fn(),
openResolvedPlayback,
},
},
{
@@ -161,6 +162,44 @@ describe('VodDetailsPlaybackService — external session ownership', () => {
expect(closeSession).toHaveBeenCalled();
});
it('launches only the newest source when two switches overlap', async () => {
activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID));
// One shared promise: both calls see the same running session, so
// both await the same close rather than each making its own.
let releaseClose: (() => void) | undefined;
const closing = new Promise<void>((resolve) => {
releaseClose = () => resolve();
});
closeSession.mockReturnValue(closing);
// These spies are module-level and never cleared between cases.
openResolvedPlayback.mockClear();
const first = service.startResolvedPlayback({
streamUrl: 'https://example.com/one.mkv',
title: 'Example Movie',
});
const second = service.startResolvedPlayback({
streamUrl: 'https://example.com/two.mkv',
title: 'Example Movie',
});
releaseClose?.();
await Promise.all([first, second]);
// Both saw the same running session and awaited its close. Launching
// both afterwards is how two detached players appear again, with the
// older one holding a source the user has already moved on from.
expect(openResolvedPlayback).toHaveBeenCalledTimes(1);
expect(openResolvedPlayback).toHaveBeenCalledWith(
expect.objectContaining({
streamUrl: 'https://example.com/two.mkv',
}),
true
);
closeSession.mockResolvedValue(undefined);
});
it('records a source started through multi-source as recently viewed', () => {
// Playing an alternative from the picker, or letting a pin decide the
// primary Play, is still watching the movie — it belongs in Recently
@@ -342,9 +342,14 @@ export class VodDetailsPlaybackService {
* template already renders through `@if`, so the player component and its
* engine survive and simply re-seek to `playback.startTime`.
*/
/** Bumped by every start; only the newest may launch after its close. */
private startGeneration = 0;
async startResolvedPlayback(
playback: ResolvedPortalPlayback
): Promise<void> {
const generation = ++this.startGeneration;
// A switch REPLACES what is playing. With MPV or VLC and instance
// reuse off, the backend spawns a second detached player otherwise —
// both sources keep running and Stop owns only the newer one.
@@ -353,6 +358,14 @@ export class VodDetailsPlaybackService {
await this.externalPlayback.closeSession(running);
}
// Closing is a round-trip, and a second pick across it would otherwise
// reach this line too: both would have seen the same session, closed
// it once, and then launched independently — two detached players
// again, the older one holding a source the user has moved on from.
if (generation !== this.startGeneration) {
return;
}
// Same movie, different source: still a view.
this.addToRecentlyViewed();
this.startPlayback(playback);
@@ -1,93 +1,34 @@
import { signal } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import {
VodSourceDiscoveryService,
VodSourceResolverService,
} from '@iptvnator/portal/shared/data-access';
import {
SettingsStore,
StreamProbeService,
VodSourcePinService,
} from '@iptvnator/services';
import type { VodSourceCandidate } from '@iptvnator/shared/interfaces';
import { VodMultiSourceHostService } from './vod-multi-source-host.service';
import type { VodMultiSourceMovie } from './vod-multi-source-identity';
import {
ALT_THREE,
ALT_TWO,
MOVIE_A,
MOVIE_B,
PROBE_OK,
createDeferred,
resolveWith,
resolvedFor,
setupVodMultiSourceHost,
} from './vod-multi-source-host.fixtures';
describe('VodMultiSourceHostService — stale resolutions', () => {
let service: VodMultiSourceHostService;
const movie = signal<VodMultiSourceMovie | null>(null);
// Whatever is on screen; the pin path distinguishes it from selection.
const playbackLive = signal(false);
const vodAutoFailover = signal(false);
const startPlayback = jest.fn();
const discovery = { isAvailable: true, discover: jest.fn() };
const resolver = { resolve: jest.fn() };
const pins = { get: jest.fn(), set: jest.fn(), clear: jest.fn() };
const probes = { probe: jest.fn() };
/** Runs pending root effects, then drains the work they started. */
async function loadMovie(
sources: VodSourceCandidate[],
target: VodMultiSourceMovie = MOVIE_A
): Promise<void> {
discovery.discover.mockResolvedValue({
sources,
matchKind: 'title-year',
});
await service.load(target);
}
function rowFor(sourceId: string) {
return service.sources().find((source) => source.id === sourceId);
}
const harness = setupVodMultiSourceHost();
const {
discovery,
movie,
pins,
probes,
resolver,
startPlayback,
vodAutoFailover,
} = harness;
const loadMovie = harness.loadMovie;
const rowFor = harness.rowFor;
let service = harness.service;
/** Two consecutive switches, so the "previous" side carries real audio. */
beforeEach(() => {
jest.resetAllMocks();
movie.set(null);
vodAutoFailover.set(false);
discovery.isAvailable = true;
discovery.discover.mockResolvedValue({
sources: [],
matchKind: 'title-year',
});
resolver.resolve.mockImplementation(resolveWith());
pins.get.mockResolvedValue(null);
pins.set.mockResolvedValue(true);
pins.clear.mockResolvedValue(true);
probes.probe.mockResolvedValue(PROBE_OK);
TestBed.configureTestingModule({
providers: [
VodMultiSourceHostService,
{ provide: VodSourceDiscoveryService, useValue: discovery },
{ provide: VodSourceResolverService, useValue: resolver },
{ provide: VodSourcePinService, useValue: pins },
{ provide: StreamProbeService, useValue: probes },
{ provide: SettingsStore, useValue: { vodAutoFailover } },
],
});
service = TestBed.inject(VodMultiSourceHostService);
TestBed.runInInjectionContext(() =>
service.bind({ startPlayback, movie, playbackLive })
);
service = harness.reset();
});
it('drops a switch once the movie identity goes null mid-navigation', async () => {
@@ -111,6 +52,37 @@ describe('VodMultiSourceHostService — stale resolutions', () => {
expect(rowFor(ALT_TWO.id)?.isActive).toBe(false);
});
it('lets a source picked during the pinned lookup win', async () => {
pins.get.mockResolvedValue({
matchKey: 'title:the matrix:1999',
playlistId: ALT_TWO.playlistId,
contentId: ALT_TWO.contentId,
portalType: 'xtream',
});
await loadMovie([ALT_TWO, ALT_THREE]);
const lookup = createDeferred<number | null>();
let insideLookup = false;
const pinnedPlay = service.playPinnedSource(() => {
insideLookup = true;
return lookup.promise;
});
while (!insideLookup) {
await Promise.resolve();
}
// The user picks a different source while the pinned resume position
// is still being read.
await service.play(ALT_THREE.id);
startPlayback.mockClear();
lookup.resolve(0);
// The older pinned attempt must not finish last and replace it.
await expect(pinnedPlay).resolves.toBe('superseded');
expect(startPlayback).not.toHaveBeenCalled();
expect(rowFor(ALT_THREE.id)?.isActive).toBe(true);
});
it('tells a superseded pinned play apart from an unusable pin', async () => {
pins.get.mockResolvedValue({
matchKey: 'title:the matrix:1999',
@@ -246,12 +246,17 @@ export class VodMultiSourceHostService {
resumeFor?: (source: VodSourceCandidate) => Promise<number | null>
): Promise<PinnedPlayOutcome> {
const session = this.sessionToken;
// Claim a switch generation up front. The discovery wait and the
// resume lookup are both awaits, and a source the user picks across
// either one has to win — otherwise this older attempt finishes last
// and replaces what they just chose.
const attempt = ++this.switchToken;
return startPinnedSource({
controller: this.controller,
loadInFlight: this.loadInFlight,
pinnedSourceId: () => this.pendingPinnedSourceId(),
resumeFor,
isCurrent: () => session === this.sessionToken,
isCurrent: () => this.isCurrentSwitch(session, attempt),
play: (sourceId) => this.runPlay(sourceId),
});
}
@@ -103,6 +103,21 @@ describe('createPrimaryActionPosition', () => {
expect(api.position()?.positionSeconds).toBe(4200);
});
it('claims nothing while the pinned copy’s row is still loading', () => {
const { api } = setup(
[source(), ALT],
position(2538),
() => new Promise(() => undefined)
);
TestBed.tick();
// The route copy's "Resume 42:18" would be a label for a stream this
// button is not going to start — and a click across this window looks
// the pinned position up separately and begins somewhere else.
expect(api.position()).toBeNull();
expect(api.hasPosition()).toBe(false);
});
it('reads Play for a pinned copy that was never watched', async () => {
const { api } = setup(
[source(), ALT],
@@ -104,10 +104,19 @@ export function createPrimaryActionPosition(
const position = computed(() => {
const pinned = foreignPin();
if (!pinned || !pinnedLoadedFor()) {
if (!pinned) {
return deps.routePosition();
}
// The pin names a copy whose row has not been read yet. Showing the
// ROUTE copy's `Resume 42:18` here would be a label for a stream this
// button is not going to start — and a click across this window looks
// the pinned position up separately and begins somewhere else. Say
// nothing until the answer is in; empty beats wrong.
if (!pinnedLoadedFor()) {
return null;
}
// What just played beats what was stored before it did.
const live = deps.livePosition();
const isPinnedCopy =
@@ -56,7 +56,9 @@ describe('StreamProbeService', () => {
const probe = jest
.fn()
.mockResolvedValueOnce({ status: 200, url: 'http://x/y.mkv' })
.mockResolvedValueOnce({ status: 403, url: 'http://x/y.mkv' });
// 404, not 403: this test is about cache keying, and a 403 would
// now earn a ranged-GET retry and muddy the call count.
.mockResolvedValueOnce({ status: 404, url: 'http://x/y.mkv' });
const service = withBridge(probe as ProbeFn);
const first = await service.probe('http://x/y.mkv', 'HEAD', {
@@ -100,7 +102,8 @@ describe('StreamProbeService', () => {
);
});
it('maps a refusal to fail', async () => {
it('maps a refusal to fail once the GET retry agrees', async () => {
// 403 earns a retry, but a 403 to the ranged GET too is a real answer.
const service = withBridge(ok(403));
await expect(service.probe('http://x/y.mkv')).resolves.toEqual(
@@ -118,11 +121,13 @@ describe('StreamProbeService', () => {
);
});
it.each([405, 501])(
it.each([400, 403, 405, 501])(
'retries with a ranged GET when the server refuses HEAD with %i',
async (status) => {
// Plenty of stream servers reject HEAD yet serve the media over
// GET; reporting those as unavailable would be a confident lie.
// 400 and 403 are what a WAF returns for an unexpected HEAD, and
// treating those as dead ranks a working source below worse ones.
const probe = jest
.fn()
.mockResolvedValueOnce({ status, url: 'u' })
@@ -148,8 +153,9 @@ describe('StreamProbeService', () => {
}
);
it('does not retry a plain refusal', async () => {
const probe = ok(403);
it('does not retry a status that names the resource, not the method', async () => {
// 404 is about this URL; retrying it with another verb proves nothing.
const probe = ok(404);
const service = withBridge(probe);
await service.probe('http://x/y.mkv');
+16 -2
View File
@@ -170,9 +170,23 @@ export class StreamProbeService {
* redirect-safety policy refused. That is NOT evidence the source is dead, so
* it maps to 'unknown' and the UI keeps offering a check.
*/
/** Statuses that mean "not this method", not "not this resource". */
/**
* Statuses that may mean "not this method" rather than "not this resource".
*
* 405/501 say so outright. 403 and 400 are the ones a WAF or a stream server
* returns for a HEAD it does not expect while serving the same URL over GET
* perfectly well — and calling that source unavailable is a confident lie that
* also ranks it below worse ones. The retry is bounded (one ranged GET), so
* being generous here costs a request, while being strict costs a working
* source.
*/
function refusesHeadRequests(httpStatus: number): boolean {
return httpStatus === 405 || httpStatus === 501;
return (
httpStatus === 400 ||
httpStatus === 403 ||
httpStatus === 405 ||
httpStatus === 501
);
}
function toProbeStatus(httpStatus: number): VodSourceProbeResult['status'] {