diff --git a/.env.example b/.env.example index a5f3443..e7c48f3 100644 --- a/.env.example +++ b/.env.example @@ -42,3 +42,10 @@ CACHE_DEFAULT_TTL=300 # --- Rate limiting --- RATE_LIMIT_ENABLED=true RATE_LIMIT_PER_MINUTE=120 + +# --- Artwork storage --- +# Local disk, shared between the api and worker containers via a volume +# (see docker-compose.yml) -- the worker downloads here, the api serves it. +MEDIA_ROOT=./media +MEDIA_URL_PREFIX=/media +ARTWORK_DOWNLOAD_MAX_BYTES=15728640 diff --git a/README.md b/README.md index a216558..323e0a2 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,22 @@ curl http://localhost:8000/api/v1/admin/ingest/jobs/ \ curl "http://localhost:8000/api/v1/search?q=the%20matrix" ``` +Ingesting a series also queues its posters/backdrops/profile images for +local download automatically (only the provider's *primary* image per +entity, not every alternate) — see the next section. + +## Artwork + +Images are referenced by their provider CDN URL (TMDb) from the moment +they're ingested, and cached locally in the background afterward — API +responses point at the provider until a local copy exists, then switch over +automatically, with no client-visible change. Cached files live under +`MEDIA_ROOT` (a Docker volume shared between `api` and `worker`, see +`docker-compose.yml`) and are served at `MEDIA_URL_PREFIX` (default +`/media`). See [`docs/architecture.md`](docs/architecture.md#artwork-self-hosting-section-10) +and [`docs/database.md`](docs/database.md#artwork-storage-self-hosting) for +how it works and its current single-host-deployment assumption. + ## Tests ```bash @@ -170,14 +186,8 @@ See [`docs/database.md#known-limitations`](docs/database.md#known-limitations) and [`docs/api.md#known-limitations`](docs/api.md#known-limitations) for the current, tracked list (no curated language/country seed data, no fuzzy entity matching, no field-level provenance, no pagination beyond search, -person credits limited to movies). None of these block the Core 0.1 -milestone (section 34): search → movie/person detail works end-to-end. - -This repository was built in an environment without Docker or a -pre-existing PostgreSQL instance; the initial Alembic migration was -hand-written from the models (not autogenerated against a live database) -and validated with `alembic history`, but has not yet been executed against -a running PostgreSQL instance. **Run `docker compose up --build` and confirm -`alembic upgrade head` succeeds against a genuinely empty database before -treating the schema as final** — see -[`docs/database.md#migrations`](docs/database.md#migrations). +person credits' episode-level guest appearances rolled up to a per-series +count rather than broken out individually). None of these block the Core +0.1 milestone (section 34): search → movie/series/person detail works +end-to-end against real TMDb data, verified on a real PostgreSQL + Redis + +Docker deployment (not just this repo's own dev machine). diff --git a/app/core/config.py b/app/core/config.py index bd2cd2a..61046f6 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -65,6 +65,20 @@ class Settings(BaseSettings): RATE_LIMIT_ENABLED: bool = Field(default=True) RATE_LIMIT_PER_MINUTE: int = Field(default=120) + # --- Artwork storage (section 10) --- + # Local-disk self-hosting, deliberately simple for 0.1: fine for a + # single-host deployment where the api and worker containers share one + # volume (see docker-compose.yml). A genuinely multi-host deployment + # would need real object storage (S3/R2/...) behind the same + # ArtworkStorage interface instead — see app/services/artwork/storage.py. + MEDIA_ROOT: str = Field(default="./media") + # Root-relative URL path artwork is served under (app/main.py mounts + # MEDIA_ROOT there). Kept relative rather than a full domain so it keeps + # working whether Aurora is reached via localhost, a Traefik host, or + # (once configured) amdbs.net. + MEDIA_URL_PREFIX: str = Field(default="/media") + ARTWORK_DOWNLOAD_MAX_BYTES: int = Field(default=15 * 1024 * 1024) + @field_validator("AURORA_ENV") @classmethod def _validate_env(cls, v: str) -> str: diff --git a/app/main.py b/app/main.py index a1297f7..b3f17af 100644 --- a/app/main.py +++ b/app/main.py @@ -10,11 +10,13 @@ import time import uuid from collections.abc import Awaitable, Callable from contextlib import asynccontextmanager +from pathlib import Path from fastapi import FastAPI, Request, Response from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse +from fastapi.staticfiles import StaticFiles from prometheus_client import CONTENT_TYPE_LATEST, Counter, Histogram, generate_latest from starlette.exceptions import HTTPException as StarletteHTTPException @@ -78,6 +80,17 @@ app.add_middleware( allow_headers=["*"], ) +# Serves what app/services/artwork/ downloads (section 10: self-hosted +# artwork). The directory must exist before StaticFiles will mount — the +# worker creates subdirectories as it writes files, but the root itself +# needs to be there even before the first download. +Path(settings.MEDIA_ROOT).mkdir(parents=True, exist_ok=True) +app.mount( + settings.MEDIA_URL_PREFIX, + StaticFiles(directory=settings.MEDIA_ROOT), + name="media", +) + @app.middleware("http") async def request_context_middleware( diff --git a/app/repositories/polymorphic.py b/app/repositories/polymorphic.py index a95d5f6..b532cea 100644 --- a/app/repositories/polymorphic.py +++ b/app/repositories/polymorphic.py @@ -6,6 +6,8 @@ an (entity_type, entity_id) pair rather than a typed foreign key. from __future__ import annotations import uuid +from collections.abc import Sequence +from typing import Protocol from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -18,6 +20,24 @@ from app.models.external_id import ExternalID from app.models.provider import Provider from app.models.video import Video +_VALID_ARTWORK_TYPES = {t.value for t in ArtworkType} + + +class _NormalizedArtworkLike(Protocol): + """Structural type for app.services.metadata.normalizer.NormalizedArtwork + — a repository shouldn't import from the services layer, so this + describes the shape `sync_artwork` needs instead of the concrete class. + """ + + artwork_type: str + url: str + provider_asset_id: str | None + width: int | None + height: int | None + aspect_ratio: float | None + vote_average: float | None + is_primary: bool + async def get_external_ids( session: AsyncSession, entity_type: str, entity_id: uuid.UUID @@ -48,6 +68,104 @@ async def get_external_ids_bulk( return out +async def get_primary_artwork_ids( + session: AsyncSession, entity_type: str, entity_ids: list[uuid.UUID] +) -> list[uuid.UUID]: + """IDs of artwork rows marked `is_primary` for any of `entity_ids` — the + small subset of a title's images (TMDb's own selected poster/backdrop/ + profile) that get auto-downloaded for local hosting, out of the dozens + of alternates ingestion also stores. See app/services/artwork/.""" + + if not entity_ids: + return [] + stmt = select(Artwork.id).where( + Artwork.entity_type == entity_type, + Artwork.entity_id.in_(entity_ids), + Artwork.is_primary.is_(True), + ) + result = await session.execute(stmt) + return list(result.scalars().all()) + + +async def sync_artwork( + session: AsyncSession, + *, + entity_type: str, + entity_id: uuid.UUID, + provider_id: uuid.UUID, + artwork_list: Sequence[_NormalizedArtworkLike], +) -> None: + """Upsert an entity's artwork rows keyed by `provider_asset_id` (TMDb's + stable per-image file path), instead of deleting and recreating them on + every re-sync. + + This matters specifically because of `app/services/artwork/` (section + 10's self-hosting): a delete-then-recreate replace would generate a + fresh Artwork.id with `local_path=None` every time a title gets + re-ingested, silently throwing away every previously-downloaded local + copy and forcing a full re-download on each sync — defeating the point + of caching them locally at all. Rows for images no longer present in + `artwork_list` (dropped from the provider's alternates) are still + deleted, same as before. + """ + + existing = ( + ( + await session.execute( + select(Artwork).where( + Artwork.entity_type == entity_type, Artwork.entity_id == entity_id + ) + ) + ) + .scalars() + .all() + ) + existing_by_asset_id = { + row.provider_asset_id: row for row in existing if row.provider_asset_id is not None + } + + seen_asset_ids: set[str] = set() + for art in artwork_list: + art_type = art.artwork_type if art.artwork_type in _VALID_ARTWORK_TYPES else "poster" + matched = ( + existing_by_asset_id.get(art.provider_asset_id) if art.provider_asset_id else None + ) + if matched is not None: + seen_asset_ids.add(art.provider_asset_id) # type: ignore[arg-type] + matched.artwork_type = ArtworkType(art_type) + matched.width = art.width + matched.height = art.height + matched.aspect_ratio = art.aspect_ratio + matched.vote_average = art.vote_average + matched.is_primary = art.is_primary + matched.original_url = art.url + # local_path/checksum deliberately untouched — see docstring. + else: + session.add( + Artwork( + entity_type=entity_type, + entity_id=entity_id, + artwork_type=ArtworkType(art_type), + width=art.width, + height=art.height, + aspect_ratio=art.aspect_ratio, + provider_id=provider_id, + provider_asset_id=art.provider_asset_id, + original_url=art.url, + vote_average=art.vote_average, + is_primary=art.is_primary, + ) + ) + if art.provider_asset_id: + seen_asset_ids.add(art.provider_asset_id) + + for row in existing: + if row.provider_asset_id not in seen_asset_ids: + await session.delete(row) + + await session.flush() + + async def get_artwork( session: AsyncSession, entity_type: str, diff --git a/app/services/artwork/__init__.py b/app/services/artwork/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/services/artwork/download.py b/app/services/artwork/download.py new file mode 100644 index 0000000..874359d --- /dev/null +++ b/app/services/artwork/download.py @@ -0,0 +1,110 @@ +"""Downloads one Artwork row's original_url and stores it locally +(section 10; the "Artwork processing" background worker from section 1's +architecture diagram). + +Deliberately narrow: only ever called for artwork already marked +`is_primary` (see `app/services/ingestion/runner.py:get_primary_artwork_ids` +and `app/workers/tasks.py`, which cascade a download job per primary image +after each ingestion succeeds) — TMDb returns dozens of backdrop/logo +alternates per title, and downloading every one of them automatically would +be a lot of unnecessary storage and bandwidth for images Aurora's own +clients are unlikely to ever show. Downloading on request for anything +beyond the primary image is a reasonable follow-up if that's ever needed. +""" + +from __future__ import annotations + +import hashlib +import uuid + +import httpx +from sqlalchemy.ext.asyncio import AsyncSession + +from app.core.config import get_settings +from app.core.logging import get_logger +from app.models.artwork import Artwork +from app.services.artwork.storage import ArtworkStorage + +logger = get_logger(__name__) + +_EXTENSION_BY_CONTENT_TYPE = { + "image/jpeg": "jpg", + "image/jpg": "jpg", + "image/png": "png", + "image/webp": "webp", + "image/gif": "gif", + "image/svg+xml": "svg", +} + + +class ArtworkDownloadError(Exception): + """Raised for any download-side failure (bad content-type, too large, + network error, ...). Callers (the arq task) let this propagate so the + job gets retried like any other ingestion failure — see + app/services/ingestion/runner.py.""" + + +async def download_artwork( + session: AsyncSession, + storage: ArtworkStorage, + artwork_id: uuid.UUID, + *, + client: httpx.AsyncClient | None = None, + force: bool = False, +) -> bool: + """Fetch `artwork.original_url`, store it, and set `local_path` + + `checksum`. Returns False (no-op) if the row is gone or already has a + local copy (unless `force=True`); True if it downloaded something. + """ + + artwork = await session.get(Artwork, artwork_id) + if artwork is None: + return False + if artwork.local_path and not force: + return False + + settings = get_settings() + owns_client = client is None + http_client = client or httpx.AsyncClient(timeout=20.0, follow_redirects=True) + try: + content, content_type = await _fetch(http_client, artwork.original_url, settings.ARTWORK_DOWNLOAD_MAX_BYTES) + finally: + if owns_client: + await http_client.aclose() + + checksum = hashlib.sha256(content).hexdigest() + extension = _EXTENSION_BY_CONTENT_TYPE.get(content_type, "jpg") + relative_path = f"{artwork.artwork_type.value}/{checksum[:2]}/{checksum}.{extension}" + + local_url = await storage.save(relative_path, content) + + artwork.local_path = local_url + artwork.checksum = checksum + await session.flush() + logger.info( + "artwork_downloaded", + artwork_id=str(artwork_id), + checksum=checksum, + bytes=len(content), + ) + return True + + +async def _fetch(client: httpx.AsyncClient, url: str, max_bytes: int) -> tuple[bytes, str]: + try: + async with client.stream("GET", url) as response: + response.raise_for_status() + content_type = response.headers.get("content-type", "").split(";")[0].strip().lower() + if not content_type.startswith("image/"): + raise ArtworkDownloadError(f"expected an image, got content-type={content_type!r}") + + body = bytearray() + async for chunk in response.aiter_bytes(): + body.extend(chunk) + if len(body) > max_bytes: + raise ArtworkDownloadError( + f"artwork at {url} exceeds ARTWORK_DOWNLOAD_MAX_BYTES ({max_bytes} bytes)" + ) + return bytes(body), content_type + except httpx.HTTPError as exc: + raise ArtworkDownloadError(f"failed to download {url}: {exc}") from exc diff --git a/app/services/artwork/storage.py b/app/services/artwork/storage.py new file mode 100644 index 0000000..eef9583 --- /dev/null +++ b/app/services/artwork/storage.py @@ -0,0 +1,66 @@ +"""Artwork storage abstraction (section 10). + +Downloaded artwork bytes need somewhere to live that both the worker +(writing) and the API (serving, via app/main.py's static mount) can reach. +`ArtworkStorage` is the seam: today there's only `LocalDiskStorage` — fine +for a single Docker host where the api and worker containers share one +volume (see docker-compose.yml), which is this project's actual deployment +shape (section 1: modular monolith, not a multi-host fleet). A deployment +that outgrows a single host would swap in an S3/R2-backed implementation +behind this same interface without touching the download pipeline that +calls it (app/services/artwork/download.py). +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from functools import lru_cache +from pathlib import Path + +from app.core.config import get_settings + + +class ArtworkStorage(ABC): + @abstractmethod + async def save(self, relative_path: str, content: bytes) -> str: + """Persist `content` at `relative_path` (already includes an + extension, e.g. "poster/ab/ab34...jpg"). Returns the root-relative + URL clients should use to fetch it.""" + + @abstractmethod + async def exists(self, relative_path: str) -> bool: ... + + +class LocalDiskStorage(ArtworkStorage): + def __init__(self, root: str, url_prefix: str) -> None: + self._root = Path(root) + self._url_prefix = url_prefix.rstrip("/") + + def _full_path(self, relative_path: str) -> Path: + # relative_path is always built by us from a sha256 checksum, never + # from unsanitized external input — but guard anyway against ever + # writing outside MEDIA_ROOT. + root = self._root.resolve() + full = (self._root / relative_path).resolve() + if full != root and root not in full.parents: + raise ValueError(f"unsafe artwork path escapes MEDIA_ROOT: {relative_path!r}") + return full + + async def save(self, relative_path: str, content: bytes) -> str: + full = self._full_path(relative_path) + full.parent.mkdir(parents=True, exist_ok=True) + # Blocking I/O, but artwork files are small (capped by + # ARTWORK_DOWNLOAD_MAX_BYTES) and this only ever runs inside the + # worker process, not a request handler — not worth wiring aiofiles + # for. + full.write_bytes(content) + return f"{self._url_prefix}/{relative_path}" + + async def exists(self, relative_path: str) -> bool: + return self._full_path(relative_path).is_file() + + +@lru_cache +def get_artwork_storage() -> ArtworkStorage: + settings = get_settings() + return LocalDiskStorage(settings.MEDIA_ROOT, settings.MEDIA_URL_PREFIX) diff --git a/app/services/ingestion/movie_ingestion.py b/app/services/ingestion/movie_ingestion.py index 40c1ae1..98f7974 100644 --- a/app/services/ingestion/movie_ingestion.py +++ b/app/services/ingestion/movie_ingestion.py @@ -20,15 +20,15 @@ from sqlalchemy import delete from sqlalchemy.ext.asyncio import AsyncSession from app.core.logging import get_logger -from app.models.artwork import Artwork from app.models.company import MovieCompany from app.models.credit import Credit -from app.models.enums import ArtworkType, MatchConfidence, MovieStatus, VideoType +from app.models.enums import MatchConfidence, MovieStatus, VideoType from app.models.genre import MovieGenre from app.models.keyword import MovieKeyword from app.models.movie import Movie, MovieRecommendation from app.models.video import Video from app.providers.base import MetadataProvider +from app.repositories import polymorphic from app.repositories import reference_repository as refs from app.repositories.provider_repository import get_or_create_provider, upsert_provider_record from app.services.metadata.matcher import match_entity @@ -115,36 +115,9 @@ async def _replace_movie_credits( ) -_ARTWORK_TYPES = {t.value for t in ArtworkType} _VIDEO_TYPES = {t.value for t in VideoType} -async def _replace_movie_artwork( - session: AsyncSession, movie_id: uuid.UUID, provider_id: uuid.UUID, artwork_list -) -> None: - await session.execute( - delete(Artwork).where(Artwork.entity_type == "movie", Artwork.entity_id == movie_id) - ) - for art in artwork_list: - art_type = art.artwork_type if art.artwork_type in _ARTWORK_TYPES else "poster" - session.add( - Artwork( - entity_type="movie", - entity_id=movie_id, - artwork_type=ArtworkType(art_type), - language_code=None, - width=art.width, - height=art.height, - aspect_ratio=art.aspect_ratio, - provider_id=provider_id, - provider_asset_id=art.provider_asset_id, - original_url=art.url, - vote_average=art.vote_average, - is_primary=False, - ) - ) - - async def _replace_movie_videos( session: AsyncSession, movie_id: uuid.UUID, provider_id: uuid.UUID, videos ) -> None: @@ -259,7 +232,13 @@ async def ingest_movie( await _replace_movie_keywords(session, movie.id, normalized.keywords) await _replace_movie_companies(session, movie.id, tmdb_provider.id, normalized.companies) await _replace_movie_credits(session, movie.id, tmdb_provider.id, normalized.cast, normalized.crew) - await _replace_movie_artwork(session, movie.id, tmdb_provider.id, normalized.artwork) + await polymorphic.sync_artwork( + session, + entity_type="movie", + entity_id=movie.id, + provider_id=tmdb_provider.id, + artwork_list=normalized.artwork, + ) await _replace_movie_videos(session, movie.id, tmdb_provider.id, normalized.videos) await _link_external_ids(session, movie.id, tmdb_provider.id, normalized) diff --git a/app/services/ingestion/person_ingestion.py b/app/services/ingestion/person_ingestion.py index 8839fb0..38648ca 100644 --- a/app/services/ingestion/person_ingestion.py +++ b/app/services/ingestion/person_ingestion.py @@ -10,14 +10,13 @@ from __future__ import annotations import uuid -from sqlalchemy import delete from sqlalchemy.ext.asyncio import AsyncSession from app.core.logging import get_logger -from app.models.artwork import Artwork -from app.models.enums import ArtworkType, MatchConfidence +from app.models.enums import MatchConfidence from app.models.person import Person from app.providers.base import MetadataProvider +from app.repositories import polymorphic from app.repositories import reference_repository as refs from app.repositories.provider_repository import get_or_create_provider, upsert_provider_record from app.services.metadata.matcher import match_entity @@ -64,24 +63,13 @@ async def ingest_person( await session.flush() - await session.execute( - delete(Artwork).where(Artwork.entity_type == "person", Artwork.entity_id == person.id) + await polymorphic.sync_artwork( + session, + entity_type="person", + entity_id=person.id, + provider_id=tmdb_provider.id, + artwork_list=normalized.artwork, ) - for art in normalized.artwork: - session.add( - Artwork( - entity_type="person", - entity_id=person.id, - artwork_type=ArtworkType.PROFILE, - width=art.width, - height=art.height, - aspect_ratio=art.aspect_ratio, - provider_id=tmdb_provider.id, - provider_asset_id=art.provider_asset_id, - original_url=art.url, - vote_average=art.vote_average, - ) - ) await refs.link_external_id( session, diff --git a/app/services/ingestion/runner.py b/app/services/ingestion/runner.py index f018597..59c203c 100644 --- a/app/services/ingestion/runner.py +++ b/app/services/ingestion/runner.py @@ -19,6 +19,7 @@ from app.models.enums import IngestionJobStatus from app.models.provider import IngestionJob from app.providers.base import MetadataProvider from app.providers.tmdb import TMDbProvider +from app.repositories.polymorphic import get_primary_artwork_ids from app.repositories.provider_repository import ( create_ingestion_job, get_ingestion_job, @@ -165,3 +166,22 @@ async def get_series_season_numbers(series_id: uuid.UUID) -> list[int]: async with session_scope() as session: seasons = await get_seasons_for_series(session, series_id) return [season.season_number for season in seasons] + + +async def get_series_season_ids(series_id: uuid.UUID) -> list[uuid.UUID]: + async with session_scope() as session: + seasons = await get_seasons_for_series(session, series_id) + return [season.id for season in seasons] + + +async def get_primary_artwork_for_download( + entity_type: str, entity_ids: list[uuid.UUID] +) -> list[uuid.UUID]: + """Artwork ids worth auto-downloading for local hosting after an + ingestion succeeds — see app/services/artwork/download.py and + app/workers/tasks.py, which enqueue one download_artwork_task per id + this returns. A short-lived session of its own, same reasoning as + get_series_season_numbers above.""" + + async with session_scope() as session: + return await get_primary_artwork_ids(session, entity_type, entity_ids) diff --git a/app/services/ingestion/series_ingestion.py b/app/services/ingestion/series_ingestion.py index c05965b..633ad0f 100644 --- a/app/services/ingestion/series_ingestion.py +++ b/app/services/ingestion/series_ingestion.py @@ -20,17 +20,16 @@ from sqlalchemy import delete from sqlalchemy.ext.asyncio import AsyncSession from app.core.logging import get_logger -from app.models.artwork import Artwork from app.models.company import SeriesCompany from app.models.credit import Credit -from app.models.enums import ArtworkType, CompanyType, MatchConfidence, SeriesStatus, VideoType +from app.models.enums import CompanyType, MatchConfidence, SeriesStatus, VideoType from app.models.genre import SeriesGenre from app.models.keyword import SeriesKeyword from app.models.tv import Series from app.models.video import Video from app.providers.base import MetadataProvider, ProviderNotFoundError +from app.repositories import polymorphic, tv_repository from app.repositories import reference_repository as refs -from app.repositories import tv_repository from app.repositories.provider_repository import get_or_create_provider, upsert_provider_record from app.services.metadata.matcher import match_entity from app.services.metadata.normalizer import ( @@ -41,7 +40,6 @@ from app.services.metadata.normalizer import ( logger = get_logger(__name__) -_ARTWORK_TYPES = {t.value for t in ArtworkType} _VIDEO_TYPES = {t.value for t in VideoType} _LEADING_ARTICLES = ("the ", "a ", "an ") @@ -140,30 +138,6 @@ async def _replace_series_credits( ) -async def _replace_entity_artwork( - session: AsyncSession, entity_type: str, entity_id: uuid.UUID, provider_id: uuid.UUID, artwork_list -) -> None: - await session.execute( - delete(Artwork).where(Artwork.entity_type == entity_type, Artwork.entity_id == entity_id) - ) - for art in artwork_list: - art_type = art.artwork_type if art.artwork_type in _ARTWORK_TYPES else "poster" - session.add( - Artwork( - entity_type=entity_type, - entity_id=entity_id, - artwork_type=ArtworkType(art_type), - width=art.width, - height=art.height, - aspect_ratio=art.aspect_ratio, - provider_id=provider_id, - provider_asset_id=art.provider_asset_id, - original_url=art.url, - vote_average=art.vote_average, - ) - ) - - async def _replace_entity_videos( session: AsyncSession, entity_type: str, entity_id: uuid.UUID, provider_id: uuid.UUID, videos ) -> None: @@ -252,7 +226,13 @@ async def ingest_series( await _replace_series_credits( session, series.id, tmdb_provider.id, normalized.cast, normalized.crew, normalized.creators ) - await _replace_entity_artwork(session, "series", series.id, tmdb_provider.id, normalized.artwork) + await polymorphic.sync_artwork( + session, + entity_type="series", + entity_id=series.id, + provider_id=tmdb_provider.id, + artwork_list=normalized.artwork, + ) await _replace_entity_videos(session, "series", series.id, tmdb_provider.id, normalized.videos) for stub in normalized.seasons: @@ -262,12 +242,12 @@ async def ingest_series( season.air_date = stub.air_date season.episode_count = stub.episode_count await session.flush() - await _replace_entity_artwork( + await polymorphic.sync_artwork( session, - "season", - season.id, - tmdb_provider.id, - [stub.poster] if stub.poster else [], + entity_type="season", + entity_id=season.id, + provider_id=tmdb_provider.id, + artwork_list=[stub.poster] if stub.poster else [], ) await refs.link_external_id( @@ -373,7 +353,13 @@ async def ingest_season( season.episode_count = normalized.episode_count await session.flush() - await _replace_entity_artwork(session, "season", season.id, tmdb_provider.id, normalized.artwork) + await polymorphic.sync_artwork( + session, + entity_type="season", + entity_id=season.id, + provider_id=tmdb_provider.id, + artwork_list=normalized.artwork, + ) for ep in normalized.episodes: episode = await tv_repository.get_or_create_episode( @@ -390,7 +376,13 @@ async def ingest_season( await session.flush() await _replace_episode_credits(session, episode.id, tmdb_provider.id, ep.guest_stars, ep.crew) - await _replace_entity_artwork(session, "episode", episode.id, tmdb_provider.id, ep.stills) + await polymorphic.sync_artwork( + session, + entity_type="episode", + entity_id=episode.id, + provider_id=tmdb_provider.id, + artwork_list=ep.stills, + ) await refs.link_external_id( session, entity_type="episode", diff --git a/app/services/metadata/normalizer.py b/app/services/metadata/normalizer.py index fba4ae2..38aee05 100644 --- a/app/services/metadata/normalizer.py +++ b/app/services/metadata/normalizer.py @@ -66,6 +66,12 @@ class NormalizedArtwork: aspect_ratio: float | None language: str | None vote_average: float | None = None + # True for the single image the provider designates as *the* image for + # this entity (TMDb's top-level poster_path/backdrop_path/profile_path), + # as opposed to the full alternates list — this is what gets + # auto-downloaded for local hosting (app/services/artwork/), so it's + # deliberately narrow rather than "the first one in the list". + is_primary: bool = False @dataclass @@ -182,6 +188,7 @@ def normalize_movie(raw: dict) -> NormalizedMovie: None, None, None, + is_primary=True, ) ) if raw.get("backdrop_path"): @@ -194,6 +201,7 @@ def normalize_movie(raw: dict) -> NormalizedMovie: None, None, None, + is_primary=True, ) ) for kind, tmdb_key in (("poster", "posters"), ("backdrop", "backdrops"), ("logo", "logos")): @@ -309,19 +317,26 @@ def normalize_person(raw: dict) -> NormalizedPerson: ) for img in images ] - if raw.get("profile_path") and not any(a.provider_asset_id == raw["profile_path"] for a in artwork): - artwork.insert( - 0, - NormalizedArtwork( - "profile", - _image_url_required(raw["profile_path"]), - raw["profile_path"], - None, - None, - None, - None, - ), + if raw.get("profile_path"): + existing = next( + (a for a in artwork if a.provider_asset_id == raw["profile_path"]), None ) + if existing is not None: + existing.is_primary = True + else: + artwork.insert( + 0, + NormalizedArtwork( + "profile", + _image_url_required(raw["profile_path"]), + raw["profile_path"], + None, + None, + None, + None, + is_primary=True, + ), + ) also_known_as = raw.get("also_known_as") or [] @@ -473,6 +488,7 @@ def normalize_series(raw: dict) -> NormalizedSeries: None, None, None, + is_primary=True, ) ) if raw.get("backdrop_path"): @@ -485,6 +501,7 @@ def normalize_series(raw: dict) -> NormalizedSeries: None, None, None, + is_primary=True, ) ) for kind, tmdb_key in (("poster", "posters"), ("backdrop", "backdrops"), ("logo", "logos")): @@ -526,7 +543,7 @@ def normalize_series(raw: dict) -> NormalizedSeries: poster=( NormalizedArtwork( "poster", _image_url_required(s["poster_path"]), s["poster_path"], - None, None, None, None, + None, None, None, None, is_primary=True, ) if s.get("poster_path") else None @@ -621,7 +638,7 @@ def normalize_season(raw: dict) -> NormalizedSeason: artwork.append( NormalizedArtwork( "poster", _image_url_required(raw["poster_path"]), raw["poster_path"], - None, None, None, None, + None, None, None, None, is_primary=True, ) ) for img in images.get("posters", []): @@ -656,7 +673,7 @@ def normalize_season(raw: dict) -> NormalizedSeason: stills.append( NormalizedArtwork( "still", _image_url_required(ep["still_path"]), ep["still_path"], - None, None, None, None, + None, None, None, None, is_primary=True, ) ) episodes.append( diff --git a/app/workers/tasks.py b/app/workers/tasks.py index 6e55904..1cd941c 100644 --- a/app/workers/tasks.py +++ b/app/workers/tasks.py @@ -1,10 +1,15 @@ """arq task functions. Thin wrappers around the ingestion runner so retry/ concurrency/observability behavior lives in arq's worker loop, not here. -Each task receives the `IngestionJob` row's id (created synchronously by the -enqueuing endpoint via `get_or_create_ingestion_job`, *not* generated here) -so status polling has a real, immediately-valid id to look up from the -moment the HTTP response comes back. +Each ingest_* task receives the `IngestionJob` row's id (created +synchronously by the enqueuing endpoint via `get_or_create_ingestion_job`, +*not* generated here) so status polling has a real, immediately-valid id to +look up from the moment the HTTP response comes back. `download_artwork_task` +is the exception — it's an internal side effect of ingestion (see +`_cascade_artwork_downloads`), not something a client triggers or polls, so +it isn't tracked in `ingestion_jobs` at all; arq's own per-function retries +are enough for it (section 10: artwork self-hosting is best-effort caching, +not a user-facing operation with a status to report). """ from __future__ import annotations @@ -12,9 +17,14 @@ from __future__ import annotations import uuid from typing import Any +from app.core.database import session_scope from app.core.logging import get_logger +from app.services.artwork.download import download_artwork +from app.services.artwork.storage import get_artwork_storage from app.services.ingestion.runner import ( get_or_create_ingestion_job, + get_primary_artwork_for_download, + get_series_season_ids, get_series_season_numbers, run_movie_ingestion, run_person_ingestion, @@ -25,13 +35,29 @@ from app.services.ingestion.runner import ( logger = get_logger(__name__) +async def _cascade_artwork_downloads( + ctx: dict[str, Any], entity_type: str, entity_ids: list[uuid.UUID] +) -> None: + artwork_ids = await get_primary_artwork_for_download(entity_type, entity_ids) + if not artwork_ids: + return + redis = ctx["redis"] + for artwork_id in artwork_ids: + await redis.enqueue_job("download_artwork_task", str(artwork_id)) + logger.info( + "artwork_downloads_cascaded", entity_type=entity_type, count=len(artwork_ids) + ) + + async def ingest_movie_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str: movie_id = await run_movie_ingestion(uuid.UUID(job_id), external_id) + await _cascade_artwork_downloads(ctx, "movie", [movie_id]) return str(movie_id) async def ingest_person_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str: person_id = await run_person_ingestion(uuid.UUID(job_id), external_id) + await _cascade_artwork_downloads(ctx, "person", [person_id]) return str(person_id) @@ -39,6 +65,11 @@ async def ingest_series_task( ctx: dict[str, Any], job_id: str, external_id: str, include_seasons: bool = False ) -> str: series_id = await run_series_ingestion(uuid.UUID(job_id), external_id) + await _cascade_artwork_downloads(ctx, "series", [series_id]) + # Season posters come from the season *stubs* ingest_series already + # creates, independent of include_seasons (which only controls whether + # full per-season episode data gets fetched) -- so this always runs. + await _cascade_artwork_downloads(ctx, "season", await get_series_season_ids(series_id)) if include_seasons: # Fan out one ingest_season job per season this series has, using @@ -65,3 +96,8 @@ async def ingest_season_task( ) -> str: season_id = await run_season_ingestion(uuid.UUID(job_id), series_external_id, season_number) return str(season_id) + + +async def download_artwork_task(ctx: dict[str, Any], artwork_id: str) -> bool: + async with session_scope() as session: + return await download_artwork(session, get_artwork_storage(), uuid.UUID(artwork_id)) diff --git a/app/workers/worker.py b/app/workers/worker.py index 7a183a5..db156b4 100644 --- a/app/workers/worker.py +++ b/app/workers/worker.py @@ -14,6 +14,7 @@ from arq.connections import RedisSettings from app.core.config import get_settings from app.core.logging import configure_logging, get_logger from app.workers.tasks import ( + download_artwork_task, ingest_movie_task, ingest_person_task, ingest_season_task, @@ -33,7 +34,13 @@ async def shutdown(ctx: dict) -> None: class WorkerSettings: - functions = [ingest_movie_task, ingest_person_task, ingest_series_task, ingest_season_task] + functions = [ + ingest_movie_task, + ingest_person_task, + ingest_series_task, + ingest_season_task, + download_artwork_task, + ] on_startup = startup on_shutdown = shutdown redis_settings = RedisSettings.from_dsn(get_settings().REDIS_URL) diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 8660c87..688a3c9 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -87,6 +87,9 @@ services: DATABASE_BEHIND_TRANSACTION_POOLER: "true" DATABASE_URL: postgresql+asyncpg://${POSTGRES_USER:-aurora}:${POSTGRES_PASSWORD}@pgbouncer:5432/${POSTGRES_DB:-aurora_core} REDIS_URL: redis://redis:6379/0 + MEDIA_ROOT: /app/media + volumes: + - aurora_media_data:/app/media depends_on: pgbouncer: condition: service_healthy @@ -119,6 +122,9 @@ services: DATABASE_BEHIND_TRANSACTION_POOLER: "true" DATABASE_URL: postgresql+asyncpg://${POSTGRES_USER:-aurora}:${POSTGRES_PASSWORD}@pgbouncer:5432/${POSTGRES_DB:-aurora_core} REDIS_URL: redis://redis:6379/0 + MEDIA_ROOT: /app/media + volumes: + - aurora_media_data:/app/media depends_on: pgbouncer: condition: service_healthy @@ -135,3 +141,4 @@ services: volumes: aurora_postgres_data: aurora_redis_data: + aurora_media_data: diff --git a/docker-compose.yml b/docker-compose.yml index 679b09e..1cf2ea3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -54,6 +54,7 @@ services: DATABASE_URL: postgresql+asyncpg://aurora:aurora@postgres:5432/aurora_core DATABASE_URL_SYNC: postgresql+psycopg2://aurora:aurora@postgres:5432/aurora_core REDIS_URL: redis://redis:6379/0 + MEDIA_ROOT: /app/media ports: - "8000:8000" depends_on: @@ -67,6 +68,7 @@ services: ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000", "--reload"] volumes: - ./app:/app/app + - aurora_media_data:/app/media worker: build: @@ -78,6 +80,7 @@ services: DATABASE_URL: postgresql+asyncpg://aurora:aurora@postgres:5432/aurora_core DATABASE_URL_SYNC: postgresql+psycopg2://aurora:aurora@postgres:5432/aurora_core REDIS_URL: redis://redis:6379/0 + MEDIA_ROOT: /app/media depends_on: postgres: condition: service_healthy @@ -87,6 +90,8 @@ services: condition: service_completed_successfully volumes: - ./app:/app/app + - aurora_media_data:/app/media volumes: aurora_postgres_data: + aurora_media_data: diff --git a/docs/architecture.md b/docs/architecture.md index 3cac28a..7cc978a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -90,6 +90,40 @@ star can race to create the same `Person` row (see recovers from that specific race directly; the arq retry here is the backstop for whatever it doesn't). +## Artwork self-hosting (section 10) + +``` +ingest_movie/ingest_series/ingest_person succeeds + -> tasks.py: get_primary_artwork_for_download(entity_type, [entity_id, ...]) + -> one download_artwork_task enqueued per primary artwork id + -> download_artwork(): fetch original_url, verify it's an image, cap + size, sha256 it, storage.save(), set Artwork.local_path + checksum +``` + +Every image API responses reference is stored with its provider CDN URL +(`original_url`) from the moment it's ingested; `local_path` starts empty +and is filled in by this pipeline running asynchronously afterward, so +responses transparently point at TMDb until a local copy exists, then +switch over. Only artwork marked `is_primary` (the one image TMDb itself +designates for an entity, not the dozens of alternates also ingested) gets +downloaded automatically — see `docs/database.md#artwork-storage-self-hosting`. + +Storage is behind an `ArtworkStorage` interface +(`app/services/artwork/storage.py`) with one implementation, +`LocalDiskStorage` — a shared Docker volume between the `api` and `worker` +containers (`docker-compose.yml`'s `aurora_media_data`), which is enough +for this project's actual deployment shape (one Docker host, principle 9: +avoid premature complexity) but assumes exactly that. A deployment that +outgrew a single host would implement the same interface against S3/R2 +instead, without the download pipeline or callers changing. + +This is also why `app/repositories/polymorphic.py:sync_artwork` upserts +artwork by `provider_asset_id` instead of the delete-then-recreate pattern +every other satellite table uses (movie/series/person credits, videos, +genre/keyword/company links): a delete-then-recreate would generate a fresh +`Artwork.id` with `local_path=None` on every re-ingestion, silently +discarding every downloaded copy and forcing a full re-download each sync. + ## Search (section 16) `app/services/search/base.py:SearchProvider` is the abstraction; API diff --git a/docs/database.md b/docs/database.md index f19c6fb..6fb9151 100644 --- a/docs/database.md +++ b/docs/database.md @@ -71,6 +71,31 @@ externally-sourced images can be freely redistributed. Aurora never downloads video media; `videos.url` is a reference to the provider's hosted copy (YouTube, Vimeo, ...). +### Artwork storage (self-hosting) + +`artwork.original_url` (the provider's CDN link) is always populated by +ingestion; `artwork.local_path` starts `NULL` and is filled in +asynchronously by `app/services/artwork/`. `app/services/common.py:artwork_url()` +prefers `local_path` and falls back to `original_url`, so API responses +point at TMDb's CDN until a copy has been downloaded, then switch over +automatically once it has. + +Only artwork marked `is_primary` gets downloaded automatically — the single +image the provider designates as *the* image for an entity (TMDb's +`poster_path`/`backdrop_path`/`profile_path`), not the dozens of alternates +also stored. `app/services/ingestion/runner.py:get_primary_artwork_for_download` +finds them and `app/workers/tasks.py` enqueues a `download_artwork_task` per +id once movie/series/person/season ingestion succeeds — see +`docs/architecture.md` for the full flow. + +Storage is local disk (`app/services/artwork/storage.py:LocalDiskStorage`) +behind an `ArtworkStorage` interface, deliberately simple for a +single-Docker-host deployment where the `api` and `worker` containers share +one volume (`docker-compose.yml`'s `aurora_media_data`). A deployment that +outgrows one host would need an S3/R2-backed implementation of the same +interface instead. Files are named by content (`//.`), +so the same image referenced by two entities is only ever stored once. + ## Polymorphic associations `credits`, `artwork`, `videos`, `external_ids`, and `release_dates` attach to @@ -120,14 +145,15 @@ same lowercase strings. Alembic (section 27); the application **never** creates tables at startup — see `app/core/database.py`. The initial migration -(`migrations/versions/0001_initial_schema.py`) is hand-written rather than -autogenerated (no throwaway Postgres was available in the environment this -was built in — see the root README's Known Limitations), but was written -directly from the same models this document describes and validated with -`alembic history`; treat a first `alembic upgrade head` against a scratch -database as the real acceptance test, and autogenerate future migrations -against it (`alembic revision --autogenerate`) rather than the empty -database this one was written from. +(`migrations/versions/0001_initial_schema.py`) was hand-written rather than +autogenerated (no throwaway Postgres was available in the environment it +was originally written in), then verified by actually running +`alembic upgrade head` against a genuinely empty PostgreSQL on a real +devbox — that run caught a real bug (pre-creating enum types and then also +letting `CREATE TABLE` auto-create them raised `DuplicateObject`; fixed by +`create_type=False` on each enum), now fixed and verified clean. Autogenerate +future migrations against a database that already has this one applied +(`alembic revision --autogenerate`), not from scratch. ```bash alembic upgrade head # apply all migrations @@ -159,10 +185,15 @@ indexes on the alternate-title tables. See - Field-level provenance (which provider supplied *this* value) is not tracked, only entity-level (`source_provider_id`) and per-provider raw payloads (`provider_records.raw_data`). -- Satellite associations (credits, artwork, videos, genre/keyword/company - links) are replaced wholesale on each re-sync rather than diffed, so their - Aurora IDs are not stable across re-ingestion — see - `app/services/ingestion/movie_ingestion.py`. +- Satellite associations (credits, videos, genre/keyword/company links) are + replaced wholesale on each re-sync rather than diffed, so their Aurora IDs + are not stable across re-ingestion — see + `app/services/ingestion/movie_ingestion.py`. **Artwork is the exception:** + `app/repositories/polymorphic.py:sync_artwork` upserts by + `provider_asset_id` instead, specifically so a previously-downloaded + `local_path`/`checksum` (see "Artwork storage" below) survives + re-ingestion instead of being silently reset to `None` and re-downloaded + every sync. - Seasons don't get their own `external_ids` row (identified instead by the natural `(series_id, season_number)` key) — episodes do, since TMDb gives each episode a distinct global id. Revisit if a provider ever needs a diff --git a/tests/integration/test_artwork_download.py b/tests/integration/test_artwork_download.py new file mode 100644 index 0000000..6764ae7 --- /dev/null +++ b/tests/integration/test_artwork_download.py @@ -0,0 +1,116 @@ +"""Integration tests for the artwork download pipeline +(app/services/artwork/download.py). Uses httpx.MockTransport to simulate +TMDb's CDN so these run without network access, while still exercising a +real Artwork row against PostgreSQL.""" + +from __future__ import annotations + +import httpx +import pytest +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.artwork import Artwork +from app.models.enums import ArtworkType +from app.services.artwork.download import ArtworkDownloadError, download_artwork +from app.services.artwork.storage import LocalDiskStorage +from tests.integration.factories import make_provider + +FAKE_JPEG_BYTES = b"\xff\xd8\xff\xe0" + b"0" * 100 # not a *real* JPEG, just image-shaped bytes + + +def _client_for(handler) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(handler)) + + +async def _make_artwork(session: AsyncSession, *, local_path: str | None = None) -> Artwork: + provider = await make_provider(session) + artwork = Artwork( + entity_type="movie", + entity_id=provider.id, # any UUID; polymorphic tables have no real FK + artwork_type=ArtworkType.POSTER, + provider_id=provider.id, + original_url="https://image.tmdb.org/t/p/original/poster.jpg", + local_path=local_path, + ) + session.add(artwork) + await session.flush() + return artwork + + +@pytest.mark.asyncio +async def test_download_artwork_saves_file_and_sets_local_path_and_checksum( + db_session: AsyncSession, tmp_path +): + artwork = await _make_artwork(db_session) + storage = LocalDiskStorage(str(tmp_path), "/media") + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "image/jpeg"}, content=FAKE_JPEG_BYTES) + + async with _client_for(handler) as client: + result = await download_artwork(db_session, storage, artwork.id, client=client) + + assert result is True + await db_session.refresh(artwork) + assert artwork.local_path is not None + assert artwork.local_path.startswith("/media/poster/") + assert artwork.checksum is not None + assert await storage.exists(artwork.local_path.removeprefix("/media/")) + + +@pytest.mark.asyncio +async def test_download_artwork_skips_when_already_downloaded(db_session: AsyncSession, tmp_path): + artwork = await _make_artwork(db_session, local_path="/media/poster/ab/existing.jpg") + storage = LocalDiskStorage(str(tmp_path), "/media") + + def handler(request: httpx.Request) -> httpx.Response: + raise AssertionError("should not make an HTTP request when already downloaded") + + async with _client_for(handler) as client: + result = await download_artwork(db_session, storage, artwork.id, client=client) + + assert result is False + + +@pytest.mark.asyncio +async def test_download_artwork_force_redownloads(db_session: AsyncSession, tmp_path): + artwork = await _make_artwork(db_session, local_path="/media/poster/ab/existing.jpg") + storage = LocalDiskStorage(str(tmp_path), "/media") + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "image/png"}, content=FAKE_JPEG_BYTES) + + async with _client_for(handler) as client: + result = await download_artwork(db_session, storage, artwork.id, client=client, force=True) + + assert result is True + + +@pytest.mark.asyncio +async def test_download_artwork_rejects_non_image_content_type( + db_session: AsyncSession, tmp_path +): + artwork = await _make_artwork(db_session) + storage = LocalDiskStorage(str(tmp_path), "/media") + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "text/html"}, content=b"") + + async with _client_for(handler) as client: + with pytest.raises(ArtworkDownloadError, match="expected an image"): + await download_artwork(db_session, storage, artwork.id, client=client) + + +@pytest.mark.asyncio +async def test_download_artwork_returns_false_for_missing_row(db_session: AsyncSession, tmp_path): + import uuid + + storage = LocalDiskStorage(str(tmp_path), "/media") + + def handler(request: httpx.Request) -> httpx.Response: + raise AssertionError("should not make an HTTP request for a nonexistent artwork row") + + async with _client_for(handler) as client: + result = await download_artwork(db_session, storage, uuid.uuid4(), client=client) + + assert result is False diff --git a/tests/integration/test_artwork_sync.py b/tests/integration/test_artwork_sync.py new file mode 100644 index 0000000..e8e4163 --- /dev/null +++ b/tests/integration/test_artwork_sync.py @@ -0,0 +1,162 @@ +"""Integration tests for app/repositories/polymorphic.py:sync_artwork. + +The one guarantee this module exists for: re-ingesting a title must not +wipe out artwork that app/services/artwork/ already downloaded locally. +Before sync_artwork, every ingestion pipeline deleted and recreated all of +an entity's Artwork rows on each sync, which would silently reset +local_path/checksum to None every time -- defeating the entire point of +caching artwork locally. +""" + +from __future__ import annotations + +import uuid + +import pytest +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.artwork import Artwork +from app.repositories.polymorphic import sync_artwork +from app.services.metadata.normalizer import NormalizedArtwork +from tests.integration.factories import make_provider + + +def _artwork(asset_id: str, *, vote_average: float = 5.0, is_primary: bool = False) -> NormalizedArtwork: + return NormalizedArtwork( + artwork_type="poster", + url=f"https://image.tmdb.org/t/p/original/{asset_id}", + provider_asset_id=asset_id, + width=1000, + height=1500, + aspect_ratio=0.667, + language=None, + vote_average=vote_average, + is_primary=is_primary, + ) + + +@pytest.mark.asyncio +async def test_sync_artwork_creates_rows_on_first_sync(db_session: AsyncSession): + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg", is_primary=True), _artwork("b.jpg")], + ) + await db_session.commit() + + rows = ( + (await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id))) + .scalars() + .all() + ) + assert {r.provider_asset_id for r in rows} == {"a.jpg", "b.jpg"} + + +@pytest.mark.asyncio +async def test_sync_artwork_preserves_local_path_across_resync(db_session: AsyncSession): + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg", vote_average=5.0)], + ) + await db_session.flush() + + row = ( + await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id)) + ).scalar_one() + row.local_path = "/media/poster/ab/deadbeef.jpg" + row.checksum = "deadbeef" + await db_session.flush() + + # Re-sync as if the movie were re-ingested: same image, but TMDb now + # reports a different vote_average. + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg", vote_average=9.0)], + ) + await db_session.commit() + + refreshed = ( + await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id)) + ).scalar_one() + assert refreshed.id == row.id # same row, not delete+recreate + assert refreshed.local_path == "/media/poster/ab/deadbeef.jpg" + assert refreshed.checksum == "deadbeef" + assert refreshed.vote_average == 9.0 # non-download fields still update + + +@pytest.mark.asyncio +async def test_sync_artwork_removes_rows_no_longer_present(db_session: AsyncSession): + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg"), _artwork("b.jpg")], + ) + await db_session.flush() + + # "b.jpg" dropped from the provider's alternates on re-sync. + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg")], + ) + await db_session.commit() + + rows = ( + (await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id))) + .scalars() + .all() + ) + assert {r.provider_asset_id for r in rows} == {"a.jpg"} + + +@pytest.mark.asyncio +async def test_sync_artwork_adds_new_rows_on_resync(db_session: AsyncSession): + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg")], + ) + await db_session.flush() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg"), _artwork("c.jpg")], + ) + await db_session.commit() + + rows = ( + (await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id))) + .scalars() + .all() + ) + assert {r.provider_asset_id for r in rows} == {"a.jpg", "c.jpg"} diff --git a/tests/unit/test_artwork_download.py b/tests/unit/test_artwork_download.py new file mode 100644 index 0000000..6960174 --- /dev/null +++ b/tests/unit/test_artwork_download.py @@ -0,0 +1,58 @@ +"""Unit tests for the low-level HTTP fetch used by artwork downloads +(app/services/artwork/download.py:_fetch). No database needed — this is +pure request/response handling, tested against a mocked transport.""" + +from __future__ import annotations + +import httpx +import pytest + +from app.services.artwork.download import ArtworkDownloadError, _fetch + +FAKE_JPEG_BYTES = b"\xff\xd8\xff\xe0" + b"0" * 100 + + +def _client_for(handler) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(handler)) + + +@pytest.mark.asyncio +async def test_fetch_returns_content_and_content_type(): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "image/jpeg"}, content=FAKE_JPEG_BYTES) + + async with _client_for(handler) as client: + content, content_type = await _fetch(client, "https://example.test/poster.jpg", 1_000_000) + + assert content == FAKE_JPEG_BYTES + assert content_type == "image/jpeg" + + +@pytest.mark.asyncio +async def test_fetch_rejects_non_image_content_type(): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "text/html"}, content=b"") + + async with _client_for(handler) as client: + with pytest.raises(ArtworkDownloadError, match="expected an image"): + await _fetch(client, "https://example.test/oops.html", 1_000_000) + + +@pytest.mark.asyncio +async def test_fetch_rejects_response_over_max_bytes(): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, headers={"content-type": "image/jpeg"}, content=FAKE_JPEG_BYTES) + + async with _client_for(handler) as client: + with pytest.raises(ArtworkDownloadError, match="exceeds ARTWORK_DOWNLOAD_MAX_BYTES"): + await _fetch(client, "https://example.test/huge.jpg", max_bytes=10) + + +@pytest.mark.asyncio +async def test_fetch_wraps_http_errors(): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(404) + + async with _client_for(handler) as client: + with pytest.raises(ArtworkDownloadError, match="failed to download"): + await _fetch(client, "https://example.test/missing.jpg", 1_000_000) diff --git a/tests/unit/test_artwork_storage.py b/tests/unit/test_artwork_storage.py new file mode 100644 index 0000000..0b76371 --- /dev/null +++ b/tests/unit/test_artwork_storage.py @@ -0,0 +1,43 @@ +"""Unit tests for LocalDiskStorage (app/services/artwork/storage.py).""" + +from __future__ import annotations + +import pytest + +from app.services.artwork.storage import LocalDiskStorage + + +@pytest.fixture +def storage(tmp_path) -> LocalDiskStorage: + return LocalDiskStorage(str(tmp_path), "/media") + + +@pytest.mark.asyncio +async def test_save_writes_file_and_returns_public_url(storage: LocalDiskStorage): + url = await storage.save("poster/ab/abcdef.jpg", b"hello world") + assert url == "/media/poster/ab/abcdef.jpg" + assert await storage.exists("poster/ab/abcdef.jpg") + + +@pytest.mark.asyncio +async def test_save_creates_intermediate_directories(storage: LocalDiskStorage): + await storage.save("still/de/deadbeef.png", b"data") + assert await storage.exists("still/de/deadbeef.png") + + +@pytest.mark.asyncio +async def test_exists_is_false_for_missing_file(storage: LocalDiskStorage): + assert not await storage.exists("poster/00/never-written.jpg") + + +@pytest.mark.asyncio +async def test_save_rejects_path_traversal(storage: LocalDiskStorage): + with pytest.raises(ValueError, match="escapes MEDIA_ROOT"): + await storage.save("../../etc/passwd", b"malicious") + + +@pytest.mark.asyncio +async def test_url_prefix_trailing_slash_is_normalized(tmp_path): + storage = LocalDiskStorage(str(tmp_path), "/media/") + url = await storage.save("poster/ab/x.jpg", b"data") + assert url == "/media/poster/ab/x.jpg" diff --git a/tests/unit/test_normalizer.py b/tests/unit/test_normalizer.py index b095e11..d341a61 100644 --- a/tests/unit/test_normalizer.py +++ b/tests/unit/test_normalizer.py @@ -145,6 +145,21 @@ def test_normalize_movie_maps_artwork_and_videos(): assert movie.videos[0].is_official is True +def test_normalize_movie_marks_exactly_one_primary_poster_and_backdrop(): + # is_primary picks out TMDb's top-level poster_path/backdrop_path (the + # image auto-downloaded for local hosting) from the full alternates + # list -- not just "the first one". + movie = normalize_movie(FAKE_MOVIE) + primary_posters = [a for a in movie.artwork if a.artwork_type == "poster" and a.is_primary] + primary_backdrops = [ + a for a in movie.artwork if a.artwork_type == "backdrop" and a.is_primary + ] + assert len(primary_posters) == 1 + assert primary_posters[0].provider_asset_id == "/poster.jpg" + assert len(primary_backdrops) == 1 + assert primary_backdrops[0].provider_asset_id == "/backdrop.jpg" + + def test_normalize_movie_external_ids_never_include_tmdb_itself(): # TMDb's own id is handled separately by the ingestion pipeline via # `normalized.external_id`, not folded into `external_ids` (section 3: @@ -187,6 +202,9 @@ def test_normalize_person_profile_artwork_deduplicated(): # not be inserted twice. paths = [a.provider_asset_id for a in person.artwork] assert paths.count("/keanu.jpg") == 1 + # ...and since it wasn't inserted as a second row, the existing one must + # be the one marked primary instead -- not silently left unmarked. + assert next(a for a in person.artwork if a.provider_asset_id == "/keanu.jpg").is_primary FAKE_SERIES = {