refactor: use item guid as thumbnail name for easier cleanup

app/features/streaming/library/thumbnail.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import asyncio
-import hashlib
 import logging
 import os
 import subprocess
@@ -12,7 +11,7 @@ from app.library.cache import Cache
 from app.library.config import Config
 from app.library.Utils import FILES_TYPE, get_file_sidecar
 
-LOG: logging.Logger = logging.getLogger("streaming.thumbnail")
+LOG: logging.Logger = logging.getLogger("player.thumbnail")
 
 IMAGE_TYPES: tuple[str, ...] = (".jpg", ".jpeg", ".png", ".webp")
 FOLDER_IMAGE_ORDER: tuple[str, ...] = ("thumbnail", "poster", "artwork", "cover", "fanart")
@@ -77,12 +76,6 @@ def pick_local_thumb(media_file: Path) -> Path | None:
     return None
 
 
-def _cache_key(media_file: Path) -> str:
-    stat = media_file.stat()
-    payload: str = f"{media_file.resolve()}:{stat.st_size}:{stat.st_mtime_ns}"
-    return hashlib.sha256(payload.encode("utf-8")).hexdigest()
-
-
 def _seek_seconds(ff_info) -> float | None:
     duration: float = 0.0
     try:
@@ -198,20 +191,21 @@ async def _run_ffmpeg(media_file: Path, output_file: Path) -> Path | None:
     raise OSError(last_error)
 
 
-async def ensure_thumb(media_file: Path, cache_root: Path) -> Path | None:
+async def ensure_thumb(media_file: Path, cache_root: Path, item_id: str | None = None) -> Path | None:
     config: Config = Config.get_instance()
     cache: Cache = Cache.get_instance()
     if bool(config.thumb_generate) is not True:
         return None
 
-    key: str = _cache_key(media_file)
+    sidecar: bool = bool(config.thumb_sidecar)
-    cache_file: Path = (
+    if not sidecar and not item_id:
-        media_file.with_name(f"{media_file.name}.thumb.jpg")
+        msg = "item_id is required when thumbnail sidecar is disabled."
-        if bool(config.thumb_sidecar)
+        raise ValueError(msg)
-        else cache_root / f"{key}.jpg"
-    )
 
-    miss_key: str = f"thumbnail-miss:{key}"
+    thumb_id: str = str(media_file.resolve()) if sidecar else str(item_id)
+    cache_file: Path = media_file.with_name(f"{media_file.name}.jpg") if sidecar else cache_root / f"{item_id}.jpg"
+
+    miss_key: str = f"thumbnail-miss:{thumb_id}"
 
     if cache_file.exists() and cache_file.stat().st_size > 0:
         cache.delete(miss_key)
@@ -228,12 +222,12 @@ async def ensure_thumb(media_file: Path, cache_root: Path) -> Path | None:
         if cache.has(miss_key):
             return None
 
-        task = _IN_PROCESS.get(key)
+        task = _IN_PROCESS.get(thumb_id)
         if task is not None and not task.done():
             LOG.debug(f"Waiting for thumbnail generation for '{media_file}'.")
         else:
-            task = asyncio.create_task(_run_ffmpeg(media_file, cache_file), name=f"thumb-{key[:8]}")
+            task = asyncio.create_task(_run_ffmpeg(media_file, cache_file), name=f"thumb-{item_id or media_file.stem}")
-            _IN_PROCESS[key] = task
+            _IN_PROCESS[thumb_id] = task
             LOG.debug(f"Starting thumbnail generation task for '{media_file}' -> '{cache_file}'.")
 
     try:
@@ -248,6 +242,6 @@ async def ensure_thumb(media_file: Path, cache_root: Path) -> Path | None:
         raise
     finally:
         async with _LOCK:
-            current = _IN_PROCESS.get(key)
+            current = _IN_PROCESS.get(thumb_id)
             if current is task and task.done():
-                _IN_PROCESS.pop(key, None)
+                _IN_PROCESS.pop(thumb_id, None)

app/features/streaming/tests/test_thumbnail.py

@@ -30,11 +30,13 @@ async def test_singleflight(monkeypatch: pytest.MonkeyPatch) -> None:
     from app.features.streaming.library import thumbnail
 
     thumbnail._IN_PROCESS.clear()
+    thumbnail.Cache.get_instance().clear()
 
     with temporary_test_dir("thumb-singleflight") as temp_dir:
         media = temp_dir / "video.mp4"
         cache_root = temp_dir / "cache"
         media.write_text("video")
+        item_id = "item-1"
 
         calls = {"count": 0}
 
@@ -48,8 +50,8 @@ async def test_singleflight(monkeypatch: pytest.MonkeyPatch) -> None:
         monkeypatch.setattr(thumbnail, "_run_ffmpeg", fake_run_ffmpeg)
 
         first, second = await asyncio.gather(
-            thumbnail.ensure_thumb(media, cache_root),
+            thumbnail.ensure_thumb(media, cache_root, item_id=item_id),
-            thumbnail.ensure_thumb(media, cache_root),
+            thumbnail.ensure_thumb(media, cache_root, item_id=item_id),
         )
 
         assert first == second
@@ -63,13 +65,15 @@ async def test_cache_hit(monkeypatch: pytest.MonkeyPatch) -> None:
     from app.features.streaming.library import thumbnail
 
     thumbnail._IN_PROCESS.clear()
+    thumbnail.Cache.get_instance().clear()
 
     with temporary_test_dir("thumb-cache") as temp_dir:
         media = temp_dir / "video.mp4"
         cache_root = temp_dir / "cache"
         media.write_text("video")
+        item_id = "item-1"
 
-        cache_file = cache_root / f"{thumbnail._cache_key(media)}.jpg"
+        cache_file = cache_root / f"{item_id}.jpg"
         cache_file.parent.mkdir(parents=True, exist_ok=True)
         cache_file.write_text("image")
 
@@ -78,7 +82,7 @@ async def test_cache_hit(monkeypatch: pytest.MonkeyPatch) -> None:
 
         monkeypatch.setattr(thumbnail, "_run_ffmpeg", fake_run_ffmpeg)
 
-        result = await thumbnail.ensure_thumb(media, cache_root)
+        result = await thumbnail.ensure_thumb(media, cache_root, item_id=item_id)
 
         assert result == cache_file
 
@@ -88,6 +92,7 @@ async def test_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
     from app.features.streaming.library import thumbnail
 
     thumbnail._IN_PROCESS.clear()
+    thumbnail.Cache.get_instance().clear()
 
     with temporary_test_dir("thumb-disabled") as temp_dir:
         media = temp_dir / "video.mp4"
@@ -99,7 +104,7 @@ async def test_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
             staticmethod(lambda: type("Cfg", (), {"thumb_generate": False, "thumb_sidecar": False})()),
         )
 
-        result = await thumbnail.ensure_thumb(media, temp_dir / "cache")
+        result = await thumbnail.ensure_thumb(media, temp_dir / "cache", item_id="item-1")
 
         assert result is None
 
@@ -111,12 +116,13 @@ async def test_sidecar_mode(monkeypatch: pytest.MonkeyPatch) -> None:
     thumbnail._IN_PROCESS.clear()
     thumbnail._SEM = None
     thumbnail._SEM_LIMIT = None
+    thumbnail.Cache.get_instance().clear()
 
     with temporary_test_dir("thumb-sidecar") as temp_dir:
         media = temp_dir / "video.mp4"
         media.write_text("video")
 
-        sidecar = media.with_name(f"{media.name}.thumb.jpg")
+        sidecar = media.with_name(f"{media.name}.jpg")
 
         monkeypatch.setattr(
             thumbnail.Config,
@@ -152,6 +158,7 @@ async def test_miss_cache(monkeypatch: pytest.MonkeyPatch) -> None:
     with temporary_test_dir("thumb-miss") as temp_dir:
         media = temp_dir / "video.mp4"
         media.write_text("video")
+        item_id = "item-1"
 
         monkeypatch.setattr(
             thumbnail.Config,
@@ -173,8 +180,8 @@ async def test_miss_cache(monkeypatch: pytest.MonkeyPatch) -> None:
 
         monkeypatch.setattr(thumbnail, "_run_ffmpeg", fake_run_ffmpeg)
 
-        first = await thumbnail.ensure_thumb(media, temp_dir / "cache")
+        first = await thumbnail.ensure_thumb(media, temp_dir / "cache", item_id=item_id)
-        second = await thumbnail.ensure_thumb(media, temp_dir / "cache")
+        second = await thumbnail.ensure_thumb(media, temp_dir / "cache", item_id=item_id)
 
         assert first is None
         assert second is None
@@ -219,6 +226,30 @@ def test_seek_bounds() -> None:
     assert _seek_seconds(ff_info) == 3.0
 
 
+@pytest.mark.asyncio
+async def test_cache_name_uses_item_id(monkeypatch: pytest.MonkeyPatch) -> None:
+    from app.features.streaming.library import thumbnail
+
+    thumbnail._IN_PROCESS.clear()
+    thumbnail.Cache.get_instance().clear()
+
+    with temporary_test_dir("thumb-id-name") as temp_dir:
+        media = temp_dir / "video.mp4"
+        cache_root = temp_dir / "cache"
+        media.write_text("video")
+
+        async def fake_run_ffmpeg(_file: Path, output_file: Path) -> Path:
+            output_file.parent.mkdir(parents=True, exist_ok=True)
+            output_file.write_text("image")
+            return output_file
+
+        monkeypatch.setattr(thumbnail, "_run_ffmpeg", fake_run_ffmpeg)
+
+        result = await thumbnail.ensure_thumb(media, cache_root, item_id="item-1")
+
+        assert result == cache_root / "item-1.jpg"
+
+
 @pytest.mark.asyncio
 async def test_retry_no_seek(monkeypatch: pytest.MonkeyPatch) -> None:
     from app.features.streaming.library.ffprobe import FFProbeResult
@@ -226,7 +257,7 @@ async def test_retry_no_seek(monkeypatch: pytest.MonkeyPatch) -> None:
 
     with temporary_test_dir("thumb-retry") as temp_dir:
         media = temp_dir / "video.mp4"
-        output = temp_dir / "thumb.jpg"
+        output = temp_dir / ".jpg"
         media.write_text("video")
 
         ff_info = FFProbeResult()

app/library/downloads/monitors.py

@@ -2,6 +2,7 @@
 
 import logging
 from datetime import UTC, datetime, timedelta
+from pathlib import Path
 from typing import TYPE_CHECKING
 
 from app.library.ag_utils import ag
@@ -171,3 +172,36 @@ async def delete_old_history(queue: "DownloadQueue") -> None:
 
     if titles:
         LOG.info(f"Automatically cleared '{', '.join(titles)}' from download history due to age.")
+
+
+async def cleanup_thumbnails(queue: "DownloadQueue") -> None:
+    """
+    Remove cached generated thumbnails whose history item no longer exists.
+
+    Args:
+        queue: DownloadQueue instance.
+
+    """
+    if queue.config.thumb_sidecar:
+        return
+
+    cache_root = Path(queue.config.temp_path) / "thumbnails"
+    if not cache_root.exists() or not cache_root.is_dir():
+        return
+
+    removed = 0
+    for thumb in cache_root.glob("*.jpg"):
+        if not thumb.is_file():
+            continue
+
+        if await queue.done.get_by_id(thumb.stem):
+            continue
+
+        try:
+            thumb.unlink(missing_ok=True)
+            removed += 1
+        except OSError as exc:
+            LOG.warning(f"Failed to remove orphaned thumbnail '{thumb}'. {exc!s}")
+
+    if removed > 0:
+        LOG.info(f"Removed '{removed}' orphaned cached thumbnails.")

app/library/downloads/queue_manager.py

@@ -18,7 +18,7 @@ from app.library.Utils import calc_download_path
 
 from .core import Download
 from .item_adder import add as add_impl
-from .monitors import check_for_stale, check_live, delete_old_history
+from .monitors import check_for_stale, check_live, cleanup_thumbnails, delete_old_history
 from .pool_manager import PoolManager
 
 if TYPE_CHECKING:
@@ -67,6 +67,12 @@ class DownloadQueue(metaclass=Singleton):
             id=check_live.__name__,
         )
 
+        Scheduler.get_instance().add(
+            timer="10 */1 * * *",
+            func=functools.partial(cleanup_thumbnails, self),
+            id=cleanup_thumbnails.__name__,
+        )
+
         if self.config.auto_clear_history_days > 0:
             Scheduler.get_instance().add(
                 timer="8 */1 * * *",

app/routes/api/history.py

@@ -312,8 +312,8 @@ async def item_thumbnail(request: Request, queue: DownloadQueue, config: Config)
     if not (id := request.match_info.get("id")):
         return web.json_response(data={"error": "id is required."}, status=web.HTTPBadRequest.status_code)
 
-    miss_key = f"history-thumb-missing:{id}"
+    miss_key: str = f"thumb_missing:{id}"
-    cache = Cache.get_instance()
+    cache: Cache = Cache.get_instance()
 
     item: Download | None = await queue.done.get_by_id(id)
     if not item or not item.info:
@@ -322,19 +322,21 @@ async def item_thumbnail(request: Request, queue: DownloadQueue, config: Config)
     if cache.has(miss_key):
         return web.json_response(data={"error": "thumbnail not found."}, status=web.HTTPNotFound.status_code)
 
-    filepath = item.info.get_file(download_path=Path(config.download_path))
+    filepath: Path | None = item.info.get_file(download_path=Path(config.download_path))
     if not filepath or not filepath.exists() or not filepath.is_file():
         cache.set(miss_key, value=True, ttl=30.0)
         return web.json_response(data={"error": "thumbnail not found."}, status=web.HTTPNotFound.status_code)
 
     cache.delete(miss_key)
 
-    local_thumb = pick_local_thumb(filepath)
+    local_thumb: Path | None = pick_local_thumb(filepath)
     if local_thumb and local_thumb.exists() and local_thumb.is_file():
         return web.FileResponse(path=str(local_thumb))
 
     try:
-        generated = await ensure_thumb(filepath, Path(config.temp_path) / "thumbnails")
+        generated: Path | None = await ensure_thumb(
+            filepath, Path(config.temp_path) / "thumbnails", item_id=item.info._id
+        )
     except OSError as e:
         LOG.warning(f"Failed to generate thumbnail for '{filepath}'. {e!s}")
         generated = None

app/tests/test_download.py

@@ -3,6 +3,7 @@ import os
 import signal
 from multiprocessing.reduction import ForkingPickler
 from pathlib import Path
+from types import SimpleNamespace
 from typing import Any, cast
 from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
@@ -1204,6 +1205,28 @@ class TestStatusTracker:
 
 
 class TestQueueManager:
+    @pytest.mark.asyncio
+    async def test_cleanup_thumbnails(self, tmp_path: Path) -> None:
+        from app.library.downloads.monitors import cleanup_thumbnails
+
+        queue_manager = object.__new__(DownloadQueue)
+        queue_manager.config = SimpleNamespace(temp_path=str(tmp_path), thumb_sidecar=False)
+        queue_manager.done = Mock()
+
+        cache_root = tmp_path / "thumbnails"
+        cache_root.mkdir(parents=True, exist_ok=True)
+        keep = cache_root / "keep-id.jpg"
+        drop = cache_root / "drop-id.jpg"
+        keep.write_text("keep")
+        drop.write_text("drop")
+
+        queue_manager.done.get_by_id = AsyncMock(side_effect=lambda item_id: item_id == "keep-id")
+
+        await cleanup_thumbnails(queue_manager)
+
+        assert keep.exists()
+        assert not drop.exists()
+
     @pytest.mark.asyncio
     async def test_cancel_running_live_item_defers_close(self) -> None:
         queue_manager = object.__new__(DownloadQueue)

app/tests/test_history_routes.py

@@ -220,11 +220,11 @@ async def test_item_thumbnail_generated(monkeypatch: pytest.MonkeyPatch) -> None
         media = temp_dir / "video.mp4"
         media.write_text("video")
         cache_dir = temp_dir / "tmp"
-        generated = cache_dir / "thumbnails" / "gen.jpg"
+        generated = cache_dir / "thumbnails" / "item-1.jpg"
 
         request = _FakeRequest()
-        request.match_info["id"] = "item-1"
         item = _make_download(filename="video.mp4", download_dir=str(temp_dir))
+        request.match_info["id"] = item.info._id
         queue = SimpleNamespace(done=SimpleNamespace(get_by_id=AsyncMock(return_value=item)))
         config = SimpleNamespace(download_path=str(temp_dir), temp_path=str(cache_dir))
 
@@ -232,8 +232,9 @@ async def test_item_thumbnail_generated(monkeypatch: pytest.MonkeyPatch) -> None
 
         called = {"count": 0}
 
-        async def fake_ensure_thumb(_file: Path, _cache_root: Path) -> Path:
+        async def fake_ensure_thumb(_file: Path, _cache_root: Path, item_id: str | None = None) -> Path:
             called["count"] += 1
+            assert item_id == request.match_info["id"]
             generated.parent.mkdir(parents=True, exist_ok=True)
             generated.write_text("generated")
             return generated