diff --git a/python-backend/musicmouse/library/podcast_feeds.py b/python-backend/musicmouse/library/podcast_feeds.py index a2b3ec7..18b0db6 100644 --- a/python-backend/musicmouse/library/podcast_feeds.py +++ b/python-backend/musicmouse/library/podcast_feeds.py @@ -116,6 +116,9 @@ _FFMPEG_TIMEOUT: Final = 600.0 #: Where a show folder remembers episodes that failed to download, so a permanently #: dead enclosure (e.g. pulled from the host's CDN) isn't retried every poll. _FAILED_DOWNLOADS_FILENAME: Final = ".failed-downloads.json" +#: The same, for episodes on disk whose cover could not be found or fetched - so an +#: episode with no art anywhere doesn't cost a page fetch on every poll. +_MISSING_COVERS_FILENAME: Final = ".missing-covers.json" #: How long a failed episode is left alone before it's given another chance - long #: enough to stop hammering a dead URL every 6 hours, short enough that a genuinely #: transient failure (a host outage, a flaky network) still recovers on its own. @@ -379,15 +382,16 @@ def prune_show(folder: Path, keep: int | None, extensions: frozenset[str] | None return removed -def _load_failed_downloads(folder: Path) -> dict[str, datetime]: - """Filename -> when it last failed to download, for episodes ``sync_show`` should - leave alone until :data:`_RETRY_BACKOFF` has passed. +def _load_failed(folder: Path, record_name: str) -> dict[str, datetime]: + """Filename -> when it last failed, for episodes ``sync_show`` should leave alone + until :data:`_RETRY_BACKOFF` has passed. ``record_name`` picks which record: failed + downloads, or covers that could not be found. A missing or corrupt record is just an empty one - nothing here is precious enough to raise over, matching the module's overall rule that bad input is logged and skipped rather than fatal. """ - path = folder / _FAILED_DOWNLOADS_FILENAME + path = folder / record_name try: raw = json.loads(path.read_text(encoding="utf-8")) except FileNotFoundError: @@ -402,12 +406,12 @@ def _load_failed_downloads(folder: Path) -> dict[str, datetime]: return {} -def _save_failed_downloads(folder: Path, failed: dict[str, datetime]) -> None: - path = folder / _FAILED_DOWNLOADS_FILENAME +def _save_failed(folder: Path, record_name: str, failed: dict[str, datetime]) -> None: + path = folder / record_name if not failed: path.unlink(missing_ok=True) return - temp_path = folder / f".{_FAILED_DOWNLOADS_FILENAME}.tmp" + temp_path = folder / f".{record_name}.tmp" payload = {filename: when.isoformat() for filename, when in failed.items()} temp_path.write_text(json.dumps(payload), encoding="utf-8") temp_path.replace(path) @@ -529,6 +533,65 @@ async def resolve_episode_cover(client: httpx2.AsyncClient, episode: Episode) -> return _extract_og_image(response.text) +def _has_sidecar_cover(audio_path: Path) -> bool: + return any(audio_path.with_suffix(ext).is_file() for ext in _IMAGE_EXTENSIONS) + + +async def _sync_covers( + client: httpx2.AsyncClient, folder: Path, episodes: list[Episode], now: datetime +) -> bool: + """Fetch the sidecar cover of every episode in ``episodes`` that is on disk without + one. Returns whether any cover was added. + + Covers every episode on disk, not just the ones this pass downloaded: episodes + placed by hand, or downloaded before a cover route existed (Wissen macht Ah's + ``og:image`` fallback), would otherwise never get theirs. An episode whose cover + can't be found or fetched is remembered and left alone for :data:`_RETRY_BACKOFF`, + like a failed download, so a feed with no per-episode art doesn't cost one page + fetch per episode on every poll. + """ + missing = _load_failed(folder, _MISSING_COVERS_FILENAME) + on_disk: set[str] = set() + added = False + for episode in episodes: + filename = episode_filename( + episode.published, episode.title, episode.enclosure_type, episode.enclosure_url + ) + audio_path = folder / filename + if not audio_path.is_file(): + continue + on_disk.add(filename) + if _has_sidecar_cover(audio_path): + missing.pop(filename, None) + continue + last_miss = missing.get(filename) + if last_miss is not None and now - last_miss < _RETRY_BACKOFF: + continue + + cover_url = await resolve_episode_cover(client, episode) + if cover_url is None: + missing[filename] = now + continue + cover_filename = episode_cover_filename(episode.published, episode.title, cover_url) + try: + await download_episode_cover(client, folder, cover_url, cover_filename) + except (httpx2.HTTPError, OSError) as exc: + _log.warning( + "Could not download cover for episode %r in %s: %s", + episode.title, + folder.name, + exc, + ) + missing[filename] = now + continue + missing.pop(filename, None) + added = True + + missing = {filename: when for filename, when in missing.items() if filename in on_disk} + _save_failed(folder, _MISSING_COVERS_FILENAME, missing) + return added + + async def sync_show( client: httpx2.AsyncClient, folder: Path, @@ -542,9 +605,9 @@ async def sync_show( Both halves use the same ``keep``, which is what makes the folder settle - see :data:`DEFAULT_EPISODE_LIMIT`. - Returns whether anything changed, deletions included: a pruned folder needs a - rescan just as much as a downloaded episode does, or the library keeps offering - tracks whose files are gone. + Returns whether anything changed, deletions and backfilled covers included: a + pruned folder needs a rescan just as much as a downloaded episode does, or the + library keeps offering tracks whose files are gone. Errors - an unreachable feed, a malformed one, a single broken enclosure - are logged and swallowed here so one bad show never stops the others or takes down the @@ -559,8 +622,9 @@ async def sync_show( _log.warning("Could not fetch podcast feed %s for %s: %s", feed_url, folder.name, exc) return False - pending = missing_episodes(folder, newest_episodes(parse_feed(response.content), keep)) - failed = _load_failed_downloads(folder) + window = newest_episodes(parse_feed(response.content), keep) + pending = missing_episodes(folder, window) + failed = _load_failed(folder, _FAILED_DOWNLOADS_FILENAME) now = datetime.now(UTC) changed = False for episode, filename in pending: @@ -579,23 +643,12 @@ async def sync_show( _log.info("Downloaded new episode %r for %s", episode.title, folder.name) changed = True - cover_url = await resolve_episode_cover(client, episode) - if cover_url: - cover_filename = episode_cover_filename(episode.published, episode.title, cover_url) - if not (folder / cover_filename).exists(): - try: - await download_episode_cover(client, folder, cover_url, cover_filename) - except (httpx2.HTTPError, OSError) as exc: - _log.warning( - "Could not download cover for episode %r in %s: %s", - episode.title, - folder.name, - exc, - ) - pending_filenames = {filename for _episode, filename in pending} failed = {filename: when for filename, when in failed.items() if filename in pending_filenames} - _save_failed_downloads(folder, failed) + _save_failed(folder, _FAILED_DOWNLOADS_FILENAME, failed) + + if await _sync_covers(client, folder, window, now): + changed = True # After downloading, not before: an episode that just arrived is one of the newest # and must be counted when deciding what falls off the end. diff --git a/python-backend/musicmouse/library/scanner.py b/python-backend/musicmouse/library/scanner.py index 7da4050..992f3b3 100644 --- a/python-backend/musicmouse/library/scanner.py +++ b/python-backend/musicmouse/library/scanner.py @@ -262,7 +262,11 @@ def scan_episodes( out: dict[str, tuple[Album, Fingerprint]] = {} for path in paths: identifier = album_id(root, path) - fingerprint = Fingerprint.of([path]) + # The sidecar cover counts too: one fetched after the episode was first scanned + # (``podcast_feeds.py`` backfills them) must invalidate the cached entry, or the + # episode keeps showing the show's shared cover. + sidecars = [path.with_suffix(ext) for ext in _SIDECAR_COVER_EXTENSIONS] + fingerprint = Fingerprint.of([path, *(p for p in sidecars if p.is_file())]) cached = known.get(identifier) if cached is not None and cached[1] == fingerprint: out[identifier] = cached diff --git a/python-backend/tests/test_library.py b/python-backend/tests/test_library.py index 5367678..da264bb 100644 --- a/python-backend/tests/test_library.py +++ b/python-backend/tests/test_library.py @@ -284,6 +284,25 @@ async def test_an_episodes_sidecar_cover_wins_over_the_shows_shared_one(config_d assert red < 60 and blue > red, "falls back to the show's shared cover" +async def test_a_sidecar_cover_added_later_replaces_the_cached_one(config_dir: Path) -> None: + """`podcast_feeds.py` backfills covers for episodes already on disk - the episode's + audio is untouched, so only the sidecar can tell the cache its entry is stale.""" + from PIL import Image + + podcast = config_dir / "music" / "Kinderpodcasts" / "Wissen macht Ah" + Image.new("RGB", (32, 32), (10, 20, 30)).save(podcast / "cover.jpg") + first = album_named(await build(config_dir), "Alt") + + alt_audio = first.tracks[0].path + Image.new("RGB", (32, 32), (200, 40, 30)).save(alt_audio.with_suffix(".jpg")) + second = album_named(await build(config_dir), "Alt") + + assert second.cover is not None + with Image.open(second.cover) as image: + red, green, _blue = image.convert("RGB").getpixel((0, 0)) # type: ignore[misc] + assert red > 150 and green < 90, "the new sidecar should replace the shared cover" + + # --------------------------------------------------------------------------- cache diff --git a/python-backend/tests/test_podcast_feeds.py b/python-backend/tests/test_podcast_feeds.py index 193d889..155cee9 100644 --- a/python-backend/tests/test_podcast_feeds.py +++ b/python-backend/tests/test_podcast_feeds.py @@ -411,6 +411,83 @@ async def test_sync_show_survives_a_failed_cover_download(tmp_path: Path) -> Non assert not (folder / "20250101 - Unique Cover.jpg").exists() +_FEED_WITH_LINKS = b""" + + +Link Show +http://example.com/show + + Old One + http://example.com/episodes/old + Wed, 01 Jan 2025 08:00:00 GMT + + ep1 + + + +""" + + +@pytest.mark.asyncio +async def test_sync_show_backfills_the_cover_of_an_episode_already_on_disk( + tmp_path: Path, +) -> None: + # Placed by hand (or downloaded before covers were fetched): the audio is there, + # so nothing is downloaded - but its cover still has to be fetched. + folder = tmp_path / "show" + folder.mkdir() + (folder / "20250101 - Old One.mp3").write_bytes(b"audio-bytes") + requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + requests.append(url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED_WITH_LINKS) + if url == "http://example.com/episodes/old": + html = '' + return httpx2.Response(200, text=html) + if url == "http://example.com/old.png": + return httpx2.Response(200, content=b"cover-bytes") + return httpx2.Response(500) + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + changed = await sync_show(client, folder, "http://example.com/feed.xml") + requests.clear() + changed_again = await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is True + assert (folder / "20250101 - Old One.png").read_bytes() == b"cover-bytes" + # Once the sidecar exists, a later poll doesn't go looking for it again. + assert changed_again is False + assert requests == ["http://example.com/feed.xml"] + + +@pytest.mark.asyncio +async def test_sync_show_does_not_look_for_a_missing_cover_again_within_the_backoff( + tmp_path: Path, +) -> None: + folder = tmp_path / "show" + folder.mkdir() + (folder / "20250101 - Old One.mp3").write_bytes(b"audio-bytes") + page_requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED_WITH_LINKS) + page_requests.append(url) + return httpx2.Response(200, text="no og:image here") + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + changed = await sync_show(client, folder, "http://example.com/feed.xml") + await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is False + assert page_requests == ["http://example.com/episodes/old"] + assert (folder / podcast_feeds._MISSING_COVERS_FILENAME).exists() + + # ---------------------------------------------------------------------- video shows