US07-03: Harden Media and Metadata Edge Cases (#85)

photo_pipeline/services/thumbnails.py

@@ -20,15 +20,16 @@ pa-imaging).
 
 from __future__ import annotations
 
+import os
 import uuid
 from pathlib import Path
 
-from PIL import Image, ImageOps, UnidentifiedImageError
+from PIL import Image, ImageOps
 from sqlalchemy import select
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import sessionmaker
 
-from photo_pipeline import path_policy
+from photo_pipeline import imaging, path_policy
 from photo_pipeline.config import Config
 from photo_pipeline.models import Asset, Thumbnail
 from photo_pipeline.services import availability
@@ -42,6 +43,9 @@ except Exception:  # pragma: no cover
     pass
 
 SIZES = (256, 512, 1280)
+# Every in-flight render writes one of these beside its destination; startup
+# cleanup recognises exactly this pattern and nothing else.
+TEMP_SUFFIX = ".tmp"
 THUMB_VERSION = 1
 THUMB_FORMAT = "webp"
 # The size kept as durable comparison evidence for archived assets (concept §9).
@@ -222,28 +226,33 @@ class ThumbnailService:
         return self._cache_dir / safe[:2] / f"{safe}.{THUMB_FORMAT}"
 
     def _render(self, source: str, size: int, cache_key: str) -> dict:
+        """Decode bounded, orient, colour-manage, resize, and write atomically.
+
+        The temporary file is removed on every failure path: a decoder that dies
+        halfway through ``save`` would otherwise leave a stray ``.tmp`` in the cache
+        forever (US07-03).
+        """
+        destination = self._cache_path(cache_key)
+        destination.parent.mkdir(parents=True, exist_ok=True)
+        tmp = destination.with_name(f".{destination.name}.{uuid.uuid4().hex}{TEMP_SUFFIX}")
         try:
-            with Image.open(source) as image:
-                width, height = image.size
-                if width * height > self._config.thumbnail_max_pixels:
-                    raise ImageTooLarge(
-                        f"{width}x{height} exceeds {self._config.thumbnail_max_pixels} px"
-                    )
+            with imaging.open_image(source, max_pixels=self._config.thumbnail_max_pixels) as image:
+                imaging.draft(image, size)  # JPEG decodes near the target size
                 oriented = ImageOps.exif_transpose(image)
                 mode = "RGBA" if _has_alpha(oriented) else "RGB"
-                converted = oriented.convert(mode)
+                converted = imaging.to_srgb(oriented, mode=mode)
                 converted.thumbnail((size, size), Image.LANCZOS)
                 out_width, out_height = converted.size
-                destination = self._cache_path(cache_key)
-                destination.parent.mkdir(parents=True, exist_ok=True)
-                tmp = destination.with_name(f".{destination.name}.{uuid.uuid4().hex}.tmp")
                 converted.save(tmp, format="WEBP", quality=82, method=4)
-        except ImageTooLarge:
+        except imaging.ImageTooLarge as error:
+            tmp.unlink(missing_ok=True)
+            raise ImageTooLarge(str(error)) from None
+        except imaging.UndecodableImage as error:
+            tmp.unlink(missing_ok=True)
+            raise UnsupportedImage(str(error)) from None
+        except Exception:
+            tmp.unlink(missing_ok=True)
             raise
-        except (UnidentifiedImageError, OSError, ValueError) as error:
-            raise UnsupportedImage(f"cannot decode {source}: {error}") from error
-
-        import os
 
         os.replace(tmp, destination)
         return {
@@ -253,6 +262,28 @@ class ThumbnailService:
             "format": THUMB_FORMAT,
         }
 
+    def cleanup_temp_files(self) -> int:
+        """Remove leftover render temporaries, and only those.
+
+        Runs at startup, where the concept is explicit: clean *recognized* stale
+        temporary files, never arbitrary ones. The pattern is this service's own —
+        a dot-prefixed name inside the managed cache directory ending in
+        ``.tmp`` — so nothing outside the cache and nothing a user put there can
+        match. Returns how many were removed.
+        """
+        if not self._cache_dir.is_dir():
+            return 0
+        removed = 0
+        for leftover in self._cache_dir.rglob(f".*{TEMP_SUFFIX}"):
+            if not leftover.is_file() or leftover.is_symlink():
+                continue
+            try:
+                leftover.unlink()
+                removed += 1
+            except OSError:  # pragma: no cover - a racing render already won
+                pass
+        return removed
+
     # ── persistence ────────────────────────────────────────────────────────────
     def _record_ready(
         self, cache_key: str, asset_id: str, size: int, rendered: dict, *, protected: bool = False