From 7c2569522f8a06ec3bc751835b4c72c16b3b4da3 Mon Sep 17 00:00:00 2001 From: JakeBreath Date: Wed, 23 Sep 2026 21:35:09 -0500 Subject: [PATCH] Make thumbnail generation atomic and warm it on import - write thumbnails to a .part file and os.replace() them, so concurrent requests never read a half-written JPEG - a stale thumbnail plus a vanished source no longer raises through the request (getmtime on a missing file returned 500); it falls back cleanly - ensure_thumbnail(item) warms the preview when a file is indexed, keeping image decoding out of the request path --- backend/apps/library/services.py | 71 ++++++++++++++++--- .../apps/library/tests/test_media_cache.py | 21 ++++++ backend/apps/library/uploads.py | 6 ++ 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/backend/apps/library/services.py b/backend/apps/library/services.py index bdd0c64..ef60dba 100644 --- a/backend/apps/library/services.py +++ b/backend/apps/library/services.py @@ -6,6 +6,7 @@ import os import re import shutil import subprocess +import uuid from datetime import datetime, timezone from pathlib import Path from urllib.parse import urlencode @@ -481,15 +482,38 @@ def sanitize_iqdb_results(results): return cleaned +def _thumbnail_is_fresh(target, path): + """True when the cached thumbnail exists and is at least as new as source.""" + try: + stat = target.stat() + if stat.st_size <= 0: + return False + return stat.st_mtime >= os.path.getmtime(path) + except OSError: + return False + + +def _thumbs_dir(): + thumbs_dir = Path(settings.MEDIA_ROOT) / "thumbs" + thumbs_dir.mkdir(parents=True, exist_ok=True) + return thumbs_dir + + def generate_video_thumbnail(md5, path): """Extract a JPEG thumbnail from a video, cached under MEDIA_ROOT/thumbs.""" if not shutil.which("ffmpeg"): return None - thumbs_dir = Path(settings.MEDIA_ROOT) / "thumbs" - thumbs_dir.mkdir(parents=True, exist_ok=True) + try: + thumbs_dir = _thumbs_dir() + except OSError: + logger.exception("Could not create the thumbnail folder") + return None target = thumbs_dir / f"{md5}.jpg" - if target.exists() and target.stat().st_mtime >= os.path.getmtime(path): + if _thumbnail_is_fresh(target, path): return target + # Write beside the target and move it into place, so a concurrent request + # can never read a half-written JPEG. + temp = thumbs_dir / f".{md5}.{uuid.uuid4().hex}.part.jpg" command = [ "ffmpeg", "-y", @@ -503,11 +527,14 @@ def generate_video_thumbnail(md5, path): "scale=480:-2", "-loglevel", "error", - str(target), + str(temp), ] try: subprocess.run(command, check=True, capture_output=True, timeout=60) + os.replace(temp, target) except (subprocess.SubprocessError, OSError): + logger.exception("Could not build a video thumbnail for %s", path) + temp.unlink(missing_ok=True) return None return target if target.exists() else None @@ -517,14 +544,18 @@ def generate_image_thumbnail(md5, path): The thumbnail action used to serve full-size originals for images; a cached 480px JPEG keeps the library grid light without touching the - original file. Returns ``None`` when Pillow cannot decode the format, so - callers can fall back to the original. + original file. Returns ``None`` when the source is missing or Pillow + cannot decode it, so callers can fall back to the original. """ - thumbs_dir = Path(settings.MEDIA_ROOT) / "thumbs" - thumbs_dir.mkdir(parents=True, exist_ok=True) + try: + thumbs_dir = _thumbs_dir() + except OSError: + logger.exception("Could not create the thumbnail folder") + return None target = thumbs_dir / f"{md5}.jpg" - if target.exists() and target.stat().st_mtime >= os.path.getmtime(path): + if _thumbnail_is_fresh(target, path): return target + temp = thumbs_dir / f".{md5}.{uuid.uuid4().hex}.part.jpg" try: with Image.open(path) as image: # Animated formats: the first frame is the preview. @@ -532,9 +563,27 @@ def generate_image_thumbnail(md5, path): frame = ImageOps.exif_transpose(image) or image frame = frame.convert("RGB") frame.thumbnail((480, 480)) - frame.save(target, "JPEG", quality=82, optimize=True) + frame.save(temp, "JPEG", quality=82, optimize=True) + os.replace(temp, target) except Exception: # noqa: BLE001 - previews must never break serving logger.exception("Could not build an image thumbnail for %s", path) - target.unlink(missing_ok=True) + temp.unlink(missing_ok=True) return None return target if target.exists() else None + + +def ensure_thumbnail(item): + """Generate an item's cached thumbnail if it is missing or stale. + + Warming thumbnails when a file is indexed keeps image decoding out of the + request path, where the upload pipeline's hashing used to starve it. + """ + location = item.locations.first() + if location is None: + return None + path = Path(location.path) + if not path.is_file(): + return None + if path.suffix.lower() in VIDEO_EXTENSIONS: + return generate_video_thumbnail(item.md5, path) + return generate_image_thumbnail(item.md5, path) diff --git a/backend/apps/library/tests/test_media_cache.py b/backend/apps/library/tests/test_media_cache.py index 03fd816..78fc99d 100644 --- a/backend/apps/library/tests/test_media_cache.py +++ b/backend/apps/library/tests/test_media_cache.py @@ -120,6 +120,27 @@ class MediaCacheTests(TestCase): self.client.get(url, self.signed("thumbnail")) self.assertEqual(thumb.stat().st_mtime_ns, before) + def test_ensure_thumbnail_reuses_the_cache(self): + first = services.ensure_thumbnail(self.item) + self.assertIsNotNone(first) + self.assertTrue(first.exists()) + mtime = first.stat().st_mtime_ns + second = services.ensure_thumbnail(self.item) + self.assertEqual(second, first) + self.assertEqual(second.stat().st_mtime_ns, mtime) + + def test_thumbnail_of_a_missing_source_does_not_error(self): + """Regression: getmtime() on a vanished source used to raise a 500.""" + thumbs = self._media / "thumbs" + thumbs.mkdir(parents=True, exist_ok=True) + (thumbs / f"{self.item.md5}.jpg").write_bytes(b"stale") + Path(self.item.locations.first().path).unlink() + self.assertIsNone(services.ensure_thumbnail(self.item)) + response = self.client.get( + f"/api/files/J-{self.item.id}/thumbnail/", self.signed("thumbnail") + ) + self.assertEqual(response.status_code, 404) + def test_staged_files_cache_briefly(self): temp = TempUpload.objects.create( user=self.user, diff --git a/backend/apps/library/uploads.py b/backend/apps/library/uploads.py index a27d47e..f15c037 100644 --- a/backend/apps/library/uploads.py +++ b/backend/apps/library/uploads.py @@ -128,6 +128,12 @@ def complete_temp_upload(temp, download_url=None): services.ensure_visual_hashes(item) temp.file.delete(save=False) + # Warm the preview while the import is still off the request path. + try: + services.ensure_thumbnail(item) + except Exception: # noqa: BLE001 - a preview must not fail the import + logger.exception("Could not warm the thumbnail for J-%s", item.id) + temp.library_item = item temp.status = TempUpload.STATUS_COMPLETED