From ab963c81d2a0174d5d020a433470b0992202f8ff Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 30 Sep 2026 03:26:57 +0200 Subject: [PATCH] =?UTF-8?q?fix(plex,audit):=20E30=20review=20round=201=20?= =?UTF-8?q?=E2=80=94=20the=20people=20job=20keeps=20going,=20HU=20genres?= =?UTF-8?q?=20everywhere,=20results=20as=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - people_enrich: a failed TMDB lookup or a dead headshot is skipped, not the end of the run (a dead file is marked and left for a week; three failures in a row end it); portraits wait for the Plex module switch; a no-Plex original keeps its own cache key, so the resize still replaces it; the job entry lives in the plex domain (run_people_enrich) - person-image takes only a headshot-shaped CDN url (no query variants of one file) - every synced row gets Hungarian genres, and a HU genre filter matches the word as picked too; migration 0094 fills the rows no overlay reached - a job run's audit row carries its result as data; audit_clear / demo_reset store their counts - one credit-column list for the prune and the enrichment order; _overlay_pass typed - tests: batch skip + stop, HU bio stored, dead headshots, module switch, the person-image route, the job body, genre filter + facet write + twin fallback, tagline/summary fallbacks, audit data, the reader's alphabet at every remaining collated() call site --- .../0094_plex_genres_hu_everywhere.py | 45 ++++ backend/app/plex/imgcache.py | 24 +++ backend/app/plex/people.py | 118 ++++++---- backend/app/plex/query.py | 12 +- backend/app/plex/sync.py | 19 +- backend/app/plex/tmdb.py | 37 +++- backend/app/routes/admin.py | 6 +- backend/app/routes/audit.py | 8 +- backend/app/routes/plex/playback.py | 9 +- backend/app/scheduler.py | 30 +-- backend/tests/test_collation.py | 93 ++++++++ backend/tests/test_plex_lang.py | 32 ++- backend/tests/test_plex_overlay.py | 36 ++++ backend/tests/test_plex_people_enrich.py | 201 +++++++++++++++--- backend/tests/test_scheduler.py | 19 ++ 15 files changed, 571 insertions(+), 118 deletions(-) create mode 100644 backend/alembic/versions/0094_plex_genres_hu_everywhere.py diff --git a/backend/alembic/versions/0094_plex_genres_hu_everywhere.py b/backend/alembic/versions/0094_plex_genres_hu_everywhere.py new file mode 100644 index 00000000..5e9c47f0 --- /dev/null +++ b/backend/alembic/versions/0094_plex_genres_hu_everywhere.py @@ -0,0 +1,45 @@ +"""Hungarian genres on every Plex row, not only on overlaid ones (E30 review) + +Revision ID: 0094_plex_genres_hu_everywhere +Revises: 0093_plex_person_known_for_votes +Create Date: 2026-09-30 + +The HU genre facets and filter read `genres_hu`, which only the overlay pass wrote — so a library with +no HU twin, or a guid-less row, offered English genres a Hungarian filter could never match. The sync +now writes `genres_hu` for every row (plex/sync.py `_apply_facet_fields`); this fills the rows it has +not reached yet, from their own English list through the same map. +""" + +import json + +import sqlalchemy as sa +from alembic import op + +from app.plex.genres import hu_genres + +revision: str = "0094_plex_genres_hu_everywhere" +down_revision: str | None = "0093_plex_person_known_for_votes" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + conn = op.get_bind() + for table in ("plex_items", "plex_shows"): + rows = conn.execute( + sa.text( + f"SELECT id, genres FROM {table} " + "WHERE genres_hu IS NULL AND jsonb_typeof(genres) = 'array'" + ) + ).fetchall() + updates = [ + {"id": rid, "g": json.dumps(hu)} for rid, genres in rows if (hu := hu_genres(genres)) + ] + if updates: + conn.execute( + sa.text(f"UPDATE {table} SET genres_hu = CAST(:g AS jsonb) WHERE id = :id"), updates + ) + + +def downgrade() -> None: + pass # derived from `genres`; the sync writes the same values again diff --git a/backend/app/plex/imgcache.py b/backend/app/plex/imgcache.py index 1053e714..130c241d 100644 --- a/backend/app/plex/imgcache.py +++ b/backend/app/plex/imgcache.py @@ -8,6 +8,9 @@ few hundred MB at most for a big library — acceptable, matching the download d The cache directory (``cache.CACHE_DIR``) is shared with the subtitle cache (see ``subtitles.py``). """ +import time +from datetime import timedelta + from fastapi import Response from sqlalchemy.orm import Session @@ -78,6 +81,27 @@ def drop(key: str) -> None: pass +def mark_miss(key: str) -> None: + """Remember that `key`'s source could not be fetched (an empty `.miss` file; its mtime is when), so + a prefetch can leave a dead source alone for a while. Best-effort.""" + try: + CACHE_DIR.mkdir(parents=True, exist_ok=True) + (CACHE_DIR / f"{key}.miss").touch() + # silent-ok: without the mark the dead source is only asked for again on the next run. + except OSError: + pass + + +def missed_within(key: str, age: timedelta) -> bool: + """`key` was marked as a miss less than `age` ago.""" + try: + mtime = (CACHE_DIR / f"{key}.miss").stat().st_mtime + # silent-ok: no mark (the usual case). + except OSError: + return False + return time.time() - mtime < age.total_seconds() + + def write(key: str, data: bytes, ct: str) -> None: ext = _CT_EXT.get((ct or "").split(";")[0].strip(), "jpg") try: diff --git a/backend/app/plex/people.py b/backend/app/plex/people.py index 88e6012e..0e1e6d61 100644 --- a/backend/app/plex/people.py +++ b/backend/app/plex/people.py @@ -4,6 +4,8 @@ orphaned people. Used by BOTH the info-page lazy enrichment (routes.plex) and th re-sync (plex.sync), so the two paths never diverge.""" import hashlib +import re +from datetime import timedelta from typing import Literal from urllib.parse import quote, unquote @@ -12,7 +14,7 @@ from sqlalchemy import select, text from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.orm import Session -from app import logs +from app import logs, sysconfig from app.models import PlexPerson from app.plex import imgcache from app.plex.client import PlexClient, PlexError, PlexNotConfigured @@ -97,14 +99,23 @@ def enrich_row_people(db: Session, row, cast: list[dict]) -> bool: return dirty +# Every credit reference: one person name per (movie/show, credit column). The ONE list of credit +# columns, so the orphan prune and the enrichment order never disagree about who is credited. +_CREDIT_COLUMNS = [ + (table, col) for table in ("plex_items", "plex_shows") for col in ("cast_names", "directors") +] + + +def _credit_refs_sql(union: Literal["UNION", "UNION ALL"]) -> str: + return f" {union} ".join( + f"SELECT jsonb_array_elements_text({col}) FROM {table} WHERE jsonb_typeof({col}) = 'array'" + for table, col in _CREDIT_COLUMNS + ) + + # The person names still referenced by ANY movie/show (cast OR directors). plex_people rows outside # this set are orphans left by a library removal. -_REFERENCED_PEOPLE_SQL = """ - SELECT jsonb_array_elements_text(cast_names) FROM plex_items WHERE jsonb_typeof(cast_names) = 'array' - UNION SELECT jsonb_array_elements_text(directors) FROM plex_items WHERE jsonb_typeof(directors) = 'array' - UNION SELECT jsonb_array_elements_text(cast_names) FROM plex_shows WHERE jsonb_typeof(cast_names) = 'array' - UNION SELECT jsonb_array_elements_text(directors) FROM plex_shows WHERE jsonb_typeof(directors) = 'array' -""" +_REFERENCED_PEOPLE_SQL = _credit_refs_sql("UNION") def prune_orphan_people(db: Session) -> int: @@ -125,14 +136,9 @@ def prune_orphan_people(db: Session) -> int: # How often each person is credited across movies, shows and episodes (cast OR director) — the order the # people_enrich job works in, so the faces met most often are local first. -_CREDIT_COUNTS_SQL = """ - SELECT ref, count(*) AS n FROM ( - SELECT jsonb_array_elements_text(cast_names) FROM plex_items WHERE jsonb_typeof(cast_names) = 'array' - UNION ALL SELECT jsonb_array_elements_text(directors) FROM plex_items WHERE jsonb_typeof(directors) = 'array' - UNION ALL SELECT jsonb_array_elements_text(cast_names) FROM plex_shows WHERE jsonb_typeof(cast_names) = 'array' - UNION ALL SELECT jsonb_array_elements_text(directors) FROM plex_shows WHERE jsonb_typeof(directors) = 'array' - ) AS refs(ref) GROUP BY ref -""" +_CREDIT_COUNTS_SQL = ( + f"SELECT ref, count(*) AS n FROM ({_credit_refs_sql('UNION ALL')}) AS refs(ref) GROUP BY ref" +) _NEEDS = { # linked to TMDB, bio never fetched "bio": "p.tmdb_person_id IS NOT NULL AND p.tmdb_enriched_at IS NULL", @@ -160,34 +166,49 @@ def people_by_credits( # A Plex CDN headshot is a full-size original (~170 KB on average, measured). Plex's photo transcoder # fits it into this box (~13 KB), which covers the largest place a portrait is drawn (the person page's # 6rem avatar) at 2x density and a raised text size. The box is part of the cache key, like a poster's: -# a new box re-caches instead of serving a stale size, and the full-size original under the old key is -# removed as its small twin lands. +# a new box re-caches instead of serving a stale size. Only a resized image is ever written under it — +# an original fetched without Plex keeps its own key, and goes once its small twin lands. PORTRAIT_BOX = 320 +# A headshot Plex answered with an error for (a dead CDN file) is not asked for again this long, so a +# few dead faces never hold up the rest of the prefetch. +PORTRAIT_RETRY = timedelta(days=7) +# The one shape a Plex CDN headshot has (…/7/people/.jpg): no query, no fragment. The +# person-image proxy takes nothing else, so it cannot be made to cache endless variants of one file +# (the image cache has no eviction). +_PORTRAIT_URL = re.compile(re.escape(PERSON_IMG_HOST) + r"[A-Za-z0-9_./-]+") + + +def is_portrait_url(url: str) -> bool: + return _PORTRAIT_URL.fullmatch(url) is not None def _portrait_keys(url: str) -> tuple[str, str]: + """(the resized portrait's key, the full-size original's key).""" base = "person_" + hashlib.sha1(url.encode()).hexdigest() return f"{base}_{PORTRAIT_BOX}", base def portrait_cached(url: str) -> tuple[bytes, str] | None: - return imgcache.read(_portrait_keys(url)[0]) + """The cached portrait: the resized one, else an original kept from a time without Plex.""" + small, original = _portrait_keys(url) + return imgcache.read(small) or imgcache.read(original) def cache_portrait(plex: PlexClient | None, url: str) -> tuple[bytes, str]: - """Fetch one CDN headshot — resized through Plex when a Plex client is given, else the original - straight from the public CDN — and cache it. Raises PlexError / httpx.HTTPError on a failed fetch.""" - key, legacy = _portrait_keys(url) - if plex is not None: - data, ct = plex.image_bytes(url, PORTRAIT_BOX, PORTRAIT_BOX) - else: + """Fetch one CDN headshot and cache it. With a Plex client it is resized (the box key; the original + is removed); without one the original comes straight from the public CDN and is cached under its own + key, so the job still resizes it once Plex is there. Raises PlexError / httpx.HTTPError.""" + small, original = _portrait_keys(url) + if plex is None: with httpx.Client(timeout=10.0, follow_redirects=True) as c: r = c.get(url) r.raise_for_status() data, ct = r.content, r.headers.get("content-type", "image/jpeg") - imgcache.write(key, data, ct) - if plex is not None: - imgcache.drop(legacy) + imgcache.write(original, data, ct) + return data, ct + data, ct = plex.image_bytes(url, PORTRAIT_BOX, PORTRAIT_BOX) + imgcache.write(small, data, ct) + imgcache.drop(original) return data, ct @@ -195,17 +216,23 @@ def cdn_url(photo_thumb: str) -> str | None: """The CDN url inside a stored proxied `/person-image?u=` value (see person_thumb_url).""" _, _, q = photo_thumb.partition("?u=") url = unquote(q) - return url if url.startswith(PERSON_IMG_HOST) else None + return url if is_portrait_url(url) else None + + +# Plex not answering this many headshots in a row ends a run: it is Plex that is failing, not the files. +_PLEX_DOWN_AFTER = 3 def warm_portraits(db: Session, limit: int) -> dict: """Cache up to `limit` not-yet-cached headshots, the most-credited people first (E30 S6), so a - person's face comes from local disk. Needs Plex (the resize); without it, nothing is prefetched — - full-size originals for every person would cost gigabytes. A failed fetch is skipped; one failing - Plex ends the run.""" + person's face comes from local disk. Needs Plex (the resize) and the Plex module on; without them + nothing is prefetched — full-size originals for every person would cost gigabytes. A headshot Plex + answers with an error for is marked and left for PORTRAIT_RETRY; Plex not answering ends the run.""" + if not sysconfig.get_bool(db, "plex_enabled"): + return {"cached": 0, "skipped": "disabled"} try: plex = PlexClient(db) - # silent-ok: Plex off is a setting, not a failure; the run's result says "disabled". + # silent-ok: no Plex server set up is a setting, not a failure; the run's result says "disabled". except PlexNotConfigured: return {"cached": 0, "skipped": "disabled"} photos: dict[int, str] = { @@ -215,25 +242,34 @@ def warm_portraits(db: Session, limit: int) -> dict: ).tuples() if thumb } - done = failed = 0 + done = dead = down = 0 with plex: for pid in people_by_credits(db, "portrait", None): - if done + failed >= limit: + if done + dead >= limit or down >= _PLEX_DOWN_AFTER: break url = cdn_url(photos.get(pid) or "") - if url is None or imgcache.has(_portrait_keys(url)[0]): + if url is None: + continue + key = _portrait_keys(url)[0] + if imgcache.has(key) or imgcache.missed_within(key, PORTRAIT_RETRY): continue try: cache_portrait(plex, url) - done += 1 - except (PlexError, httpx.HTTPError) as exc: - failed += 1 + done, down = done + 1, 0 + except PlexError as exc: + # An error status is Plex's answer about this one file; no answer is Plex itself. + if isinstance(exc.__cause__, httpx.HTTPStatusError): + dead += 1 + imgcache.mark_miss(key) + else: + down += 1 log.warning( "a portrait could not be cached", extra=logs.event( - "plex.people.portrait_failed", person_id=pid, reason=type(exc).__name__ + "plex.people.portrait_failed", + person_id=pid, + reason=type(exc.__cause__).__name__, ), ) - if failed >= 5: - break + failed = dead + down return {"cached": done, **({"failed": failed} if failed else {})} diff --git a/backend/app/plex/query.py b/backend/app/plex/query.py index 2bcf6366..636d34f1 100644 --- a/backend/app/plex/query.py +++ b/backend/app/plex/query.py @@ -182,13 +182,15 @@ def meta_filter_conds(model, p: dict, lang: str = "en") -> list: Genre chips are language-specific strings, so they filter the active-language genre column.""" conds: list = [] gsel = split_csv(p.get("genres")) - if gsel and lang == "hu": - # The HU column holds one Hungarian word per genre (plex/genres.py); a filter saved while it - # still offered "Action" is read as "Akció", so it keeps matching instead of emptying the grid. - gsel = list(dict.fromkeys(hu_genre(g) for g in gsel)) if gsel: gcol = genres_col(model, lang) - gc = [gcol.contains([g]) for g in gsel] + if lang == "hu": + # The HU column holds one Hungarian word per genre (plex/genres.py), so a filter saved while + # it offered "Action" is read as "Akció" too. The word as picked still matches as well: a row + # the sync has not reached since keeps its English (or Plex's mixed) list until it does. + gc = [or_(*(gcol.contains([w]) for w in dict.fromkeys((hu_genre(g), g)))) for g in gsel] + else: + gc = [gcol.contains([g]) for g in gsel] conds.append(and_(*gc) if p.get("genre_mode") == "all" else or_(*gc)) crs = split_csv(p.get("content_ratings")) if crs: diff --git a/backend/app/plex/sync.py b/backend/app/plex/sync.py index b8e05dfd..90f358b9 100644 --- a/backend/app/plex/sync.py +++ b/backend/app/plex/sync.py @@ -15,6 +15,7 @@ dead `enabled` column) is a separate follow-up. Cast + intro/credit markers are item-detail/player path (P2), not here. """ +from collections.abc import Iterable from datetime import datetime, timezone from sqlalchemy import func, select @@ -449,6 +450,10 @@ def _apply_facet_fields(row, meta: dict) -> None: row.studio = (meta.get("studio") or None) and str(meta.get("studio"))[:255] row.originally_available_at = _date(meta.get("originallyAvailableAt")) row.genres = _tags(meta, "Genre") + # Every row has Hungarian genres, not only one with an HU overlay twin (the overlay pass, which runs + # after this, puts the twin's own list over it): the HU genre facets and filter read this column, + # and an English word there could never match a filter read in Hungarian. + row.genres_hu = hu_genres(row.genres) row.directors = _tags(meta, "Director") # The listing carries only the top ~20 cast, so re-applying it would DOWNGRADE an already-enriched # full cast (up to 24, grown by the rich backfill/info-page enrich), silently breaking the @@ -702,13 +707,19 @@ def _sync_overlay( def _overlay_pass( - db: Session, plex: PlexClient, lib: PlexLibrary, item_type: int, rows, stats: dict -): + db: Session, + plex: PlexClient, + lib: PlexLibrary, + item_type: int, + rows: Iterable[PlexItem] | Iterable[PlexShow], + stats: dict, +) -> None: """One kind of the overlay (movies, shows or episodes) — see `_sync_overlay`.""" # guid -> the EN rows carrying it (a list: two editions of one film can share a guid; both overlay). - by_guid: dict[str, list] = {} + by_guid: dict[str, list[PlexItem | PlexShow]] = {} for r in rows: - by_guid.setdefault(r.guid, []).append(r) + if r.guid: # the passes select guid-bearing rows only + by_guid.setdefault(r.guid, []).append(r) # Reconcile step: clear HU fields on all guid-bearing rows. Only rows that actually held HU data # become dirty (SQLAlchemy skips a no-net-change UPDATE), so this is cheap; matches are re-set below. # The genres are the exception: every row gets Hungarian ones, from its HU twin or, lacking one, from diff --git a/backend/app/plex/tmdb.py b/backend/app/plex/tmdb.py index bd980516..0f0896c9 100644 --- a/backend/app/plex/tmdb.py +++ b/backend/app/plex/tmdb.py @@ -24,7 +24,7 @@ from app import logs, sysconfig from app.config import settings from app.models import PlexItem, PlexPerson, PlexShow from app.plex.cards import display_title -from app.plex.people import people_by_credits +from app.plex.people import people_by_credits, warm_portraits log = logs.get_logger(__name__) # Politeness throttle: TMDB tolerates ~50 req/s; ~33/s leaves comfortable headroom for a shared host. @@ -417,6 +417,19 @@ def run_enrich(db: Session) -> dict: return {"enriched": n, **({"countries_coded": coded} if coded else {})} +def run_people_enrich(db: Session) -> dict: + """Scheduler entry point of people_enrich (E30 S6): a batch of person bios (the most-credited + first) and a batch of portraits. Each half is a no-op — never an error — without its TMDB key / + without Plex.""" + batch = settings.people_enrich_batch + if sysconfig.get_str(db, "tmdb_api_key"): + with TmdbClient(db) as tmdb: + bios = enrich_people_batch(db, tmdb, batch) + else: + bios = {"skipped": "tmdb_api_key not set"} + return {"bios": bios, "portraits": warm_portraits(db, batch)} + + def resolve_library_guids( db: Session, entries: list[dict] | None, @@ -627,14 +640,21 @@ def enrich_person(db: Session, person: PlexPerson) -> bool: return True +# Lookups failing this many people in a row end a people batch: TMDB itself is degraded, and hammering +# it helps nobody. One person failing alone is skipped, so an id TMDB keeps refusing never stalls the rest. +_PEOPLE_FAILS_IN_A_ROW = 3 + + def enrich_people_batch(db: Session, tmdb: TmdbClient, limit: int) -> dict: """Fetch up to `limit` linked, not-yet-enriched people (E30 S6), the most-credited first, so a person page reads its bio from the row instead of waiting on TMDB. Each person is committed as it - lands: a page open during the run already sees it. A transient TmdbError ends the batch (TMDB is - degraded; hammering it helps nobody) and the rest wait for the next run.""" + lands: a page open during the run already sees it. A failed lookup is skipped (left unstamped for the + next run); _PEOPLE_FAILS_IN_A_ROW of them in a row end the batch.""" ids = people_by_credits(db, "bio", limit) - done = 0 + done = failed = in_a_row = 0 for pid in ids: + if in_a_row >= _PEOPLE_FAILS_IN_A_ROW: + break person = db.get(PlexPerson, pid) # A page open may have enriched this person since the select: skip, don't fetch twice. if person is None or person.tmdb_enriched_at is not None: @@ -643,13 +663,14 @@ def enrich_people_batch(db: Session, tmdb: TmdbClient, limit: int) -> dict: _fetch_person_into(tmdb, person) except TmdbError as exc: db.rollback() + failed, in_a_row = failed + 1, in_a_row + 1 log.warning( - "the TMDB person batch stopped on a failed lookup", + "a TMDB person lookup in the batch failed", extra=logs.event( "plex.tmdb.people_batch_failed", person_id=pid, reason=type(exc).__name__ ), ) - return {"enriched": done, "failed": 1} + continue db.commit() - done += 1 - return {"enriched": done} + done, in_a_row = done + 1, 0 + return {"enriched": done, **({"failed": failed} if failed else {})} diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index b58ff724..a24c0326 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -834,7 +834,11 @@ def reset_demo(admin: User = Depends(admin_user), db: Session = Depends(get_db)) demo = get_or_create_demo_user(db) seeded = reset_demo_state(db, demo) audit.record( - db, admin.id, AuditAction.DEMO_RESET, summary=f"{seeded} sample playlists re-seeded" + db, + admin.id, + AuditAction.DEMO_RESET, + summary=f"{seeded} sample playlists re-seeded", + after={"playlists_seeded": seeded}, # the count as data: named in the reader's language ) db.commit() return {"ok": True, "reset": True, "playlists_seeded": seeded} diff --git a/backend/app/routes/audit.py b/backend/app/routes/audit.py index d9a907b9..1c007231 100644 --- a/backend/app/routes/audit.py +++ b/backend/app/routes/audit.py @@ -66,6 +66,12 @@ def list_audit( def clear_audit(admin: User = Depends(admin_user), db: Session = Depends(get_db)) -> dict: """Clear the audit log. Leaves ONE row recording who cleared it + how many (tamper-evidence).""" n = db.execute(delete(AuditLog)).rowcount or 0 - audit.record(db, admin.id, AuditAction.AUDIT_CLEAR, summary=f"Cleared {n} audit entries") + audit.record( + db, + admin.id, + AuditAction.AUDIT_CLEAR, + summary=f"Cleared {n} audit entries", + after={"cleared": n}, # the count as data: the log names it in the reader's language + ) db.commit() return {"ok": True, "cleared": n} diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index d39ad6e7..562973a4 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -213,11 +213,12 @@ def _resume_push_superseded(user_id: int, item_id: int, intent: datetime) -> boo @router.get("/person-image") def person_image(u: str, request: Request) -> Response: - """Proxy a cast/crew photo from Plex's public metadata CDN (host-whitelisted; no token needed), - resized through Plex and cached (plex.people.cache_portrait; the people_enrich job fills the same - cache ahead of time). Signed-session auth; like /image, the DB session is released before the fetch.""" + """Proxy a cast/crew photo from Plex's public metadata CDN (only a headshot-shaped URL on its host; + no token needed), resized through Plex and cached (plex.people.cache_portrait; the people_enrich job + fills the same cache ahead of time). Signed-session auth; like /image, the DB session is released + before the fetch.""" _require_session(request) - if not u.startswith(plex_people.PERSON_IMG_HOST): + if not plex_people.is_portrait_url(u): raise refused(400, "image_host") hit = plex_people.portrait_cached(u) if hit: diff --git a/backend/app/scheduler.py b/backend/app/scheduler.py index ca6b4257..feaf43fd 100644 --- a/backend/app/scheduler.py +++ b/backend/app/scheduler.py @@ -18,6 +18,7 @@ from app.downloads.gc import run_download_gc from app.models import SchedulerSetting, User from app.plex.sync import sync as run_plex_sync from app.plex.tmdb import run_enrich as run_tmdb_enrich +from app.plex.tmdb import run_people_enrich from app.plex.watch_sync import run_plex_watch_reconcile, run_plex_watch_sync from app.notifications import create_notification from app.state import is_sync_paused @@ -160,6 +161,9 @@ def _audit_run(db, actor_id, job_id, status, summary, result) -> None: they changed something, so routine no-op polls don't spam the trail. Best-effort.""" if actor_id is None and status == "ok" and not _run_changed(result): return + # The result as data too (E30): the audit log names it in the reader's language, as the inbox + # does; `summary` stays the English line of the row's own summary column. + data = _result_data(result) try: audit.record( db, @@ -168,7 +172,7 @@ def _audit_run(db, actor_id, job_id, status, summary, result) -> None: target_type="job", target_id=job_id, summary=f"{job_id}: {status}" + (f" — {summary}" if summary else ""), - after={"status": status, "summary": summary}, + after={"status": status, "summary": summary, **({"result": data} if data else {})}, ) db.commit() except Exception: @@ -205,7 +209,7 @@ def _job(name: str, fn) -> None: ) if actor_id is not None: _notify_done(db, actor_id, name, "skipped", "sync paused", _PAUSED) - _audit_run(db, actor_id, name, "skipped", "sync paused", None) + _audit_run(db, actor_id, name, "skipped", "sync paused", _PAUSED) return with progress.bind(sink): result = fn(db) @@ -252,7 +256,7 @@ _PAUSED = {"skipped": "sync paused"} def _result_data(result) -> dict | None: """A job's result as the client names it in the reader's language (E30): the dict itself (counts, a `skipped` / `reason` word, a nested dict per phase), `enrich`'s bare count as one. The one-line - `_summarize` is the English fallback — and what the audit log keeps.""" + `_summarize` is the English fallback.""" if isinstance(result, dict): return result or None if isinstance(result, int) and not isinstance(result, bool): @@ -410,23 +414,9 @@ def _tmdb_enrich_job() -> None: def _people_enrich_job() -> None: - # Make the people local (E30 S6): a batch of person bios from TMDB (the most-credited first) and a - # batch of portraits resized through Plex into the image cache, so a person page and a cast strip - # never wait on an outside service. Each half is a no-op without its key / without Plex. - def work(db): - from app import sysconfig - from app.plex import people as plex_people - from app.plex import tmdb as plex_tmdb - - batch = settings.people_enrich_batch - if sysconfig.get_str(db, "tmdb_api_key"): - with plex_tmdb.TmdbClient(db) as tmdb: - bios = plex_tmdb.enrich_people_batch(db, tmdb, batch) - else: - bios = {"skipped": "tmdb_api_key not set"} - return {"bios": bios, "portraits": plex_people.warm_portraits(db, batch)} - - _job("people_enrich", work) + # Make the people local (E30 S6): a batch of person bios from TMDB and a batch of portraits resized + # through Plex, so a person page and a cast strip never wait on an outside service. + _job("people_enrich", run_people_enrich) def _audit_gc_job() -> None: diff --git a/backend/tests/test_collation.py b/backend/tests/test_collation.py index f1d0ba42..2558be23 100644 --- a/backend/tests/test_collation.py +++ b/backend/tests/test_collation.py @@ -207,3 +207,96 @@ def test_channel_discovery_keeps_that_order_after_enriching(db, monkeypatch, ove _seed_discovery(db, user, synced=False) r = _client(db, user, monkeypatch, override_dep).get("/api/channels/discovery") assert [c["title"] for c in r.json()] == HU + + +# --- The other lists the server puts in the reader's alphabet (E30 review): each call site, once. --- + + +def _others(db, n_offset: int = 0) -> list[User]: + users = [ + User(email=f"{w.lower()}@example.com", display_name=w) for w in WORDS + ] # emails and names in WORDS' order + db.add_all(users) + db.flush() + return users + + +def test_the_share_and_message_recipients_are_in_the_readers_alphabet( + db, monkeypatch, override_dep +): + from app.models import MessageKey + + user = _user(db, "hu") + others = _others(db) + db.add_all( + MessageKey( + user_id=u.id, + public_key="k", + wrapped_private_key="w", + salt="s", + wrap_iv="i", + key_check="c", + ) + for u in others + ) + db.commit() + client = _client(db, user, monkeypatch, override_dep) + emails = [r["email"] for r in client.get("/api/downloads/recipients").json()] + assert emails == [f"{w.lower()}@example.com" for w in HU] + names = [r["name"] for r in client.get("/api/messages/users").json()] + assert names == HU + + +def test_the_blocked_channels_are_in_the_readers_alphabet(db, monkeypatch, override_dep): + from app.models import BlockedChannel + + user = _user(db, "hu") + db.add_all(Channel(id=f"UC{i}", title=w) for i, w in enumerate(WORDS)) + db.flush() + db.add_all(BlockedChannel(user_id=user.id, channel_id=f"UC{i}") for i in range(len(WORDS))) + db.commit() + r = _client(db, user, monkeypatch, override_dep).get("/api/channels/blocked/list") + assert [c["title"] for c in r.json()] == HU + + +def test_the_plex_libraries_and_collections_are_in_the_readers_alphabet( + db, monkeypatch, override_dep +): + user = _user(db, "hu") + libs = [ + PlexLibrary(plex_key=str(i), title=w, kind="movie", enabled=True) + for i, w in enumerate(WORDS) + ] + db.add_all(libs) + db.flush() + db.add_all( + PlexCollection(rating_key=f"c{i}", library_id=libs[0].id, title=w, child_count=1) + for i, w in enumerate(WORDS) + ) + db.commit() + client = _client(db, user, monkeypatch, override_dep) + assert [lib["title"] for lib in client.get("/api/plex/libraries").json()["libraries"]] == HU + cols = client.get("/api/plex/collections").json()["collections"] + assert [c["title"] for c in cols] == HU + + +def test_a_persons_titles_of_one_year_are_in_the_readers_alphabet(db, monkeypatch, override_dep): + user = _user(db, "hu") + lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) + db.add(lib) + db.flush() + db.add_all( + PlexItem( + rating_key=f"m{i}", + library_id=lib.id, + kind="movie", + title=f"EN {i}", + title_hu=w, + year=2000, + cast_names=["Anna"], + ) + for i, w in enumerate(WORDS) + ) + db.commit() + body = _client(db, user, monkeypatch, override_dep).get("/api/plex/person/Anna?lang=hu").json() + assert [m["title"] for m in body["movies"]] == HU diff --git a/backend/tests/test_plex_lang.py b/backend/tests/test_plex_lang.py index 15465fe1..00da79c9 100644 --- a/backend/tests/test_plex_lang.py +++ b/backend/tests/test_plex_lang.py @@ -13,6 +13,7 @@ from app.auth import current_user from app.db import get_db from app.main import app from app import sysconfig +from app.plex.cards import display_summary, display_title from app.models import ( PlexCollection, PlexItem, @@ -57,7 +58,9 @@ def _seed(db): library_id=lib.id, kind="movie", title="EN Only", - genres=["Comedy"], # no HU twin + summary="English only.", + tagline="English only tagline.", + genres=["Comedy"], # no HU twin, never reached by a sync since: no genres_hu year=2002, added_at=datetime(2024, 1, 2, tzinfo=timezone.utc), people_enriched_at=datetime(2024, 1, 2, tzinfo=timezone.utc), @@ -125,6 +128,33 @@ def test_a_genre_filter_saved_in_english_still_matches_in_hungarian(db, monkeypa assert _titles(resp) == {"Magyar Cím"} +def test_a_genre_the_hungarian_list_offers_in_english_still_matches(db, monkeypatch, override_dep): + """E30 review: a row the sync has not given Hungarian genres yet is listed under its English word + (the facets read coalesce(genres_hu, genres)); filtering by that word must find it.""" + user = _seed(db) + facets = _get(db, user, monkeypatch, override_dep, "/api/plex/facets?scope=movie&lang=hu") + assert "Comedy" in {g["value"] for g in facets.json()["genres"]} + resp = _get( + db, user, monkeypatch, override_dep, "/api/plex/library?scope=movie&lang=hu&genres=Comedy" + ) + assert _titles(resp) == {"EN Only"} + + +def test_a_title_without_a_hungarian_twin_reads_in_english(db, monkeypatch, override_dep): + user = _seed(db) + body = _get(db, user, monkeypatch, override_dep, "/api/plex/item/m2?lang=hu").json() + assert (body["title"], body["summary"], body["tagline"]) == ( + "EN Only", + "English only.", + "English only tagline.", + ) + row = PlexItem(title="T", summary="S") + assert (display_title(row, "hu"), display_summary(row, "hu")) == ("T", "S") + row.title_hu, row.summary_hu = "HT", "HS" + assert (display_title(row, "hu"), display_summary(row, "hu")) == ("HT", "HS") + assert (display_title(row, "en"), display_summary(row, "en")) == ("T", "S") + + def test_item_detail_language_and_rating_split(db, monkeypatch, override_dep): user = _seed(db) resp = _get(db, user, monkeypatch, override_dep, "/api/plex/item/m1?lang=hu") diff --git a/backend/tests/test_plex_overlay.py b/backend/tests/test_plex_overlay.py index ae8fab37..4d1706e8 100644 --- a/backend/tests/test_plex_overlay.py +++ b/backend/tests/test_plex_overlay.py @@ -334,6 +334,42 @@ def test_the_overlay_gives_every_row_one_hungarian_word_per_genre(db): assert lone.tagline_hu is None +def test_a_twin_without_genres_keeps_the_rows_own_hungarian_list(db): + en_lib = PlexLibrary(plex_key="4", title="Movies", kind="movie") + hu_lib = PlexLibrary(plex_key="9", title="Filmek", kind="movie", metadata_lang="hu") + db.add_all([en_lib, hu_lib]) + db.flush() + row = PlexItem( + rating_key="m1", + library_id=en_lib.id, + kind="movie", + title="Heat", + guid="tmdb:1", + genres=["Crime"], + ) + db.add(row) + db.commit() + _sync_overlay( + db, + _StubPlex([_hu_movie("h1", "Szemtől szemben", "HU.", [], "tmdb://1")]), + hu_lib, + en_lib.id, + {}, + ) + db.commit() + db.refresh(row) + assert (row.title_hu, row.genres_hu) == ("Szemtől szemben", ["Bűnügyi"]) + + +def test_every_synced_row_gets_hungarian_genres(): + """E30 review: not only an overlaid row — the HU facets and filter read genres_hu for all.""" + from app.plex.sync import _apply_facet_fields + + row = PlexItem(title="Solo") + _apply_facet_fields(row, {"Genre": [{"tag": "Drama"}, {"tag": "Action"}]}) + assert (row.genres, row.genres_hu) == (["Drama", "Action"], ["Dráma", "Akció"]) + + def test_the_genre_map_is_idempotent_and_keeps_an_unknown_word(): assert hu_genre("science fiction") == "Sci-Fi" # case-insensitive assert hu_genre("Dráma") == "Dráma" diff --git a/backend/tests/test_plex_people_enrich.py b/backend/tests/test_plex_people_enrich.py index b866b688..c59cfeb9 100644 --- a/backend/tests/test_plex_people_enrich.py +++ b/backend/tests/test_plex_people_enrich.py @@ -6,6 +6,7 @@ DB lane (Postgres); the portrait cache runs in a temp directory. from datetime import datetime, timezone +import httpx import pytest from app import scheduler @@ -32,13 +33,13 @@ def _library(db): class _Tmdb: """A TmdbClient stand-in: records the ids asked for, fails on `fail_on`.""" - def __init__(self, fail_on=None): + def __init__(self, fail_on=()): self.asked: list[int] = [] - self.fail_on = fail_on + self.fail_on = set(fail_on) def fetch_person(self, pid): self.asked.append(pid) - if pid == self.fail_on: + if pid in self.fail_on: raise plex_tmdb.TmdbError("429") return {"biography": f"Bio {pid}."} @@ -67,22 +68,52 @@ def test_the_batch_takes_the_most_credited_linked_people_first(db): assert db.query(PlexPerson).filter_by(name="Rare").one().tmdb_enriched_at is None # next run -def test_a_failed_lookup_ends_the_batch_and_keeps_what_landed(db): +def test_a_failed_lookup_is_skipped_and_the_rest_still_land(db): db.add_all([PlexPerson(name="A", tmdb_person_id=1), PlexPerson(name="B", tmdb_person_id=2)]) db.add(PlexPerson(name="C", tmdb_person_id=3)) db.commit() - tmdb = _Tmdb(fail_on=2) - assert plex_tmdb.enrich_people_batch(db, tmdb, 10) == {"enriched": 1, "failed": 1} # type: ignore[arg-type] - assert tmdb.asked == [1, 2] # C is not asked: TMDB is struggling + tmdb = _Tmdb(fail_on={2}) + assert plex_tmdb.enrich_people_batch(db, tmdb, 10) == {"enriched": 2, "failed": 1} # type: ignore[arg-type] + assert tmdb.asked == [1, 2, 3] # B refused: skipped, not the end of the batch db.expire_all() stamped = {p.name: p.tmdb_enriched_at is not None for p in db.query(PlexPerson)} - assert stamped == {"A": True, "B": False, "C": False} + assert stamped == {"A": True, "B": False, "C": True} # B is tried again next run + + +def test_failures_in_a_row_end_the_batch(db): + db.add_all([PlexPerson(name=f"P{i}", tmdb_person_id=i) for i in range(1, 7)]) + db.commit() + tmdb = _Tmdb(fail_on={1, 2, 3, 4, 5, 6}) # TMDB is down + assert plex_tmdb.enrich_people_batch(db, tmdb, 10) == {"enriched": 0, "failed": 3} # type: ignore[arg-type] + assert tmdb.asked == [1, 2, 3] + + +def test_the_batch_stores_the_hungarian_biography(db): + db.add(PlexPerson(name="A", tmdb_person_id=1)) + db.commit() + + class _WithHu(_Tmdb): + def fetch_person(self, pid): + return { + "biography": "English.", + "translations": { + "translations": [{"iso_639_1": "hu", "data": {"biography": "Magyar."}}] + }, + } + + assert plex_tmdb.enrich_people_batch(db, _WithHu(), 10) == {"enriched": 1} # type: ignore[arg-type] + db.expire_all() + person = db.query(PlexPerson).one() + assert (person.biography, person.biography_hu) == ("English.", "Magyar.") class _Plex: - def __init__(self, fail=False): + """A PlexClient stand-in. `down`: Plex does not answer; `dead`: urls Plex answers with an error.""" + + def __init__(self, down=False, dead=()): self.asked: list[tuple[str, int | None]] = [] - self.fail = fail + self.down = down + self.dead = set(dead) def __enter__(self): return self @@ -92,8 +123,14 @@ class _Plex: def image_bytes(self, url, width=None, height=None): self.asked.append((url, width)) - if self.fail: - raise plex_people.PlexError("down") + if self.down: + raise plex_people.PlexError("down") from httpx.ConnectError("refused") + if url in self.dead: + req = httpx.Request("GET", url) + status = httpx.HTTPStatusError( + "404", request=req, response=httpx.Response(404, request=req) + ) + raise plex_people.PlexError("404") from status return b"small", "image/jpeg" @@ -103,7 +140,26 @@ def cache_dir(tmp_path, monkeypatch): return tmp_path -def test_portraits_are_cached_small_and_replace_the_full_size_original(db, cache_dir, monkeypatch): +@pytest.fixture +def plex_on(monkeypatch): + """The Plex module switched on (the job's gate), with `plex` as its client.""" + state = {"enabled": True, "plex": _Plex()} + monkeypatch.setattr(plex_people.sysconfig, "get_bool", lambda d, k: state["enabled"]) + monkeypatch.setattr(plex_people, "PlexClient", lambda d: state["plex"]) + return state + + +def _faces(db, n): + db.add_all( + [ + PlexPerson(name=f"P{i}", photo_thumb=plex_people.person_thumb_url(CDN + f"{i}.jpg")) + for i in range(n) + ] + ) + db.commit() + + +def test_portraits_are_cached_small_and_replace_the_full_size_original(db, cache_dir, plex_on): lib = _library(db) _movie(db, lib, "a", ["Often"]) _movie(db, lib, "b", ["Often", "Cached"]) @@ -115,23 +171,21 @@ def test_portraits_are_cached_small_and_replace_the_full_size_original(db, cache ] ) db.commit() - small, legacy = plex_people._portrait_keys(CDN + "often.jpg") - imgcache.write(legacy, b"a big original", "image/jpeg") # from before E30 S6 + small, original = plex_people._portrait_keys(CDN + "often.jpg") + imgcache.write(original, b"a big original", "image/jpeg") # cached without Plex imgcache.write(plex_people._portrait_keys(CDN + "cached.jpg")[0], b"x", "image/jpeg") - plex = _Plex() - monkeypatch.setattr(plex_people, "PlexClient", lambda d: plex) + plex = plex_on["plex"] assert plex_people.warm_portraits(db, 10) == {"cached": 1} assert plex.asked == [ (CDN + "often.jpg", plex_people.PORTRAIT_BOX) ] # resized, the cached one skipped assert imgcache.read(small) == (b"small", "image/jpeg") - assert not imgcache.has(legacy) + assert not imgcache.has(original) -def test_portraits_wait_for_plex(db, cache_dir, monkeypatch): - db.add(PlexPerson(name="P", photo_thumb=plex_people.person_thumb_url(CDN + "p.jpg"))) - db.commit() +def test_portraits_wait_for_plex(db, cache_dir, plex_on, monkeypatch): + _faces(db, 1) def not_configured(d): raise plex_people.PlexNotConfigured() @@ -141,18 +195,99 @@ def test_portraits_wait_for_plex(db, cache_dir, monkeypatch): assert list(cache_dir.iterdir()) == [] # no full-size originals behind Plex's back -def test_a_failing_plex_ends_the_portrait_run(db, cache_dir, monkeypatch): - db.add_all( - [ - PlexPerson(name=f"P{i}", photo_thumb=plex_people.person_thumb_url(CDN + f"{i}.jpg")) - for i in range(8) - ] - ) - db.commit() - plex = _Plex(fail=True) - monkeypatch.setattr(plex_people, "PlexClient", lambda d: plex) - assert plex_people.warm_portraits(db, 100) == {"cached": 0, "failed": 5} - assert len(plex.asked) == 5 +def test_portraits_wait_for_the_plex_module_to_be_on(db, cache_dir, plex_on): + _faces(db, 1) + plex_on["enabled"] = False # switched off, its server still set up + assert plex_people.warm_portraits(db, 10) == {"cached": 0, "skipped": "disabled"} + assert plex_on["plex"].asked == [] + + +def test_plex_not_answering_ends_the_portrait_run(db, cache_dir, plex_on): + _faces(db, 8) + plex_on["plex"] = _Plex(down=True) + assert plex_people.warm_portraits(db, 100) == {"cached": 0, "failed": 3} + assert len(plex_on["plex"].asked) == 3 + assert not list(cache_dir.glob("*.miss")) # Plex was down, the files are not to blame + + +def test_dead_headshots_are_left_alone_and_the_rest_still_land(db, cache_dir, plex_on): + _faces(db, 8) + dead = {CDN + f"{i}.jpg" for i in range(6)} # more dead files than any failure limit + plex_on["plex"] = _Plex(dead=dead) + assert plex_people.warm_portraits(db, 100) == {"cached": 2, "failed": 6} + + plex_on["plex"] = again = _Plex(dead=dead) + assert plex_people.warm_portraits(db, 100) == {"cached": 0} + assert again.asked == [] # dead ones marked, live ones cached: nothing to ask for + + +def test_the_person_image_route(db, cache_dir, monkeypatch): + from fastapi.testclient import TestClient + + import app.state as state + from app.main import app + from app.routes.plex import playback + + monkeypatch.setattr(state, "_configured_cache", True) + monkeypatch.setattr(playback, "_require_session", lambda request: 1) + client = TestClient(app) + url = CDN + "face.jpg" + small, original = plex_people._portrait_keys(url) + + def get(u): + return client.get("/api/plex/person-image", params={"u": u}) + + # Only a headshot-shaped url on the CDN host: no query variants of one file, no other host. + for bad in (url + "?n=1", url + "#x", "https://example.com/people/a.jpg"): + assert get(bad).status_code == 400, bad + + # No Plex: the original from the CDN, kept under its own key (the job resizes it later). + def not_configured(d): + raise plex_people.PlexNotConfigured() + + class _Cdn: + def __init__(self, *a, **k): + pass + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def get(self, u): + return httpx.Response( + 200, + content=b"original", + headers={"content-type": "image/jpeg"}, + request=httpx.Request("GET", u), + ) + + monkeypatch.setattr(playback, "PlexClient", not_configured) + monkeypatch.setattr(plex_people.httpx, "Client", _Cdn) + resp = get(url) + assert (resp.status_code, resp.content) == (200, b"original") + assert imgcache.has(original) and not imgcache.has(small) + + # With Plex: resized, and it replaces the original. + monkeypatch.setattr(playback, "PlexClient", lambda d: _Plex()) + imgcache.drop(original) + resp = get(url) + assert (resp.status_code, resp.content) == (200, b"small") + assert imgcache.has(small) + assert get(url).content == b"small" # the cache answers now + + monkeypatch.setattr(playback, "PlexClient", lambda d: _Plex(down=True)) + assert get(CDN + "other.jpg").status_code == 502 + + +def test_the_people_job_runs_its_halves_without_a_tmdb_key(db, cache_dir, plex_on, monkeypatch): + _faces(db, 1) + monkeypatch.setattr(plex_tmdb.sysconfig, "get_str", lambda d, k: "") + assert plex_tmdb.run_people_enrich(db) == { + "bios": {"skipped": "tmdb_api_key not set"}, + "portraits": {"cached": 1}, + } def test_a_phase_count_is_a_change_worth_an_audit_row(): diff --git a/backend/tests/test_scheduler.py b/backend/tests/test_scheduler.py index ab77e44d..f77cd6c8 100644 --- a/backend/tests/test_scheduler.py +++ b/backend/tests/test_scheduler.py @@ -67,3 +67,22 @@ def test_a_failed_run_drops_the_previous_result(db): rss = _snapshot_job("rss_poll") assert rss["last_error"] == "boom" assert rss["result"] is None + + +def test_a_job_runs_audit_row_carries_its_result_as_data(db): + """E30 review: the audit log names a run's result in the reader's language, as the inbox does — + from data, not from the English one-liner (a nested phase there was a Python dict repr).""" + from app.models import AuditLog, User + + admin = User(email="auditrun@example.com") + db.add(admin) + db.commit() + scheduler._audit_run(db, None, "people_enrich", "ok", "x", {"bios": {"enriched": 2}}) + scheduler._audit_run(db, admin.id, "rss_poll", "skipped", "sync paused", scheduler._PAUSED) + scheduler._audit_run(db, admin.id, "rss_poll", "error", "boom", None) + rows = db.execute(select(AuditLog).order_by(AuditLog.id)).scalars().all() + assert [r.after for r in rows] == [ + {"status": "ok", "summary": "x", "result": {"bios": {"enriched": 2}}}, + {"status": "skipped", "summary": "sync paused", "result": {"skipped": "sync paused"}}, + {"status": "error", "summary": "boom"}, + ]