Serve direct thumbnail URLs to reduce load on main app server

ami/main/api/serializers.py

@@ -1,6 +1,5 @@
 import datetime
 
-from django.conf import settings
 from django.db.models import QuerySet
 from guardian.shortcuts import get_perms
 from rest_framework import serializers
@@ -74,20 +73,16 @@ class Meta:
 
 
 class SourceImageThumbnailSerializer(DefaultSerializer):
+    """Adds a ``thumbnails`` field via :meth:`SourceImage.thumbnail_urls`.
+    Viewsets must apply :meth:`SourceImageQuerySet.with_thumbnails`.
+    """
+
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields["thumbnails"] = serializers.SerializerMethodField()
 
-    def get_thumbnails(self, obj: SourceImage) -> dict | None:
-        return {
-            label: reverse_with_params(
-                "sourceimagethumbnail-detail",
-                args=(obj.pk,),
-                request=self.context.get("request"),
-                params={"label": label},
-            )
-            for label in settings.THUMBNAILS["SIZES"]
-        }
+    def get_thumbnails(self, obj: SourceImage) -> dict[str, str]:
+        return obj.thumbnail_urls(request=self.context.get("request"))
 
 
 class SourceImageNestedSerializer(DefaultSerializer):

ami/main/api/views.py

@@ -451,9 +451,10 @@ def get_queryset(self) -> QuerySet:
                 Prefetch(
                     "captures",
                     queryset=SourceImage.objects.order_by("-size").select_related(
-                        "deployment",
-                        "deployment__data_source",
-                    )[:num_example_captures],
+                        "deployment", "deployment__data_source"
+                    )
+                    # Required by SourceImage.thumbnail_urls in the nested serializer.
+                    .with_thumbnails()[:num_example_captures],
                     to_attr="example_captures",
                 )
             )
@@ -632,11 +633,11 @@ def get_queryset(self) -> QuerySet:
             self.require_project = True
         project = self.get_active_project()
 
-        queryset = queryset.select_related(
-            "event",
-            "deployment",
-            "deployment__data_source",
-        ).order_by("timestamp")
+        queryset = (
+            queryset.select_related("event", "deployment", "deployment__data_source")
+            # Required by SourceImage.thumbnail_urls in SourceImageThumbnailSerializer.
+            .with_thumbnails().order_by("timestamp")
+        )
 
         if self.action == "list":
             # It's cumbersome to override the default list view, so customize the queryset here

ami/main/models.py

@@ -1309,7 +1309,8 @@ def first_capture(self):
         # Ideally this would be an annotated field, rather than an additional query.
         # raise NotImplementedError("This is added an annotated field, it should not be called directly.")
         # return SourceImage.objects.filter(event=self).order_by("timestamp").first().with_detections()
-        return SourceImage.objects.filter(event=self).order_by("timestamp").first()
+        # with_thumbnails() satisfies the thumbnail_urls prefetch contract for the nested serializer.
+        return SourceImage.objects.filter(event=self).order_by("timestamp").with_thumbnails().first()
 
     def summary_data(self):
         """
@@ -2092,6 +2093,12 @@ def with_was_processed(self):
         processed_exists = models.Exists(Detection.objects.filter(source_image_id=models.OuterRef("pk")))
         return self.annotate(was_processed=processed_exists)
 
+    def with_thumbnails(self):
+        """Prefetch ``thumbnails`` so :meth:`SourceImage.thumbnail_urls` decides
+        warm/cold in memory instead of firing a SELECT per row.
+        """
+        return self.prefetch_related("thumbnails")
+
 
 class SourceImageManager(models.Manager.from_queryset(SourceImageQuerySet)):
     pass
@@ -2247,7 +2254,7 @@ def get_was_processed(self, algorithm_key: str | None = None) -> bool:
         ``SourceImageQuerySet.with_was_processed()``). Falls back to a DB query otherwise.
 
         Do not call in bulk without the annotation — use ``with_was_processed()``
-        on the queryset instead to avoid N+1 queries.
+        on the queryset instead so each row does not trigger its own DB query.
 
         :param algorithm_key: If provided, only detections from this algorithm are checked.
                               The annotation does not filter by algorithm; per-algorithm
@@ -2399,6 +2406,60 @@ def get_custom_user_permissions(self, user) -> list[str]:
             custom_perms.add(Project.Permissions.RUN_SINGLE_IMAGE_JOB)
         return list(custom_perms)
 
+    def thumbnail_is_valid(self, spec: dict, thumb: "SourceImageThumbnail | None") -> bool:
+        """Whether ``thumb`` satisfies ``spec`` and need not be regenerated.
+
+        ``thumb.width`` stores the requested spec width (see the generator), so the
+        comparison is strict equality; legacy encoder-width rows read invalid and
+        self-heal on next generation. A None ``last_modified`` on either side means
+        "no signal of change" (matches ``NULL < x`` → ``False`` in SQL).
+        """
+        if thumb is None or not thumb.path or thumb.width != spec["width"]:
+            return False
+        source_changed = (
+            self.last_modified is not None
+            and thumb.last_modified is not None
+            and thumb.last_modified < self.last_modified
+        )
+        return not source_changed
+
+    def thumbnail_urls(self, request: Request | None = None) -> dict[str, str]:
+        """Per-label ``{label: url}`` for this capture's thumbnails.
+
+        Warm (cached row valid for the spec) → direct storage URL. Cold/stale →
+        route URL into the thumbnail viewset, which (re)generates lazily.
+
+        Requires prefetched ``thumbnails`` (use :meth:`SourceImageQuerySet.with_thumbnails`);
+        the guard below turns a forgotten prefetch into a loud error instead of a
+        silent per-row N+1.
+        """
+        # TODO: drop this guard once django-zen-queries enforces prefetch globally.
+        if "thumbnails" not in getattr(self, "_prefetched_objects_cache", {}):
+            raise RuntimeError(
+                "thumbnail_urls() requires prefetched thumbnails — call via SourceImageQuerySet.with_thumbnails()."
+            )
+
+        # Local import avoids a models ↔ serializers cycle at module load time.
+        from ami.base.serializers import reverse_with_params
+
+        thumbs: dict[str, "SourceImageThumbnail"] = {t.label: t for t in self.thumbnails.all()}
+
+        out: dict[str, str] = {}
+        for label, spec in settings.THUMBNAILS["SIZES"].items():
+            thumb = thumbs.get(label)
+            if self.thumbnail_is_valid(spec, thumb):
+                out[label] = default_storage.url(thumb.path)
+            else:
+                # Qualified ``api:`` namespace so this also resolves when ``request``
+                # is None (management commands, template tags).
+                out[label] = reverse_with_params(
+                    "api:sourceimagethumbnail-detail",
+                    args=(self.pk,),
+                    request=request,
+                    params={"label": label},
+                )
+        return out
+
     def find_or_generate_thumbnail_for_label(self, label):
         try:
             thumb = self.thumbnails.get(label=label)
@@ -2407,14 +2468,9 @@ def find_or_generate_thumbnail_for_label(self, label):
         size = settings.THUMBNAILS["SIZES"].get(label)
         prefix = settings.THUMBNAILS["STORAGE_PREFIX"]
 
-        # ``self.last_modified`` can be None on legacy rows synced before upload
-        # backfill — treat None as "no signal of change", not an error.
-        source_changed = (
-            self.last_modified is not None and thumb is not None and thumb.last_modified < self.last_modified
-        )
         # The row is trusted without a storage existence check; an orphan row (blob
         # deleted out of band) shows a broken image until the row is removed.
-        if not thumb or not thumb.path or thumb.width != size["width"] or source_changed:
+        if not self.thumbnail_is_valid(size, thumb):
             img = PIL.Image.open(BytesIO(fetch_image_content(self.public_url(raise_errors=True))))
             # JPEG only supports L, RGB, CMYK — convert other modes (e.g. RGBA PNGs)
             # or PIL raises ``OSError: cannot write mode <X> as JPEG``.

ami/main/tests.py

@@ -518,6 +518,139 @@ def test_thumbnail_delete_removes_storage_blob(self):
 
         self.assertFalse(default_storage.exists(path), "pre_delete signal must clean the storage blob")
 
+    def test_serializer_emits_storage_url_when_thumb_row_exists(self):
+        """Warm-path optimization: when a SourceImageThumbnail row exists with the
+        configured width, the serializer must emit ``default_storage.url(row.path)``
+        directly instead of the lazy-regen route URL. This is the whole point of
+        the direct-URLs change — every cached thumbnail must skip Django on the
+        warm path.
+        """
+        from django.core.files.storage import default_storage
+
+        # Pre-create a thumbnail row whose width matches THUMBNAILS['SIZES']['small'].
+        configured_width = settings.THUMBNAILS["SIZES"]["small"]["width"]
+        self.first_capture.thumbnails.create(
+            path="thumbnails/cached/abc.jpg", label="small", width=configured_width, height=180, size=42
+        )
+
+        response = self.client.get(f"/api/v2/captures/{self.first_capture.pk}/?project_id={self.project.pk}")
+        self.assertEqual(response.status_code, 200)
+        small_url = response.json()["thumbnails"]["small"]
+
+        # ``small`` should be the storage URL, not the route URL.
+        expected_storage_url = default_storage.url("thumbnails/cached/abc.jpg")
+        self.assertEqual(small_url, expected_storage_url)
+        # ``medium`` has no cached row → falls back to the route URL.
+        self.assertURLEqual(
+            response.json()["thumbnails"]["medium"], f"{self.base_url}{self.first_capture.pk}/?label=medium"
+        )
+
+    def test_serializer_falls_back_to_route_url_when_width_is_stale(self):
+        """If the cached row's width no longer matches the configured size (settings
+        changed since last gen), emit the route URL so the next browser fetch
+        triggers lazy regen via the redirect viewset.
+        """
+        configured_width = settings.THUMBNAILS["SIZES"]["small"]["width"]
+        self.first_capture.thumbnails.create(
+            path="thumbnails/stale/abc.jpg",
+            label="small",
+            width=configured_width + 100,  # mismatch — settings changed
+            height=180,
+            size=42,
+        )
+
+        response = self.client.get(f"/api/v2/captures/{self.first_capture.pk}/?project_id={self.project.pk}")
+        self.assertEqual(response.status_code, 200)
+        # Stale width → emit route URL so the redirect viewset can regenerate.
+        self.assertURLEqual(
+            response.json()["thumbnails"]["small"], f"{self.base_url}{self.first_capture.pk}/?label=small"
+        )
+
+    def test_serializer_treats_legacy_encoder_width_row_as_stale(self):
+        """Rows written before the spec-width fix stored PIL's rounded output
+        (e.g. 239 for a 240 spec). The warm-check compares against the spec
+        with strict equality, so such rows must fall back to the route URL —
+        the redirect viewset regenerates them once and they store the spec
+        width from then on.
+        """
+        configured_width = settings.THUMBNAILS["SIZES"]["small"]["width"]
+        # Stored width 1px below spec — what the generator used to record.
+        self.first_capture.thumbnails.create(
+            path="thumbnails/pilround/abc.jpg",
+            label="small",
+            width=configured_width - 1,
+            height=180,
+            size=42,
+        )
+        response = self.client.get(f"/api/v2/captures/{self.first_capture.pk}/?project_id={self.project.pk}")
+        self.assertEqual(response.status_code, 200)
+        # Legacy width → route URL so the next browser fetch self-heals the row.
+        self.assertURLEqual(
+            response.json()["thumbnails"]["small"], f"{self.base_url}{self.first_capture.pk}/?label=small"
+        )
+
+    def test_serializer_falls_back_to_route_url_when_source_changed(self):
+        """If the source image was re-uploaded after the cached row was
+        generated (``source.last_modified > row.last_modified``), the serializer
+        must emit the route URL so the next browser fetch regenerates via the
+        redirect viewset. Mirrors the predicate the generator already uses.
+
+        Guards https://github.com/RolnickLab/antenna/pull/1331#discussion_r3373715397.
+        """
+        configured_width = settings.THUMBNAILS["SIZES"]["small"]["width"]
+        # Row generated at T0. ``last_modified`` is ``auto_now_add`` so set
+        # ``self.first_capture.last_modified`` to a later timestamp via UPDATE
+        # below to flip the staleness predicate.
+        self.first_capture.thumbnails.create(
+            path="thumbnails/preupload/abc.jpg", label="small", width=configured_width, height=180, size=42
+        )
+        # Source bytes were re-uploaded after the cached row was written.
+        SourceImage.objects.filter(pk=self.first_capture.pk).update(
+            last_modified=timezone.now() + datetime.timedelta(minutes=1)
+        )
+
+        response = self.client.get(f"/api/v2/captures/{self.first_capture.pk}/?project_id={self.project.pk}")
+        self.assertEqual(response.status_code, 200)
+        # Source-changed → emit route URL so the redirect viewset regenerates.
+        self.assertURLEqual(
+            response.json()["thumbnails"]["small"], f"{self.base_url}{self.first_capture.pk}/?label=small"
+        )
+
+    def test_thumbnail_urls_model_method_emits_warm_and_route(self):
+        """``SourceImage.thumbnail_urls`` is the single source of truth for the
+        per-label URL contract: warm storage URL when a cached row exists at
+        the configured width, route URL otherwise.
+
+        Exercising the model method directly keeps the contract regression-tested
+        even if the serializer surface changes (e.g. a future template tag or
+        management command that needs the same dict).
+        """
+        from django.core.files.storage import default_storage
+
+        configured_width = settings.THUMBNAILS["SIZES"]["small"]["width"]
+        self.first_capture.thumbnails.create(
+            path="thumbnails/cached/direct.jpg", label="small", width=configured_width, height=180, size=42
+        )
+
+        # Re-fetch through ``with_thumbnails()`` to satisfy the prefetch contract
+        # (thumbnail_urls raises without prefetched thumbnails).
+        capture = SourceImage.objects.with_thumbnails().get(pk=self.first_capture.pk)
+        urls = capture.thumbnail_urls(request=None)
+
+        # Warm path → direct storage URL for the cached label.
+        self.assertEqual(urls["small"], default_storage.url("thumbnails/cached/direct.jpg"))
+        # Cold path → route URL for labels without a cached row. With ``request=None``
+        # the URL is path-only (no scheme/host) — matches DRF's reverse() contract.
+        self.assertIn(f"/api/v2/captures/thumbnails/{self.first_capture.pk}/", urls["medium"])
+        self.assertIn("label=medium", urls["medium"])
+
+    def test_thumbnail_urls_requires_prefetch(self):
+        """``thumbnail_urls`` fails loud without prefetched thumbnails rather than
+        silently firing a per-row SELECT (an N+1 in list contexts)."""
+        capture = SourceImage.objects.get(pk=self.first_capture.pk)
+        with self.assertRaises(RuntimeError):
+            capture.thumbnail_urls(request=None)
+
 
 class TestImageGrouping(TestCase):
     def setUp(self) -> None: