From d76b2d2a570bf60f044dafc439e67adcc84f3ec4 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:34:17 +0200 Subject: [PATCH] fix(tmdb): stop a broken provider tmdb_id from suppressing enrichment (#1239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(tmdb): stop a broken provider tmdb_id from suppressing enrichment Providers ship dead and stale tmdb_id values, and enrich() trusted them unconditionally: parseProviderTmdbId(query.tmdbId) ?? await resolveIdBySearch(...) A garbage-but-integer id short-circuited the title search entirely. The details fetch then 404'd, the outer catch swallowed it, and the item was left permanently unenriched — no plot, no cast, no artwork — for a title the search would have matched. Failed detail fetches cache nothing, so the wasted request repeated on every re-open. The stale-but-valid case was worse: it never threw, nothing sanity-checked the resolved title, and we confidently rendered another film's metadata. enrich() now treats the provider id as a hint. If it fails to resolve, or resolves to something whose title matches none of the search variants we would have queried, the confidence-gated title search gets its turn — and proven-bad ids are negative-cached (7d, language-independent row) so the 404 is not repeated forever. Deliberately NOT a hard rejection on title mismatch: TMDB returns titles in the REQUEST language, so a Russian provider title legitimately fails the name check against an en-US payload. A mismatch only lets the search compete; when the search finds nothing confident, the provider payload is kept. The change can therefore only add enrichment, never remove it. Extracts the search resolution and the bad-id cache into TmdbIdResolverService — tmdb-enrichment.service.ts was at 290 lines against the 300-line target, and the resolver is independently testable. Tests: new tmdb-enrichment.service.spec.ts covers the happy path issuing exactly one details call and no search, 404 fallback, stale-id override, the keep-the-payload safety property, bad-id skip, and the no-match case; matcher spec covers detailsMatchProviderTitle and the namespaced cache key. Refs docs/architecture/tmdb-roadmap.md A1. Co-Authored-By: Claude Opus 4.8 * fix(tmdb): only blame a provider id when TMDB confirms it does not exist Review found the bad-id negative cache too eager in two ways, both of which could deny enrichment to items whose id was fine. 1. Any failure recorded the verdict. A 401, 429, 5xx or an offline blip would mark a perfectly valid id as dead for seven days, so after the service recovered — or the user fixed their API key — titles that the search cannot resolve confidently stayed unenriched until the marker expired. TmdbApiService now throws a typed TmdbApiError carrying the status, and only a confirmed 404 is recorded. 2. Title mismatches were recorded too. That id EXISTS; it is merely wrong for this item. The row is keyed by id alone and shared across playlists, so a stale mapping on one item disabled the direct lookup for every other item that legitimately used the same id. Mismatches are no longer cached at all — the search verdict is cached anyway, so the repeat cost is a single details fetch. Documents the row kind in the cache contract, which listed only two of the (now six) lookup_key shapes. Tests: 404 records, 429 does not, network error does not, mismatch does not. Co-Authored-By: Claude Opus 4.8 * fix(tmdb): keep provider details when the competing search fails detailsForProviderId only runs the search to see whether it can beat a title-mismatched provider payload. A throw from that best-effort search (offline, rate limit, 5xx) propagated to enrich()'s outer catch and threw away details we already had — the searched-details fetch right below it was already tolerant. Fail to the details in hand instead. Co-Authored-By: Claude Opus 4.8 * fix(tmdb): decide a suspect provider id on evidence, not on the title The title check alone was both too weak and too dangerous. Too weak: normalizeTitle strips trailing years, so "Blade Runner 2049" carrying the 1982 film's id matched and the wrong film was rendered — exactly the stale-id case this was meant to catch. Too dangerous: an ALL-CAPS leading token reads as a language tag, so "IT - Chapter Two" normalizes to "chapter two". The correct payload failed the name check, and a year-less search for "chapter two" would confidently return the 1979 film and overwrite it. Master trusted the provider id here and got it right. assessProviderId weighs both signals: title or year agrees means use the details; both years known and incompatible means the search may take over; a title-only mismatch is inconclusive and keeps the details. The search branch now always has a year, so its own gate corroborates whatever it returns instead of matching on name alone. Co-Authored-By: Claude Opus 4.8 * fix(tmdb): do not search after a transient provider-id failure enrich() reads a null from detailsForProviderId as "the id is unusable, try the search". A 401/429/5xx/offline failure gave it that null, so an outage turned into a second request that would fail too — and if it did come back, a title match replaced a provider id that was probably fine. Only a 404 falls through to the search now; everything else rethrows and leaves the id retryable. Co-Authored-By: Claude Opus 4.8 * docs(tmdb): add the release note for the provider-id fix Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changes/tmdb-broken-provider-id.md | 10 + CLAUDE.md | 2 +- docs/architecture/tmdb-metadata-enrichment.md | 46 ++- libs/services/src/lib/tmdb/index.ts | 1 + .../services/src/lib/tmdb/tmdb-api.service.ts | 23 +- .../lib/tmdb/tmdb-enrichment.service.spec.ts | 294 ++++++++++++++++++ .../src/lib/tmdb/tmdb-enrichment.service.ts | 187 +++++------ .../src/lib/tmdb/tmdb-id-resolver.service.ts | 158 ++++++++++ .../src/lib/tmdb/tmdb-matcher.spec.ts | 57 ++++ libs/services/src/lib/tmdb/tmdb-matcher.ts | 99 ++++++ .../src/lib/tmdb/tmdb-provider-id.spec.ts | 92 ++++++ 11 files changed, 869 insertions(+), 100 deletions(-) create mode 100644 .changes/tmdb-broken-provider-id.md create mode 100644 libs/services/src/lib/tmdb/tmdb-enrichment.service.spec.ts create mode 100644 libs/services/src/lib/tmdb/tmdb-id-resolver.service.ts create mode 100644 libs/services/src/lib/tmdb/tmdb-provider-id.spec.ts diff --git a/.changes/tmdb-broken-provider-id.md b/.changes/tmdb-broken-provider-id.md new file mode 100644 index 000000000..cbc5b6cf4 --- /dev/null +++ b/.changes/tmdb-broken-provider-id.md @@ -0,0 +1,10 @@ +--- +type: fix +area: tmdb +--- + +Movies whose provider ships a dead or wrong TMDB id are enriched again. The +id is weighed against the title and release year: a dead one falls back to +the title search, a stale one that clearly points at another film loses to +it, and a working id is no longer thrown away just because the provider +spells the title differently. diff --git a/CLAUDE.md b/CLAUDE.md index ae30b0305..f699a20a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -836,7 +836,7 @@ engine` (restart required) or - Actor pages: cast avatar chips are clickable (TMDB person id) and open `actor/:personId` inside the current portal — TMDB person bio + full filmography (acting + directing credits merged; acting wins the per-title dedup); director/creator chips (`tmdb_directors` via `enrichedDirectors`/`enrichedCreators` in `tmdb-merge.ts`) are clickable the same way and open the same person page; Xtream matches titles against the loaded catalog (direct navigation), unmatched titles and all Stalker titles open the portal search prefilled (`?q=`); the in-portal search page shows a Back button (`SearchLayoutComponent.showBackButton` → `Location.back()`) so users can return to the actor page; shared UI in `libs/ui/shared-portals` (`ActorViewComponent`) - Actor page "All portals" scope (Electron only): batched `DB_MATCH_TITLES` worker op (trigram FTS over all imported Xtream playlists, `apps/electron-backend/src/app/database/operations/title-match.operations.ts`); `normalizeTitle` is shared renderer/worker via `libs/shared/interfaces/src/lib/title-normalization.util.ts` - Opt-in via `Settings > Metadata (TMDB)` (sends titles to TMDB); optional user API key overrides the embedded default (`DEFAULT_TMDB_API_KEY` in `libs/services/src/lib/tmdb/tmdb-config.ts` — an empty placeholder in the repo by design; the real key lives in the `TMDB_API_KEY` GitHub Actions secret and is injected at CI build time by `tools/tmdb/inject-tmdb-key.mjs`) -- Match confidence: provider `tmdb_id` trusted fully; otherwise normalized-title + year (±1) search with a strict gate — no confident match means no enrichment +- Match confidence: a provider `tmdb_id` is a strong hint, not gospel — its payload is weighed against the item (`assessProviderId`: title or year agrees → use it; both years known and incompatible → the search may take over; title-only mismatch → keep it, since TMDB localizes titles). A 404 marks the id dead (`badProviderId:` row); transient failures never do. Without a usable id: normalized-title + year (±1) search with a strict gate — no confident match means no enrichment - Detail views render provider data immediately; enrichment patches the selection asynchronously (staleness-guarded) - Cached in SQLite `tmdb_metadata` (Electron, via DB worker ops `DB_GET/SET_TMDB_METADATA`) or in-memory (PWA); localized via the app language setting. Search-match lookup keys are versioned, and connection startup removes obsolete unversioned rows once through the `migration:tmdb-search-lookup-v2-cache-cleanup:v1` app-state marker. - Service layer: `libs/services/src/lib/tmdb/`; store glue: `libs/portal/xtream/data-access/src/lib/stores/xtream-tmdb-enrichment.ts` and `libs/portal/stalker/data-access/src/lib/stores/stalker-tmdb-enrichment.ts` (hooked in `withStalkerSelection().setSelectedItem`) diff --git a/docs/architecture/tmdb-metadata-enrichment.md b/docs/architecture/tmdb-metadata-enrichment.md index e78dd4bd9..49a329d9f 100644 --- a/docs/architecture/tmdb-metadata-enrichment.md +++ b/docs/architecture/tmdb-metadata-enrichment.md @@ -83,8 +83,29 @@ in the settings section validates the API key against `/configuration`. Wrong metadata is worse than no metadata, so id resolution is conservative: 1. If the provider returns a usable `tmdb_id` (Xtream VOD info often does), - it is trusted fully and no search runs. Series have no show-level - `tmdb_id`, so they always go through search. + its details are fetched directly and normally used as-is. Series have no + show-level `tmdb_id`, so they always go through search. The id is a + strong hint rather than gospel — panels ship dead and stale ones — so + the payload it returns is weighed against the provider item + (`assessProviderId`): + - a matching title **or** a compatible release year (±1; for series, + any earlier premiere) → **corroborated**, use it; + - both years known and incompatible → **contradicted**, the stale-id + signature ("Blade Runner 2049" carrying the 1982 film's id): the + title search may take over, and does so only if it finds a confident + match of its own; + - title differs with no year to arbitrate → **inconclusive**, keep the + details. TMDB returns titles in the request language and + normalization strips stylized prefixes ("IT - Chapter Two" → + "chapter two"), so a name mismatch alone says more about our inputs + than about the id. + + A 404 is the one hard verdict: the id is recorded as dead + (`badProviderId:` row), skipped next time, and the title search + takes over. Transient failures (auth, rate limit, 5xx, offline) neither + disable the id nor trigger a search — that request would hit the same + outage, and a title match that did come back would be weaker evidence + than the id already in hand. 2. Otherwise `/search/movie` (or `/search/tv`) runs with the normalized title. Normalization strips bracketed tags, quality markers (`4K`, `1080p`, `MULTI`, …), leading language prefixes (`EN - `), diacritics, @@ -247,14 +268,18 @@ Filmography has two scopes: ## Cache -Single table with two row kinds discriminated by `lookup_key` prefix: +Single table with several row kinds discriminated by the `lookup_key` +prefix: ``` tmdb_metadata ( media_type 'movie' | 'tv' | 'person', lookup_key 'id:|v2' -- details payload row + 'id:|season:' -- season payload row 'title:|year:|v2' -- search resolution row 'person:' -- person payload row + 'trending:week' -- trending list row + 'badProviderId:' -- id confirmed 404 by TMDB language TEXT, -- TMDB language code tmdb_id INTEGER, -- NULL on a search row = negative cache payload TEXT, -- raw JSON details, NULL for search rows @@ -343,3 +368,18 @@ dashboard rail, artwork upgrade for M3U VOD, persistent PWA cache heroes additionally show the tracked "S{n}·E{n}" badge from the playback position (no TMDB involved); the watch-progress bar is limited to movie/series heroes. + +### `badProviderId:` rows + +Providers ship `tmdb_id` values that do not exist. A failed details fetch +caches nothing, so without a marker the same 404 is re-issued on every +detail open, forever. These rows record that verdict: `tmdb_id` NULL, +language `any` (a dead id is dead in every language), read with the +negative-match TTL. + +Only a **confirmed 404** is recorded. Transient failures (401, 429, 5xx, +offline) leave no marker — they say nothing about the id. Neither does a +title mismatch: that id exists and may be correct for a *different* item, +and since the row is keyed by id alone and shared across playlists, +recording per-item mismatches here would deny the direct lookup to every +other item that legitimately uses the same id. diff --git a/libs/services/src/lib/tmdb/index.ts b/libs/services/src/lib/tmdb/index.ts index cd28e0596..0b94b3c1f 100644 --- a/libs/services/src/lib/tmdb/index.ts +++ b/libs/services/src/lib/tmdb/index.ts @@ -2,6 +2,7 @@ export * from './tmdb-api.service'; export * from './tmdb-cache.service'; export * from './tmdb-config'; export * from './tmdb-enrichment.service'; +export * from './tmdb-id-resolver.service'; export * from './tmdb-episode-merge'; export * from './tmdb-matcher'; export * from './tmdb-merge'; diff --git a/libs/services/src/lib/tmdb/tmdb-api.service.ts b/libs/services/src/lib/tmdb/tmdb-api.service.ts index 4d7144591..8aacd83af 100644 --- a/libs/services/src/lib/tmdb/tmdb-api.service.ts +++ b/libs/services/src/lib/tmdb/tmdb-api.service.ts @@ -14,6 +14,25 @@ import { * both the Electron renderer and the PWA can call it without a proxy. * Supports classic v3 keys (query param) and v4 read tokens (Bearer). */ +/** + * A non-2xx TMDB response. Carries the status so callers can tell a + * definitive verdict (404 — this id does not exist) apart from a + * transient one (401/429/5xx, offline), which must stay retryable. + */ +export class TmdbApiError extends Error { + constructor( + readonly status: number, + statusText = '' + ) { + super(`TMDB request failed: ${status} ${statusText}`.trim()); + this.name = 'TmdbApiError'; + } +} + +export function isTmdbNotFound(error: unknown): boolean { + return error instanceof TmdbApiError && error.status === 404; +} + @Injectable({ providedIn: 'root' }) export class TmdbApiService { async searchMovie( @@ -155,9 +174,7 @@ export class TmdbApiService { }); if (!response.ok) { - throw new Error( - `TMDB request failed: ${response.status} ${response.statusText}` - ); + throw new TmdbApiError(response.status, response.statusText); } return (await response.json()) as T; diff --git a/libs/services/src/lib/tmdb/tmdb-enrichment.service.spec.ts b/libs/services/src/lib/tmdb/tmdb-enrichment.service.spec.ts new file mode 100644 index 000000000..6d5ecae41 --- /dev/null +++ b/libs/services/src/lib/tmdb/tmdb-enrichment.service.spec.ts @@ -0,0 +1,294 @@ +import { Injector, runInInjectionContext } from '@angular/core'; +import { TmdbApiError, TmdbApiService } from './tmdb-api.service'; +import { TmdbCacheService } from './tmdb-cache.service'; +import { TmdbEnrichmentService } from './tmdb-enrichment.service'; +import { TmdbIdResolverService } from './tmdb-id-resolver.service'; +import { TmdbPersonService } from './tmdb-person.service'; +import { TmdbRuntimeService } from './tmdb-runtime.service'; +import { TmdbSeasonService } from './tmdb-season.service'; +import { TmdbTrendingService } from './tmdb-trending.service'; +import { TmdbMovieDetails } from './tmdb.types'; + +/** + * Regression coverage for provider-supplied `tmdb_id` handling. Panels + * ship dead and stale ids; before this, a dead one short-circuited the + * title search (leaving the item permanently unenriched) and a stale one + * silently rendered another film's metadata. + */ +describe('TmdbEnrichmentService — provider tmdb_id handling', () => { + const matrix: TmdbMovieDetails = { + id: 603, + title: 'The Matrix', + original_title: 'The Matrix', + overview: 'Plot', + videos: { results: [{ key: 'k', site: 'YouTube', type: 'Trailer' }] }, + }; + const unrelated: TmdbMovieDetails = { + id: 999, + title: 'Completely Different Film', + original_title: 'Completely Different Film', + release_date: '1979-12-14', + overview: 'Other plot', + videos: { results: [{ key: 'x', site: 'YouTube', type: 'Trailer' }] }, + }; + + let getMovieDetails: jest.Mock; + let resolveBySearch: jest.Mock; + let isKnownBadProviderId: jest.Mock; + let rememberBadProviderId: jest.Mock; + + // The services Jest target has no @angular/core/testing — build the + // service in a plain injection context instead of TestBed. + function createService(): TmdbEnrichmentService { + const injector = Injector.create({ + providers: [ + { + provide: TmdbRuntimeService, + useValue: { + isEnabled: () => true, + apiKey: () => 'key', + language: () => 'en-US', + appLanguage: () => 'en', + }, + }, + { + provide: TmdbApiService, + useValue: { getMovieDetails, getTvDetails: jest.fn() }, + }, + { + provide: TmdbCacheService, + useValue: { + get: jest.fn().mockResolvedValue(null), + set: jest.fn().mockResolvedValue(undefined), + isFresh: () => false, + }, + }, + { + provide: TmdbIdResolverService, + useValue: { + resolveBySearch, + isKnownBadProviderId, + rememberBadProviderId, + }, + }, + { provide: TmdbPersonService, useValue: {} }, + { provide: TmdbSeasonService, useValue: {} }, + { provide: TmdbTrendingService, useValue: {} }, + ], + }); + return runInInjectionContext( + injector, + () => new TmdbEnrichmentService() + ); + } + + beforeEach(() => { + getMovieDetails = jest.fn().mockResolvedValue(matrix); + resolveBySearch = jest.fn().mockResolvedValue(null); + isKnownBadProviderId = jest.fn().mockResolvedValue(false); + rememberBadProviderId = jest.fn().mockResolvedValue(undefined); + }); + + it('uses a valid provider id without searching at all', async () => { + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 603, + title: 'The Matrix', + }); + + expect(details?.id).toBe(603); + expect(getMovieDetails).toHaveBeenCalledTimes(1); + expect(resolveBySearch).not.toHaveBeenCalled(); + expect(rememberBadProviderId).not.toHaveBeenCalled(); + }); + + it('falls back to the title search when the provider id 404s', async () => { + getMovieDetails + .mockRejectedValueOnce(new TmdbApiError(404, 'Not Found')) + .mockResolvedValueOnce(matrix); + resolveBySearch.mockResolvedValue(603); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 123456789, + title: 'The Matrix', + }); + + // Previously this returned null: the dead id skipped the search + expect(details?.id).toBe(603); + expect(resolveBySearch).toHaveBeenCalledTimes(1); + expect(rememberBadProviderId).toHaveBeenCalledWith( + 'movie', + 123456789 + ); + }); + + it('prefers a confident search match over a stale provider id', async () => { + // The id resolves, but to a 1979 film while our item is from 1999 + getMovieDetails + .mockResolvedValueOnce(unrelated) + .mockResolvedValueOnce(matrix); + resolveBySearch.mockResolvedValue(603); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 999, + title: 'The Matrix', + year: 1999, + }); + + expect(details?.id).toBe(603); + // The id EXISTS — it is just wrong for this item. The bad-id row is + // keyed by id alone and shared across playlists, so recording a + // per-item mismatch would deny the direct lookup to every other + // item that legitimately uses the same id. + expect(rememberBadProviderId).not.toHaveBeenCalled(); + }); + + it('does not blame the id for a transient failure', async () => { + // Rate limit, bad key, 5xx or offline: the id may be perfectly + // fine, so it must stay retryable rather than be disabled for days + getMovieDetails.mockRejectedValue(new TmdbApiError(429, 'Too Many')); + resolveBySearch.mockResolvedValue(null); + const service = createService(); + + await service.enrichMovie({ tmdbId: 603, title: 'The Matrix' }); + + expect(rememberBadProviderId).not.toHaveBeenCalled(); + }); + + it('does not search after a transient provider-id failure', async () => { + // The search would hit the same outage, and a title match that + // does come back is weaker evidence than the id we already have + getMovieDetails.mockRejectedValue(new TmdbApiError(503, 'Down')); + resolveBySearch.mockResolvedValue(603); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 603, + title: 'The Matrix', + }); + + expect(details).toBeNull(); + expect(resolveBySearch).not.toHaveBeenCalled(); + }); + + it('does not blame the id for a network error', async () => { + getMovieDetails.mockRejectedValue(new Error('offline')); + resolveBySearch.mockResolvedValue(null); + const service = createService(); + + await service.enrichMovie({ tmdbId: 603, title: 'The Matrix' }); + + expect(rememberBadProviderId).not.toHaveBeenCalled(); + }); + + it('keeps the provider payload when the title differs but the search finds nothing', async () => { + // A localized provider title legitimately fails the name check — + // TMDB returns titles in the REQUEST language. Never trade real + // metadata for none on suspicion alone. + getMovieDetails.mockResolvedValue(unrelated); + resolveBySearch.mockResolvedValue(null); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 999, + title: 'Ирония судьбы', + year: 1979, + }); + + expect(details?.id).toBe(999); + expect(rememberBadProviderId).not.toHaveBeenCalled(); + }); + + it('never lets a name mismatch alone unseat a working provider id', async () => { + // "IT - Chapter Two" normalizes to "chapter two" (the ALL-CAPS "IT" + // reads as a language tag), so the correct payload fails the name + // check. With no year to arbitrate, a search for "chapter two" + // would confidently return the 1979 film of that name. + getMovieDetails.mockResolvedValue({ + id: 474350, + title: 'It Chapter Two', + original_title: 'It Chapter Two', + overview: 'Plot', + }); + resolveBySearch.mockResolvedValue(30619); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 474350, + title: 'IT - Chapter Two', + }); + + expect(details?.id).toBe(474350); + expect(resolveBySearch).not.toHaveBeenCalled(); + }); + + it('catches a stale id that shares the title but not the year', async () => { + // The scraper case: "Blade Runner 2049" carrying the 1982 film's id. + // Both titles normalize to "blade runner", so only the year sees it. + getMovieDetails + .mockResolvedValueOnce({ + id: 78, + title: 'Blade Runner', + release_date: '1982-06-25', + }) + .mockResolvedValueOnce({ + id: 335984, + title: 'Blade Runner 2049', + release_date: '2017-10-04', + }); + resolveBySearch.mockResolvedValue(335984); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 78, + title: 'Blade Runner 2049', + year: 2017, + }); + + expect(details?.id).toBe(335984); + }); + + it('keeps the provider payload when the competing search throws', async () => { + // Same reasoning as above, but the search never returns a verdict: + // TMDB is offline or rate-limiting. Details in hand beat nothing. + getMovieDetails.mockResolvedValue(unrelated); + resolveBySearch.mockRejectedValue(new TmdbApiError(503, 'Down')); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 999, + title: 'Ирония судьбы', + year: 1999, + }); + + expect(details?.id).toBe(999); + }); + + it('skips a provider id already known to be bad', async () => { + isKnownBadProviderId.mockResolvedValue(true); + resolveBySearch.mockResolvedValue(603); + const service = createService(); + + const details = await service.enrichMovie({ + tmdbId: 123456789, + title: 'The Matrix', + }); + + expect(details?.id).toBe(603); + // One details call for the searched id — none for the dead one + expect(getMovieDetails).toHaveBeenCalledTimes(1); + expect(getMovieDetails).toHaveBeenCalledWith(603, 'en-US', 'key'); + }); + + it('returns null when there is no provider id and no search match', async () => { + const service = createService(); + + await expect( + service.enrichMovie({ title: 'Unknown Thing' }) + ).resolves.toBeNull(); + expect(getMovieDetails).not.toHaveBeenCalled(); + }); +}); diff --git a/libs/services/src/lib/tmdb/tmdb-enrichment.service.ts b/libs/services/src/lib/tmdb/tmdb-enrichment.service.ts index bfed5f41b..882f8a8b3 100644 --- a/libs/services/src/lib/tmdb/tmdb-enrichment.service.ts +++ b/libs/services/src/lib/tmdb/tmdb-enrichment.service.ts @@ -1,21 +1,14 @@ import { Injectable, inject } from '@angular/core'; import { TmdbMediaType } from '@iptvnator/shared/interfaces'; -import { TmdbApiService } from './tmdb-api.service'; +import { TmdbApiService, isTmdbNotFound } from './tmdb-api.service'; import { TmdbCacheService } from './tmdb-cache.service'; +import { TMDB_DETAILS_CACHE_TTL_MS } from './tmdb-config'; import { - TMDB_DETAILS_CACHE_TTL_MS, - TMDB_MATCH_CACHE_TTL_MS, - TMDB_NEGATIVE_MATCH_CACHE_TTL_MS, - tmdbSearchLanguageForTitle, -} from './tmdb-config'; -import { + assessProviderId, buildDetailsLookupKey, - buildSearchLookupKey, - buildSearchTitleVariants, - extractYear, parseProviderTmdbId, - pickConfidentMatch, } from './tmdb-matcher'; +import { TmdbIdResolverService } from './tmdb-id-resolver.service'; import { detailsFallbackLanguage, fillDetailsFromFallback, @@ -53,6 +46,7 @@ export class TmdbEnrichmentService { private readonly person = inject(TmdbPersonService); private readonly season = inject(TmdbSeasonService); private readonly trending = inject(TmdbTrendingService); + private readonly idResolver = inject(TmdbIdResolverService); isEnabled(): boolean { return this.runtime.isEnabled(); @@ -107,15 +101,31 @@ export class TmdbEnrichmentService { } try { - const tmdbId = - parseProviderTmdbId(query.tmdbId) ?? - (await this.resolveIdBySearch(mediaType, query)); - - if (tmdbId === null) { - return null; + const providerId = parseProviderTmdbId(query.tmdbId); + if ( + providerId !== null && + !(await this.idResolver.isKnownBadProviderId( + mediaType, + providerId + )) + ) { + const details = await this.detailsForProviderId( + mediaType, + providerId, + query + ); + if (details) { + return details; + } } - return await this.getDetails(mediaType, tmdbId); + const searchedId = await this.idResolver.resolveBySearch( + mediaType, + query + ); + return searchedId === null + ? null + : await this.getDetails(mediaType, searchedId); } catch (error) { console.warn(`TMDB ${mediaType} enrichment failed:`, error); return null; @@ -123,88 +133,79 @@ export class TmdbEnrichmentService { } /** - * Resolve a title/year to a TMDB id via /search with the confidence - * gate. Both hits and misses are cached; misses use a shorter TTL. + * Details for a provider-supplied `tmdb_id`, or `null` when the id is + * unusable and the caller should fall back to the title search. + * + * Providers ship broken ids. A dead one used to short-circuit the + * search and leave the item permanently unenriched; a stale-but-valid + * one silently rendered another title's plot and cast. Both cases now + * defer to the (confidence-gated) search — but only on evidence the id + * is wrong, and only when the search actually produces something, so a + * localized or stylized title never costs the user their metadata. */ - private async resolveIdBySearch( + private async detailsForProviderId( mediaType: TmdbMediaType, + providerId: number, query: TmdbEnrichmentQuery - ): Promise { - // Try the original title, the display title, then language-prefix- - // stripped fallbacks; the first confident match wins. - const variants = buildSearchTitleVariants( - query.title, - query.originalTitle - ); - if (variants.length === 0) { - return null; - } - - const year = query.year ?? extractYear(null, query.title); - const cacheLanguage = tmdbSearchLanguageForTitle( - variants[0], - this.runtime.appLanguage() - ); - const lookupKey = buildSearchLookupKey(variants[0], year); - - const cached = await this.cache.get( - mediaType, - lookupKey, - cacheLanguage - ); - const ttl = - cached?.tmdbId !== null && cached?.tmdbId !== undefined - ? TMDB_MATCH_CACHE_TTL_MS - : TMDB_NEGATIVE_MATCH_CACHE_TTL_MS; - if (this.cache.isFresh(cached, ttl)) { - return cached?.tmdbId ?? null; - } - - let match = null; - for (const variant of variants) { - // Cyrillic (and other non-app-script) titles search in their - // own language so TMDB returns comparable titles — see - // tmdbSearchLanguageForTitle. Search by title only: TMDB's - // year params filter strictly; the ±1/season tolerance lives - // in pickConfidentMatch instead. - const language = tmdbSearchLanguageForTitle( - variant, - this.runtime.appLanguage() - ); - const results = - mediaType === 'movie' - ? await this.api.searchMovie( - variant, - null, - language, - this.runtime.apiKey() - ) - : await this.api.searchTv( - variant, - null, - language, - this.runtime.apiKey() - ); - - match = pickConfidentMatch( - results, - { title: variant, year }, - mediaType - ); - if (match) { - break; + ): Promise { + let details: TmdbDetails | null; + try { + details = await this.getDetails(mediaType, providerId); + } catch (error) { + // Only a 404 proves the id is dead. Auth, rate-limit, 5xx and + // offline failures are transient — remembering those would + // disable a perfectly good id until the marker expires. + if (isTmdbNotFound(error)) { + console.warn( + `TMDB ${mediaType} provider id ${providerId} does not exist, falling back to search:`, + error + ); + await this.idResolver.rememberBadProviderId( + mediaType, + providerId + ); + return null; } + + // Rethrow instead: `enrich` reads a null here as "try the + // search", and searching during an outage or a rate limit just + // adds a request that will fail too — or, worse, succeeds and + // swaps a strong id for a title match. The id stays good for + // the next attempt. + throw error; } - await this.cache.set({ - mediaType, - lookupKey, - language: cacheLanguage, - tmdbId: match?.id ?? null, - payload: null, - }); + if (!details || assessProviderId(details, query, mediaType) !== 'contradicted') { + return details; + } - return match?.id ?? null; + // The id resolves to a title whose release year contradicts the + // provider's — the stale-id signature. Let the search try to beat + // it; keep these details if it cannot. + // + // Deliberately NOT remembered: the id exists and may be correct for + // a DIFFERENT item. The bad-id cache is keyed by id alone and shared + // across playlists, so recording a per-item mismatch there would + // deny the direct lookup to every other item that legitimately uses + // the same id. The search verdict is cached anyway, so the repeat + // cost is one details fetch. + // + // The search is best-effort here: offline, rate-limited or 5xx must + // fall back to the details we already have, not discard them. Its + // own gate needs a year, which this branch guarantees we have, so a + // result that comes back is corroborated rather than merely named + // alike. + const searchedId = await this.idResolver + .resolveBySearch(mediaType, query) + .catch(() => null); + if (searchedId === null || searchedId === providerId) { + return details; + } + + const searched = await this.getDetails(mediaType, searchedId).catch( + () => null + ); + return searched ?? details; } private async getDetails( diff --git a/libs/services/src/lib/tmdb/tmdb-id-resolver.service.ts b/libs/services/src/lib/tmdb/tmdb-id-resolver.service.ts new file mode 100644 index 000000000..d8e90051a --- /dev/null +++ b/libs/services/src/lib/tmdb/tmdb-id-resolver.service.ts @@ -0,0 +1,158 @@ +import { Injectable, inject } from '@angular/core'; +import { TmdbMediaType } from '@iptvnator/shared/interfaces'; +import { TmdbApiService } from './tmdb-api.service'; +import { TmdbCacheService } from './tmdb-cache.service'; +import { + TMDB_MATCH_CACHE_TTL_MS, + TMDB_NEGATIVE_MATCH_CACHE_TTL_MS, + tmdbSearchLanguageForTitle, +} from './tmdb-config'; +import { + buildBadProviderIdLookupKey, + buildSearchLookupKey, + buildSearchTitleVariants, + extractYear, + pickConfidentMatch, +} from './tmdb-matcher'; +import { TmdbRuntimeService } from './tmdb-runtime.service'; +import { TmdbEnrichmentQuery } from './tmdb.types'; + +/** + * "This id is wrong" verdicts are about the provider's data, not about a + * translation, so they are cached under one language-independent row. + */ +const BAD_ID_CACHE_LANGUAGE = 'any'; + +/** + * Resolves a provider item to a TMDB id: the confidence-gated title search + * plus the negative cache for provider-supplied ids we have proven wrong. + * Extracted from the enrichment orchestrator so both concerns stay + * testable and the facade keeps its size in check. + */ +@Injectable({ providedIn: 'root' }) +export class TmdbIdResolverService { + private readonly runtime = inject(TmdbRuntimeService); + private readonly api = inject(TmdbApiService); + private readonly cache = inject(TmdbCacheService); + + /** + * Resolve a title/year to a TMDB id via /search with the confidence + * gate. Both hits and misses are cached; misses use a shorter TTL. + */ + async resolveBySearch( + mediaType: TmdbMediaType, + query: TmdbEnrichmentQuery + ): Promise { + // Try the original title, the display title, then language-prefix- + // stripped fallbacks; the first confident match wins. + const variants = buildSearchTitleVariants( + query.title, + query.originalTitle + ); + if (variants.length === 0) { + return null; + } + + const year = query.year ?? extractYear(null, query.title); + const cacheLanguage = tmdbSearchLanguageForTitle( + variants[0], + this.runtime.appLanguage() + ); + const lookupKey = buildSearchLookupKey(variants[0], year); + + const cached = await this.cache.get( + mediaType, + lookupKey, + cacheLanguage + ); + const ttl = + cached?.tmdbId !== null && cached?.tmdbId !== undefined + ? TMDB_MATCH_CACHE_TTL_MS + : TMDB_NEGATIVE_MATCH_CACHE_TTL_MS; + if (this.cache.isFresh(cached, ttl)) { + return cached?.tmdbId ?? null; + } + + let match = null; + for (const variant of variants) { + // Cyrillic (and other non-app-script) titles search in their + // own language so TMDB returns comparable titles — see + // tmdbSearchLanguageForTitle. Search by title only: TMDB's + // year params filter strictly; the ±1/season tolerance lives + // in pickConfidentMatch instead. + const language = tmdbSearchLanguageForTitle( + variant, + this.runtime.appLanguage() + ); + const results = + mediaType === 'movie' + ? await this.api.searchMovie( + variant, + null, + language, + this.runtime.apiKey() + ) + : await this.api.searchTv( + variant, + null, + language, + this.runtime.apiKey() + ); + + match = pickConfidentMatch( + results, + { title: variant, year }, + mediaType + ); + if (match) { + break; + } + } + + await this.cache.set({ + mediaType, + lookupKey, + language: cacheLanguage, + tmdbId: match?.id ?? null, + payload: null, + }); + + return match?.id ?? null; + } + + /** + * True when this id is known NOT TO EXIST on TMDB. Without this a dead + * id costs one wasted 404 on every single detail open, forever — + * failed detail fetches cache nothing. + * + * The verdict is deliberately about the ID, not about the item that + * supplied it: the row is keyed by id alone and shared across + * playlists, so only "TMDB returned 404 for this id" may be recorded + * here. A per-item mismatch is never cached — see + * `TmdbEnrichmentService.detailsForProviderId`. + */ + async isKnownBadProviderId( + mediaType: TmdbMediaType, + tmdbId: number + ): Promise { + const cached = await this.cache.get( + mediaType, + buildBadProviderIdLookupKey(tmdbId), + BAD_ID_CACHE_LANGUAGE + ); + return this.cache.isFresh(cached, TMDB_NEGATIVE_MATCH_CACHE_TTL_MS); + } + + async rememberBadProviderId( + mediaType: TmdbMediaType, + tmdbId: number + ): Promise { + await this.cache.set({ + mediaType, + lookupKey: buildBadProviderIdLookupKey(tmdbId), + language: BAD_ID_CACHE_LANGUAGE, + tmdbId: null, + payload: null, + }); + } +} diff --git a/libs/services/src/lib/tmdb/tmdb-matcher.spec.ts b/libs/services/src/lib/tmdb/tmdb-matcher.spec.ts index f61f7ce86..7a3b737a1 100644 --- a/libs/services/src/lib/tmdb/tmdb-matcher.spec.ts +++ b/libs/services/src/lib/tmdb/tmdb-matcher.spec.ts @@ -1,6 +1,8 @@ import { + buildBadProviderIdLookupKey, buildDetailsLookupKey, buildSearchLookupKey, + detailsMatchProviderTitle, buildSearchTitleVariants, extractYear, normalizeTitle, @@ -295,3 +297,58 @@ describe('pickConfidentMatch', () => { ).toBeNull(); }); }); + +describe('buildBadProviderIdLookupKey', () => { + it('namespaces the id so it cannot collide with a details row', () => { + expect(buildBadProviderIdLookupKey(603)).toBe('badProviderId:603'); + expect(buildBadProviderIdLookupKey(603)).not.toBe( + buildDetailsLookupKey(603) + ); + }); +}); + +describe('detailsMatchProviderTitle', () => { + const details = { + title: 'The Matrix', + original_title: 'The Matrix', + }; + + it('accepts a match on a noisy provider title', () => { + expect( + detailsMatchProviderTitle(details, { + title: 'EN - The Matrix (1999) 1080p', + }) + ).toBe(true); + }); + + it('accepts a match against the original title only', () => { + expect( + detailsMatchProviderTitle( + { title: 'Die Hard', original_title: 'Stirb langsam' }, + { title: 'Stirb langsam' } + ) + ).toBe(true); + }); + + it('matches series payloads through name/original_name', () => { + expect( + detailsMatchProviderTitle( + { name: 'The Boys', original_name: 'The Boys' }, + { title: 'The Boys s05' } + ) + ).toBe(true); + }); + + it('reports a mismatch for an unrelated title', () => { + expect( + detailsMatchProviderTitle(details, { title: 'Blade Runner' }) + ).toBe(false); + }); + + it('never reports a mismatch when there is nothing to compare', () => { + expect(detailsMatchProviderTitle(details, { title: '' })).toBe(true); + expect(detailsMatchProviderTitle({}, { title: 'The Matrix' })).toBe( + false + ); + }); +}); diff --git a/libs/services/src/lib/tmdb/tmdb-matcher.ts b/libs/services/src/lib/tmdb/tmdb-matcher.ts index 2c60cca56..390e373a9 100644 --- a/libs/services/src/lib/tmdb/tmdb-matcher.ts +++ b/libs/services/src/lib/tmdb/tmdb-matcher.ts @@ -82,6 +82,11 @@ export function buildDetailsLookupKey(tmdbId: number): string { return `id:${tmdbId}|v2`; } +/** Negative-cache key for a provider tmdb_id we have proven wrong */ +export function buildBadProviderIdLookupKey(tmdbId: number): string { + return `badProviderId:${tmdbId}`; +} + /** Provider tmdb_id fields arrive as number, numeric string, or garbage */ export function parseProviderTmdbId( tmdbId: number | string | null | undefined @@ -90,6 +95,100 @@ export function parseProviderTmdbId( return Number.isInteger(parsed) && parsed > 0 ? parsed : null; } +/** + * What the payload behind a provider `tmdb_id` says about that id. + * + * - `corroborated`: title or release year agrees; use the details. + * - `contradicted`: both years are known and they disagree — the classic + * stale id ("Blade Runner 2049" carrying the 1982 film's id). Only this + * verdict is strong enough to let the title search take over. + * - `inconclusive`: the title differs and there is no year to arbitrate + * with. Keep the details: TMDB returns titles in the REQUEST language, + * and normalization strips stylized prefixes ("IT - Chapter Two"), so a + * name mismatch alone says more about our own inputs than about the id. + */ +export type ProviderIdVerdict = + | 'corroborated' + | 'contradicted' + | 'inconclusive'; + +/** The same tolerance the search gate uses, applied in reverse */ +function yearsAgree( + providerYear: number, + detailsYear: number, + mediaType: TmdbMediaType +): boolean { + if (Math.abs(detailsYear - providerYear) <= 1) { + return true; + } + // Series: portals report the current season's year while TMDB reports + // the premiere, so a show that started earlier still agrees. + return mediaType === 'tv' && detailsYear < providerYear; +} + +export function assessProviderId( + details: { + title?: string; + original_title?: string; + release_date?: string; + name?: string; + original_name?: string; + first_air_date?: string; + }, + query: { title?: string | null; originalTitle?: string | null; year?: number | null }, + mediaType: TmdbMediaType +): ProviderIdVerdict { + // Same effective year the search would use, so the two agree on what + // "the provider's year" means + const providerYear = query.year ?? extractYear(null, query.title); + const detailsYear = extractYear( + mediaType === 'movie' ? details.release_date : details.first_air_date + ); + + if (providerYear !== null && detailsYear !== null) { + return yearsAgree(providerYear, detailsYear, mediaType) + ? 'corroborated' + : 'contradicted'; + } + + return detailsMatchProviderTitle(details, query) + ? 'corroborated' + : 'inconclusive'; +} + +/** + * Does a details payload plausibly describe the item we asked about? + * Title-only signal — see {@link assessProviderId} for the verdict callers + * should act on. + */ +export function detailsMatchProviderTitle( + details: { + title?: string; + original_title?: string; + name?: string; + original_name?: string; + }, + query: { title?: string | null; originalTitle?: string | null } +): boolean { + const variants = new Set( + buildSearchTitleVariants(query.title, query.originalTitle) + ); + if (variants.size === 0) { + // Nothing to compare against — never call that a mismatch + return true; + } + + return [ + details.title, + details.original_title, + details.name, + details.original_name, + ].some((title) => { + const normalized = normalizeTitle(title); + return normalized !== '' && variants.has(normalized); + }); +} + function resultTitles(result: TmdbSearchResult, mediaType: TmdbMediaType) { return mediaType === 'movie' ? [result.title, result.original_title] diff --git a/libs/services/src/lib/tmdb/tmdb-provider-id.spec.ts b/libs/services/src/lib/tmdb/tmdb-provider-id.spec.ts new file mode 100644 index 000000000..99c199191 --- /dev/null +++ b/libs/services/src/lib/tmdb/tmdb-provider-id.spec.ts @@ -0,0 +1,92 @@ +import { assessProviderId } from './tmdb-matcher'; + +describe('assessProviderId', () => { + it('corroborates on a matching year even when the title reads wrong', () => { + // "IT - Chapter Two" normalizes to "chapter two" because the + // ALL-CAPS "IT" looks like a language tag + expect( + assessProviderId( + { + title: 'It Chapter Two', + release_date: '2019-09-04', + }, + { title: 'IT - Chapter Two', year: 2019 }, + 'movie' + ) + ).toBe('corroborated'); + }); + + it('is inconclusive when the title differs and no year can arbitrate', () => { + // Never 'contradicted': normalization artifacts and localized + // titles both land here, and the id is usually fine + expect( + assessProviderId( + { title: 'It Chapter Two' }, + { title: 'IT - Chapter Two' }, + 'movie' + ) + ).toBe('inconclusive'); + }); + + it('contradicts a same-title id from another year', () => { + // Both normalize to "blade runner" — only the year sees the swap + expect( + assessProviderId( + { title: 'Blade Runner', release_date: '1982-06-25' }, + { title: 'Blade Runner 2049', year: 2017 }, + 'movie' + ) + ).toBe('contradicted'); + }); + + it('reads the year out of the provider title when there is no field', () => { + expect( + assessProviderId( + { title: 'The Lion King', release_date: '1994-06-24' }, + { title: 'The Lion King 2019' }, + 'movie' + ) + ).toBe('contradicted'); + }); + + it('allows a one-year drift between provider and TMDB dates', () => { + expect( + assessProviderId( + { title: 'Some Film', release_date: '2018-12-28' }, + { title: 'Totally Other Name', year: 2019 }, + 'movie' + ) + ).toBe('corroborated'); + }); + + it('accepts a series that premiered before the reported season year', () => { + // Portals report the current season's year, TMDB the premiere + expect( + assessProviderId( + { name: 'The Boys', first_air_date: '2019-07-25' }, + { title: 'The Boys', year: 2026 }, + 'tv' + ) + ).toBe('corroborated'); + }); + + it('contradicts a series that started after the reported year', () => { + expect( + assessProviderId( + { name: 'Something Else', first_air_date: '2024-01-01' }, + { title: 'The Boys', year: 2019 }, + 'tv' + ) + ).toBe('contradicted'); + }); + + it('corroborates on the title when neither side has a year', () => { + expect( + assessProviderId( + { title: 'The Matrix' }, + { title: 'The Matrix' }, + 'movie' + ) + ).toBe('corroborated'); + }); +});