From 58704bd3a4ed9048dc4e1b5a4c0df1ca165d024f Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Wed, 19 Aug 2026 22:17:34 -0400
Subject: [PATCH 1/5] Add Redis-backed caching to API detail and list endpoints

Detail responses cache under a self-versioning key derived from the object's `modified` timestamp, so writes invalidate automatically with no explicit cache-busting. List responses cache under a per-model version counter in Redis, bumped by the existing modified-cascade signals (extended here to cover Publisher/Imprint/Universe/Series/Arc/Character/Team/Issue, plus a new Credits -> Issue cascade that was previously missing). User-scoped viewsets (Collection/PullList/WishList/ReadingList) are intentionally excluded from list caching to avoid leaking one user's data to another.
---
 api/cache.py                                |  93 +++++++++
 api/views.py                                | 202 +++++++++++++++++---
 comicsdb/apps.py                            |  63 ++++++
 comicsdb/signals.py                         |  74 +++++++
 tests/comicsdb/test_api_response_caching.py | 165 ++++++++++++++++
 5 files changed, 566 insertions(+), 31 deletions(-)
 create mode 100644 api/cache.py
 create mode 100644 tests/comicsdb/test_api_response_caching.py

diff --git a/api/cache.py b/api/cache.py
new file mode 100644
index 00000000..16a8c3ff
--- /dev/null
+++ b/api/cache.py
@@ -0,0 +1,93 @@
+"""Redis-backed response caching for the read-only DRF API.
+
+Two independent invalidation schemes are used, depending on endpoint shape:
+
+* Detail responses (retrieve, and detail-scoped actions like issue_list) are
+  cached under a self-versioning key derived from the object's ``modified``
+  timestamp -- when ``modified`` changes, the key changes with it, so old
+  entries are simply orphaned and expire via TTL. No explicit invalidation
+  is needed.
+* List responses (list, and collection-scoped actions like series_list) are
+  cached under a key that includes a per-model cache-generation counter in
+  Redis, bumped by signal handlers (see comicsdb/signals.py) whenever data a
+  list response could embed changes.
+"""
+
+import hashlib
+from collections.abc import Iterable
+from typing import Any
+
+from django.core.cache import cache
+
+DETAIL_CACHE_TTL = 60 * 60 * 24  # 24h safety net; live keys self-invalidate on write.
+LIST_CACHE_TTL = 60 * 2  # 2min; bounds staleness from nested-object changes we don't chase.
+
+_VERSION_KEY_PREFIX = "cachever"
+
+
+class ModelLabel:
+    """Stable cache-key labels shared between signal handlers and views."""
+
+    ARC = "arc"
+    CHARACTER = "character"
+    CREATOR = "creator"
+    IMPRINT = "imprint"
+    ISSUE = "issue"
+    PUBLISHER = "publisher"
+    SERIES = "series"
+    TEAM = "team"
+    UNIVERSE = "universe"
+
+
+def detail_cache_key(model_label: str, pk: Any, modified) -> str:
+    """Cache key for a single object's serialized detail response.
+
+    Self-invalidating: a change to `modified` produces a new key, so old
+    entries are simply orphaned and expire via TTL.
+    """
+    return f"api:detail:{model_label}:{pk}:{modified.timestamp()}"
+
+
+def get_model_version(model_label: str) -> int:
+    """Return the current cache-generation counter for a model, initializing
+    it to 1 on first use."""
+    key = f"{_VERSION_KEY_PREFIX}:{model_label}"
+    version = cache.get(key)
+    if version is None:
+        cache.add(key, 1, timeout=None)
+        version = cache.get(key) or 1
+    return version
+
+
+def bump_model_version(model_label: str) -> None:
+    """Invalidate list caches that depend on `model_label` by advancing its
+    generation counter."""
+    key = f"{_VERSION_KEY_PREFIX}:{model_label}"
+    try:
+        cache.incr(key)
+    except ValueError:
+        # Key doesn't exist yet. At most one concurrent caller's `add` wins;
+        # the other's bump is harmlessly absorbed, since a version key that
+        # didn't exist means no list cache entry was ever computed under any
+        # version of it either.
+        cache.add(key, 1, timeout=None)
+
+
+def list_cache_key(
+    model_label: str,
+    *dependent_labels: str,
+    query: Iterable[tuple[str, list[str]]],
+    scope: str = "",
+) -> str:
+    """Cache key for a list-type response: one or more model versions plus a
+    normalized hash of the request's query params.
+
+    `query` should come from `request.query_params.lists()` (multi-value),
+    not `.dict()` -- `.dict()` silently drops all-but-the-last value for
+    repeated params (e.g. IssueFilter's `role_id`), which would let distinct
+    multi-value requests collide on the same key.
+    """
+    versions = "-".join(str(get_model_version(lbl)) for lbl in (model_label, *dependent_labels))
+    normalized = "&".join(f"{k}={v}" for k, v in sorted(query))
+    digest = hashlib.sha256(normalized.encode()).hexdigest()[:16]
+    return f"api:list:{model_label}:{scope}:{versions}:{digest}"
diff --git a/api/views.py b/api/views.py
index 297a3d41..206b62cc 100644
--- a/api/views.py
+++ b/api/views.py
@@ -1,3 +1,4 @@
+from django.core.cache import cache
 from django.db import models
 from django.db.models import (
     Avg,
@@ -24,6 +25,13 @@
 from rest_framework.response import Response
 from rest_framework_condition import last_modified
 
+from api.cache import (
+    DETAIL_CACHE_TTL,
+    LIST_CACHE_TTL,
+    ModelLabel,
+    detail_cache_key,
+    list_cache_key,
+)
 from api.v1_0.serializers import (
     ArcListSerializer,
     ArcSerializer,
@@ -112,26 +120,80 @@ class ReadingListItemsPagination(PageNumberPagination):
 
 
 class CachedObjectMixin:
+    #: Set on concrete viewsets to enable response caching; None disables it
+    #: (fail-open -- behaves exactly as before this attribute existed).
+    cache_model_label: str | None = None
+
     def get_object(self):
         if not hasattr(self, "_cached_object"):
             self._cached_object = super().get_object()
 
         return self._cached_object
 
+    def get_object_modified(self):
+        """Cheap (pk, modified) lookup for conditional-request checks and
+        cache-key computation -- does NOT trigger the full get_object()
+        queryset (select_related/prefetch_related/annotate). That heavier
+        query only runs lazily if get_object() is later called for an
+        actual cache-miss render.
+        """
+        if not hasattr(self, "_cached_object_modified"):
+            if hasattr(self, "_cached_object"):
+                # get_object() already ran this request -- reuse it instead
+                # of doing a second query.
+                obj = self._cached_object
+                self._cached_object_modified = (
+                    getattr(obj, "pk", None),
+                    getattr(obj, "modified", None),
+                )
+            else:
+                lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field
+                pk = self.kwargs[lookup_url_kwarg]
+                # get_queryset()/filter_queryset(), not a bare Model.objects,
+                # so this still respects per-viewset row scoping (e.g.
+                # CollectionViewSet/PullListViewSet/WishListViewSet filtering
+                # by user).
+                row = (
+                    self.filter_queryset(self.get_queryset())
+                    .filter(**{self.lookup_field: pk})
+                    .values_list(self.lookup_field, "modified")
+                    .first()
+                )
+                self._cached_object_modified = row if row else (None, None)
+
+        return self._cached_object_modified
+
 
 class ConditionalRetrieveModelMixin(CachedObjectMixin, mixins.RetrieveModelMixin):
     def retrieve(self, request, *args, **kwargs):
-        retrieve = last_modified(last_modified_func=self._retrieve_last_modified)(super().retrieve)
+        retrieve = last_modified(last_modified_func=self._retrieve_last_modified)(
+            self._cached_retrieve
+        )
 
         return retrieve(self, request, *args, **kwargs)
 
     def _retrieve_last_modified(self, *args, **kwargs):
-        obj = self.get_object()
+        _pk, modified = self.get_object_modified()
 
-        if obj and getattr(obj, "modified", None):
-            return obj.modified
+        return modified
 
-        return None
+    def _cached_retrieve(self, request, *args, **kwargs):
+        """Reached only once rest_framework_condition's last_modified() has
+        already ruled out a 304 -- i.e. exactly where a cache lookup is
+        worth doing."""
+        pk, modified = self.get_object_modified()
+        if not self.cache_model_label or modified is None:
+            return mixins.RetrieveModelMixin.retrieve(self, request, *args, **kwargs)
+
+        key = detail_cache_key(self.cache_model_label, pk, modified)
+        cached = cache.get(key)
+        if cached is not None:
+            return Response(cached)
+
+        response = mixins.RetrieveModelMixin.retrieve(self, request, *args, **kwargs)
+        if response.status_code == status.HTTP_200_OK:
+            cache.set(key, response.data, DETAIL_CACHE_TTL)
+        return response
 
 
 class UserTrackingMixin:
@@ -144,7 +206,63 @@ def perform_update(self, serializer):
         serializer.save(edited_by=self.request.user)
 
 
-class IssueListMixin(CachedObjectMixin):
+class CachedListModelMixin(mixins.ListModelMixin):
+    """Drop-in replacement for mixins.ListModelMixin that caches list()
+    responses under a per-model cache-generation counter (see api/cache.py).
+    """
+
+    cache_model_label: str | None = None
+    cache_dependent_labels: tuple[str, ...] = ()
+
+    def list(self, request, *args, **kwargs):
+        if not self.cache_model_label:
+            return super().list(request, *args, **kwargs)
+
+        key = list_cache_key(
+            self.cache_model_label,
+            *self.cache_dependent_labels,
+            query=request.query_params.lists(),
+        )
+        cached = cache.get(key)
+        if cached is not None:
+            return Response(cached)
+
+        response = super().list(request, *args, **kwargs)
+        if response.status_code == status.HTTP_200_OK:
+            cache.set(key, response.data, LIST_CACHE_TTL)
+        return response
+
+
+class CachedDetailActionMixin(CachedObjectMixin):
+    """Shared cache-wrapping logic for detail-scoped list actions
+    (issue_list, etc.) that paginate a related queryset off a parent object.
+    Cached under the parent's own `modified` timestamp, since that already
+    captures "has this parent's related collection changed" via the
+    existing modified-cascade signals.
+    """
+
+    def _cached_paginated_action(self, *, build_queryset, serializer_class):
+        pk, modified = self.get_object_modified()
+        key = None
+        if self.cache_model_label and modified is not None:
+            key = detail_cache_key(self.cache_model_label, pk, modified)
+            cached = cache.get(key)
+            if cached is not None:
+                return Response(cached)
+
+        obj = self.get_object()
+        queryset = build_queryset(obj)
+        page = self.paginate_queryset(queryset)
+        if page is None:
+            raise Http404
+        serializer = serializer_class(page, many=True, context={"request": self.request})
+        response = self.get_paginated_response(serializer.data)
+        if key is not None:
+            cache.set(key, response.data, DETAIL_CACHE_TTL)
+        return response
+
+
+class IssueListMixin(CachedDetailActionMixin):
     """Mixin to provide a standard issue_list action for related models."""
 
     def get_issue_queryset(self, obj):
@@ -164,21 +282,15 @@ def issue_list(self, request, *args, **kwargs):
 
     def _issue_list(self, request, *args, **kwargs):
         """Returns a list of issues for this object."""
-        obj = self.get_object()
-        queryset = self.get_issue_queryset(obj)
-        page = self.paginate_queryset(queryset)
-        if page is not None:
-            serializer = IssueListSerializer(page, many=True, context={"request": self.request})
-            return self.get_paginated_response(serializer.data)
-        raise Http404
+        return self._cached_paginated_action(
+            build_queryset=self.get_issue_queryset,
+            serializer_class=IssueListSerializer,
+        )
 
     def _issue_list_last_modified(self, *args, **kwargs):
-        obj = self.get_object()
-
-        if obj and getattr(obj, "modified", None):
-            return obj.modified
+        _pk, modified = self.get_object_modified()
 
-        return None
+        return modified
 
 
 class ArcViewSet(
@@ -186,7 +298,7 @@ class ArcViewSet(
     IssueListMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -201,6 +313,7 @@ class ArcViewSet(
     queryset = Arc.objects.all()
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.ARC
 
     def get_serializer_class(self):
         match self.action:
@@ -217,7 +330,7 @@ class CharacterViewSet(
     IssueListMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -232,6 +345,7 @@ class CharacterViewSet(
     queryset = Character.objects.all()
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.CHARACTER
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -255,7 +369,7 @@ class CreatorViewSet(
     UserTrackingMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -270,6 +384,7 @@ class CreatorViewSet(
     queryset = Creator.objects.all()
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.CREATOR
 
     def get_serializer_class(self):
         match self.action:
@@ -305,7 +420,7 @@ class ImprintViewSet(
     UserTrackingMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -326,6 +441,7 @@ class ImprintViewSet(
     queryset = Imprint.objects.all()
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.IMPRINT
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -347,7 +463,7 @@ class IssueViewSet(
     UserTrackingMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -365,6 +481,7 @@ class IssueViewSet(
     queryset = Issue.objects.all()
     filterset_class = IssueFilter
     parser_classes = (JSONParser, MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.ISSUE
 
     def get_queryset(self):
         if self.action == "list":
@@ -418,7 +535,7 @@ class PublisherViewSet(
     UserTrackingMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -439,6 +556,7 @@ class PublisherViewSet(
     queryset = Publisher.objects.all()
     filterset_class = PublisherFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.PUBLISHER
 
     def get_serializer_class(self):
         match self.action:
@@ -455,6 +573,21 @@ def series_list(self, request, pk=None):
         """
         Returns a list of series for a publisher.
         """
+        # series_list's payload depends on the Series/Issue graph, not on
+        # Publisher.modified (adding a series, or issues under one, doesn't
+        # touch the publisher row) -- so this uses the version-counter list
+        # scheme, scoped to this publisher, rather than a parent-modified
+        # detail key.
+        key = list_cache_key(
+            ModelLabel.SERIES,
+            ModelLabel.ISSUE,
+            query=self.request.query_params.lists(),
+            scope=f"publisher:{pk}:series_list",
+        )
+        cached = cache.get(key)
+        if cached is not None:
+            return Response(cached)
+
         publisher = self.get_object()
         queryset = (
             publisher.series.select_related("series_type")
@@ -462,10 +595,12 @@ def series_list(self, request, pk=None):
             .order_by("sort_name", "year_began")
         )
         page = self.paginate_queryset(queryset)
-        if page is not None:
-            serializer = SeriesListSerializer(page, many=True, context={"request": request})
-            return self.get_paginated_response(serializer.data)
-        raise Http404
+        if page is None:
+            raise Http404
+        serializer = SeriesListSerializer(page, many=True, context={"request": request})
+        response = self.get_paginated_response(serializer.data)
+        cache.set(key, response.data, LIST_CACHE_TTL)
+        return response
 
 
 class RoleViewset(mixins.ListModelMixin, viewsets.GenericViewSet):
@@ -484,7 +619,7 @@ class SeriesViewSet(
     IssueListMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -504,6 +639,9 @@ class SeriesViewSet(
 
     queryset = Series.objects.select_related("series_type", "publisher")
     filterset_class = SeriesFilter
+    cache_model_label = ModelLabel.SERIES
+    # Series list embeds num_issues, which changes on every Issue write.
+    cache_dependent_labels = (ModelLabel.ISSUE,)
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -567,7 +705,7 @@ class TeamViewSet(
     IssueListMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -581,6 +719,7 @@ class TeamViewSet(
 
     queryset = Team.objects.all()
     filterset_class = ComicVineFilter
+    cache_model_label = ModelLabel.TEAM
     parser_classes = (MultiPartParser, FormParser)
 
     def get_queryset(self):
@@ -605,7 +744,7 @@ class UniverseViewSet(
     UserTrackingMixin,
     mixins.CreateModelMixin,
     ConditionalRetrieveModelMixin,
-    mixins.ListModelMixin,
+    CachedListModelMixin,
     mixins.UpdateModelMixin,
     viewsets.GenericViewSet,
 ):
@@ -620,6 +759,7 @@ class UniverseViewSet(
     queryset = Universe.objects.all()
     filterset_class = UniverseFilter
     parser_classes = (MultiPartParser, FormParser)
+    cache_model_label = ModelLabel.UNIVERSE
 
     def get_queryset(self):
         queryset = super().get_queryset()
diff --git a/comicsdb/apps.py b/comicsdb/apps.py
index 2f6b4eb8..9e18e8f1 100644
--- a/comicsdb/apps.py
+++ b/comicsdb/apps.py
@@ -2,10 +2,19 @@
 from django.db.models.signals import m2m_changed, post_delete, post_save, pre_delete
 
 from comicsdb.signals import (
+    bump_arc_cache,
+    bump_character_cache,
+    bump_creator_cache,
+    bump_imprint_cache,
+    bump_publisher_cache,
+    bump_series_cache,
+    bump_team_cache,
+    bump_universe_cache,
     pre_delete_credit,
     pre_delete_image,
     update_arc_modified,
     update_character_modified,
+    update_issue_modified_on_credit_change,
     update_series_modified_on_issue_delete,
     update_series_modified_on_issue_save,
     update_team_modified,
@@ -19,12 +28,26 @@ class ComicsdbConfig(AppConfig):
     def ready(self):
         arc = self.get_model("Arc")
         pre_delete.connect(pre_delete_image, sender=arc, dispatch_uid="pre_delete_arc")
+        post_save.connect(bump_arc_cache, sender=arc, dispatch_uid="post_save_arc_cache")
+        post_delete.connect(bump_arc_cache, sender=arc, dispatch_uid="post_delete_arc_cache")
 
         character = self.get_model("Character")
         pre_delete.connect(pre_delete_image, sender=character, dispatch_uid="pre_delete_character")
+        post_save.connect(
+            bump_character_cache, sender=character, dispatch_uid="post_save_character_cache"
+        )
+        post_delete.connect(
+            bump_character_cache, sender=character, dispatch_uid="post_delete_character_cache"
+        )
 
         creator = self.get_model("Creator")
         pre_delete.connect(pre_delete_image, sender=creator, dispatch_uid="pre_delete_creator")
+        post_save.connect(
+            bump_creator_cache, sender=creator, dispatch_uid="post_save_creator_cache"
+        )
+        post_delete.connect(
+            bump_creator_cache, sender=creator, dispatch_uid="post_delete_creator_cache"
+        )
 
         issue = self.get_model("Issue")
         pre_delete.connect(pre_delete_image, sender=issue, dispatch_uid="pre_delete_issue")
@@ -54,14 +77,54 @@ def ready(self):
             dispatch_uid="m2m_changed_issue_team_modified",
         )
 
+        imprint = self.get_model("Imprint")
+        post_save.connect(
+            bump_imprint_cache, sender=imprint, dispatch_uid="post_save_imprint_cache"
+        )
+        post_delete.connect(
+            bump_imprint_cache, sender=imprint, dispatch_uid="post_delete_imprint_cache"
+        )
+
         publisher = self.get_model("Publisher")
         pre_delete.connect(pre_delete_image, sender=publisher, dispatch_uid="pre_delete_publisher")
+        post_save.connect(
+            bump_publisher_cache, sender=publisher, dispatch_uid="post_save_publisher_cache"
+        )
+        post_delete.connect(
+            bump_publisher_cache, sender=publisher, dispatch_uid="post_delete_publisher_cache"
+        )
+
+        series = self.get_model("Series")
+        post_save.connect(bump_series_cache, sender=series, dispatch_uid="post_save_series_cache")
+        post_delete.connect(
+            bump_series_cache, sender=series, dispatch_uid="post_delete_series_cache"
+        )
 
         team = self.get_model("Team")
         pre_delete.connect(pre_delete_image, sender=team, dispatch_uid="pre_delete_team")
+        post_save.connect(bump_team_cache, sender=team, dispatch_uid="post_save_team_cache")
+        post_delete.connect(bump_team_cache, sender=team, dispatch_uid="post_delete_team_cache")
+
+        universe = self.get_model("Universe")
+        post_save.connect(
+            bump_universe_cache, sender=universe, dispatch_uid="post_save_universe_cache"
+        )
+        post_delete.connect(
+            bump_universe_cache, sender=universe, dispatch_uid="post_delete_universe_cache"
+        )
 
         variant = self.get_model("Variant")
         pre_delete.connect(pre_delete_image, sender=variant, dispatch_uid="pre_delete_variant")
 
         credits_ = self.get_model("Credits")
         pre_delete.connect(pre_delete_credit, sender=credits_, dispatch_uid="pre_delete_credits")
+        post_save.connect(
+            update_issue_modified_on_credit_change,
+            sender=credits_,
+            dispatch_uid="post_save_credit_issue_modified",
+        )
+        post_delete.connect(
+            update_issue_modified_on_credit_change,
+            sender=credits_,
+            dispatch_uid="post_delete_credit_issue_modified",
+        )
diff --git a/comicsdb/signals.py b/comicsdb/signals.py
index a8507dc2..676e4a07 100644
--- a/comicsdb/signals.py
+++ b/comicsdb/signals.py
@@ -16,15 +16,21 @@ def pre_delete_credit(sender, instance, **kwargs):
 
 
 def update_series_modified_on_issue_save(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Series  # noqa: PLC0415
 
     Series.objects.filter(pk=instance.series_id).update(modified=timezone.now())
+    bump_model_version(ModelLabel.ISSUE)
+    bump_model_version(ModelLabel.SERIES)
 
 
 def update_series_modified_on_issue_delete(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Series  # noqa: PLC0415
 
     Series.objects.filter(pk=instance.series_id).update(modified=timezone.now())
+    bump_model_version(ModelLabel.ISSUE)
+    bump_model_version(ModelLabel.SERIES)
 
 
 def update_related_modified(parent_model, instance, action, pk_set):
@@ -44,18 +50,86 @@ def update_related_modified(parent_model, instance, action, pk_set):
 
 
 def update_arc_modified(sender, instance, action, pk_set, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Arc  # noqa: PLC0415
 
     update_related_modified(Arc, instance, action, pk_set)
+    if action in ("post_add", "post_remove", "post_clear"):
+        bump_model_version(ModelLabel.ARC)
 
 
 def update_character_modified(sender, instance, action, pk_set, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Character  # noqa: PLC0415
 
     update_related_modified(Character, instance, action, pk_set)
+    if action in ("post_add", "post_remove", "post_clear"):
+        bump_model_version(ModelLabel.CHARACTER)
 
 
 def update_team_modified(sender, instance, action, pk_set, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Team  # noqa: PLC0415
 
     update_related_modified(Team, instance, action, pk_set)
+    if action in ("post_add", "post_remove", "post_clear"):
+        bump_model_version(ModelLabel.TEAM)
+
+
+def update_issue_modified_on_credit_change(sender, instance, **kwargs):
+    """Credits changes aren't reflected on the parent Issue's `modified` by
+    default; bump it explicitly so the issue's cached detail response
+    (which embeds credits) invalidates."""
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+    from comicsdb.models import Issue  # noqa: PLC0415
+
+    Issue.objects.filter(pk=instance.issue_id).update(modified=timezone.now())
+    bump_model_version(ModelLabel.ISSUE)
+
+
+def bump_arc_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.ARC)
+
+
+def bump_character_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.CHARACTER)
+
+
+def bump_creator_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.CREATOR)
+
+
+def bump_imprint_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.IMPRINT)
+
+
+def bump_publisher_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.PUBLISHER)
+
+
+def bump_series_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.SERIES)
+
+
+def bump_team_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.TEAM)
+
+
+def bump_universe_cache(sender, instance, **kwargs):
+    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+
+    bump_model_version(ModelLabel.UNIVERSE)
diff --git a/tests/comicsdb/test_api_response_caching.py b/tests/comicsdb/test_api_response_caching.py
new file mode 100644
index 00000000..2775f184
--- /dev/null
+++ b/tests/comicsdb/test_api_response_caching.py
@@ -0,0 +1,165 @@
+import uuid
+from unittest.mock import patch
+
+import pytest
+from django.core.cache.backends.locmem import LocMemCache
+from django.db import connection
+from django.test.utils import CaptureQueriesContext
+from django.urls import reverse
+from rest_framework import status
+
+from api.views import CollectionViewSet, PullListViewSet, WishListViewSet
+from comicsdb.models import Credits
+
+
+@pytest.fixture
+def local_cache():
+    """Isolate the response cache so assertions about exact hit/miss/version
+    behavior aren't affected by concurrent xdist workers sharing the real
+    Redis backend (the `cachever:*` counters are global-per-model)."""
+    test_cache = LocMemCache(f"test-api-response-caching-{uuid.uuid4()}", {})
+    with patch("api.views.cache", test_cache), patch("api.cache.cache", test_cache):
+        yield test_cache
+
+
+def test_issue_retrieve_cache_hit_skips_heavy_query(
+    api_client_with_credentials, basic_issue, local_cache
+):
+    url = reverse("api:issue-detail", kwargs={"pk": basic_issue.pk})
+    resp = api_client_with_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    first_body = resp.json()
+
+    with CaptureQueriesContext(connection) as queries:
+        resp = api_client_with_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json() == first_body
+
+    # Cache hit: none of the heavy prefetch/join queries the full retrieve
+    # queryset would run should appear -- only the cheap (pk, modified)
+    # lookup used for the conditional-request/cache-key check.
+    sql_statements = [q["sql"] for q in queries.captured_queries]
+    assert not any("comicsdb_credits" in sql for sql in sql_statements)
+    assert not any("comicsdb_issue_arcs" in sql for sql in sql_statements)
+
+
+def test_issue_retrieve_after_update_is_not_stale(
+    api_client_with_staff_credentials, basic_issue, local_cache
+):
+    url = reverse("api:issue-detail", kwargs={"pk": basic_issue.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["desc"] != "Updated description"
+
+    resp = api_client_with_staff_credentials.patch(url, {"desc": "Updated description"})
+    assert resp.status_code == status.HTTP_200_OK
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["desc"] == "Updated description"
+
+
+def test_credits_change_busts_issue_detail_cache(
+    api_client_with_staff_credentials, basic_issue, john_byrne, writer, local_cache
+):
+    url = reverse("api:issue-detail", kwargs={"pk": basic_issue.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["credits"] == []
+
+    credit = Credits.objects.create(issue=basic_issue, creator=john_byrne)
+    credit.role.add(writer)
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert len(resp.json()["credits"]) == 1
+    assert resp.json()["credits"][0]["creator"] == john_byrne.name
+
+
+def test_arc_list_reflects_newly_created_arc(
+    api_client_with_staff_credentials, wwh_arc, local_cache
+):
+    url = reverse("api:arc-list")
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 1
+
+    resp = api_client_with_staff_credentials.post(url, {"name": "Final Crisis", "desc": "New arc"})
+    assert resp.status_code == status.HTTP_201_CREATED
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 2
+
+
+def test_arc_list_reflects_deleted_arc_via_admin(
+    api_client_with_credentials, wwh_arc, fc_arc, local_cache
+):
+    url = reverse("api:arc-list")
+    resp = api_client_with_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 2
+
+    fc_arc.delete()
+
+    resp = api_client_with_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 1
+
+
+def test_series_list_reflects_new_issue_num_issues(
+    api_client_with_staff_credentials, fc_series, basic_issue, local_cache
+):
+    url = reverse("api:series-list")
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    result = next(r for r in resp.json()["results"] if r["id"] == fc_series.pk)
+    assert result["issue_count"] == 1
+
+    resp = api_client_with_staff_credentials.post(
+        reverse("api:issue-list"),
+        {"series": fc_series.pk, "number": "2", "cover_date": "2008-01-01"},
+    )
+    assert resp.status_code == status.HTTP_201_CREATED
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    result = next(r for r in resp.json()["results"] if r["id"] == fc_series.pk)
+    assert result["issue_count"] == 2
+
+
+def test_publisher_series_list_reflects_new_series(
+    api_client_with_staff_credentials, dc_comics, fc_series, single_issue_type, local_cache
+):
+    url = reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 1
+
+    resp = api_client_with_staff_credentials.post(
+        reverse("api:series-list"),
+        {
+            "name": "New Series",
+            "sort_name": "New Series",
+            "volume": 1,
+            "publisher": dc_comics.pk,
+            "series_type": single_issue_type.pk,
+            "year_began": 2024,
+            "status": 4,
+        },
+    )
+    assert resp.status_code == status.HTTP_201_CREATED
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["count"] == 2
+
+
+def test_user_scoped_viewsets_are_not_list_cached():
+    """CollectionViewSet/PullListViewSet/WishListViewSet are user-scoped
+    (get_queryset filters by request.user) -- they must never use
+    CachedListModelMixin, since a shared list cache key would serve one
+    user's private data to another. This is a cheap regression guard for
+    that intentional exclusion; see api/views.py."""
+    for viewset in (CollectionViewSet, PullListViewSet, WishListViewSet):
+        assert getattr(viewset, "cache_model_label", None) is None

From 3f2906fc1dd9d7680c0257077602f4bc30fda1a8 Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Thu, 20 Aug 2026 07:05:13 -0400
Subject: [PATCH 2/5] Fix response-cache staleness gaps and query cost in API
 caching

Several detail/action caches keyed off an object's own `modified` missed edits to related data that don't cascade a bump onto it:
Arc/Character/Team issue_list didn't see plain issue field edits, and Series retrieve didn't see Publisher/Imprint renames. Both now mix the dependent model's version counter into the cache key.

Also fixes a race where CreditSerializer.create() bumped Issue.modified before attaching roles, letting a request cache an issue with an empty role list under a key nothing would ever invalidate; a new m2m_changed signal on Credits.role bumps again once roles actually land.

IssueViewSet/SeriesViewSet's cheap (pk, modified) lookup was still carrying the retrieve queryset's annotate() aggregates into an extra JOIN + GROUP BY on every request; get_modified_queryset() now skips them. Also drops the now-unnecessary deferred api.cache imports in signals.py and de-duplicates the per-model cache-bump functions.
---
 api/cache.py                                | 14 +++-
 api/views.py                                | 70 ++++++++++++++++++--
 comicsdb/apps.py                            |  6 ++
 comicsdb/signals.py                         | 73 ++++++++-------------
 tests/comicsdb/test_api_response_caching.py | 67 +++++++++++++++++++
 5 files changed, 176 insertions(+), 54 deletions(-)

diff --git a/api/cache.py b/api/cache.py
index 16a8c3ff..eba992bd 100644
--- a/api/cache.py
+++ b/api/cache.py
@@ -39,13 +39,23 @@ class ModelLabel:
     UNIVERSE = "universe"
 
 
-def detail_cache_key(model_label: str, pk: Any, modified) -> str:
+def detail_cache_key(model_label: str, pk: Any, modified, *dependent_labels: str) -> str:
     """Cache key for a single object's serialized detail response.
 
     Self-invalidating: a change to `modified` produces a new key, so old
     entries are simply orphaned and expire via TTL.
+
+    `dependent_labels` (optional) mix in other models' version counters, for
+    responses that embed data from a related object whose own edits don't
+    cascade a `modified` bump onto this one (e.g. a Series response embeds
+    its Publisher's name, but renaming the Publisher doesn't touch the
+    Series row).
     """
-    return f"api:detail:{model_label}:{pk}:{modified.timestamp()}"
+    key = f"api:detail:{model_label}:{pk}:{modified.timestamp()}"
+    if dependent_labels:
+        versions = "-".join(str(get_model_version(lbl)) for lbl in dependent_labels)
+        key = f"{key}:{versions}"
+    return key
 
 
 def get_model_version(model_label: str) -> int:
diff --git a/api/views.py b/api/views.py
index 206b62cc..31e5fc8c 100644
--- a/api/views.py
+++ b/api/views.py
@@ -130,6 +130,17 @@ def get_object(self):
 
         return self._cached_object
 
+    def get_modified_queryset(self):
+        """Queryset used by get_object_modified() for its (pk, modified)
+        lookup. Defaults to get_queryset(), but a viewset whose
+        get_queryset() adds annotate() aggregates should override this to
+        return a lean, unannotated queryset with the same row-level
+        permission scoping -- an annotated aggregate still forces a JOIN +
+        GROUP BY even when values_list() doesn't select the annotated
+        field.
+        """
+        return self.get_queryset()
+
     def get_object_modified(self):
         """Cheap (pk, modified) lookup for conditional-request checks and
         cache-key computation -- does NOT trigger the full get_object()
@@ -149,12 +160,12 @@ def get_object_modified(self):
             else:
                 lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field
                 pk = self.kwargs[lookup_url_kwarg]
-                # get_queryset()/filter_queryset(), not a bare Model.objects,
-                # so this still respects per-viewset row scoping (e.g.
-                # CollectionViewSet/PullListViewSet/WishListViewSet filtering
-                # by user).
+                # get_modified_queryset()/filter_queryset(), not a bare
+                # Model.objects, so this still respects per-viewset row
+                # scoping (e.g. CollectionViewSet/PullListViewSet/
+                # WishListViewSet filtering by user).
                 row = (
-                    self.filter_queryset(self.get_queryset())
+                    self.filter_queryset(self.get_modified_queryset())
                     .filter(**{self.lookup_field: pk})
                     .values_list(self.lookup_field, "modified")
                     .first()
@@ -165,6 +176,12 @@ def get_object_modified(self):
 
 
 class ConditionalRetrieveModelMixin(CachedObjectMixin, mixins.RetrieveModelMixin):
+    #: Other models' cache-generation counters (see api/cache.py) to mix
+    #: into the detail cache key, for responses that embed a related
+    #: object's fields where an edit to that related object doesn't cascade
+    #: a `modified` bump onto this one.
+    cache_detail_dependent_labels: tuple[str, ...] = ()
+
     def retrieve(self, request, *args, **kwargs):
         retrieve = last_modified(last_modified_func=self._retrieve_last_modified)(
             self._cached_retrieve
@@ -185,7 +202,9 @@ def _cached_retrieve(self, request, *args, **kwargs):
         if not self.cache_model_label or modified is None:
             return mixins.RetrieveModelMixin.retrieve(self, request, *args, **kwargs)
 
-        key = detail_cache_key(self.cache_model_label, pk, modified)
+        key = detail_cache_key(
+            self.cache_model_label, pk, modified, *self.cache_detail_dependent_labels
+        )
         cached = cache.get(key)
         if cached is not None:
             return Response(cached)
@@ -241,11 +260,21 @@ class CachedDetailActionMixin(CachedObjectMixin):
     existing modified-cascade signals.
     """
 
+    #: Other models' cache-generation counters (see api/cache.py) to mix
+    #: into the action's cache key, for actions whose payload can change
+    #: without the parent object's own `modified` being touched (e.g.
+    #: issue_list embeds fields from Issue rows that don't cascade a
+    #: `modified` bump onto the parent Arc/Character/Team on every edit --
+    #: only on M2M add/remove/clear).
+    cache_action_dependent_labels: tuple[str, ...] = ()
+
     def _cached_paginated_action(self, *, build_queryset, serializer_class):
         pk, modified = self.get_object_modified()
         key = None
         if self.cache_model_label and modified is not None:
-            key = detail_cache_key(self.cache_model_label, pk, modified)
+            key = detail_cache_key(
+                self.cache_model_label, pk, modified, *self.cache_action_dependent_labels
+            )
             cached = cache.get(key)
             if cached is not None:
                 return Response(cached)
@@ -314,6 +343,9 @@ class ArcViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.ARC
+    # issue_list embeds fields from Issue rows that don't cascade a
+    # `modified` bump onto this Arc except on M2M add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE,)
 
     def get_serializer_class(self):
         match self.action:
@@ -346,6 +378,9 @@ class CharacterViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.CHARACTER
+    # issue_list embeds fields from Issue rows that don't cascade a
+    # `modified` bump onto this Character except on M2M add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE,)
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -483,6 +518,14 @@ class IssueViewSet(
     parser_classes = (JSONParser, MultiPartParser, FormParser)
     cache_model_label = ModelLabel.ISSUE
 
+    def get_modified_queryset(self):
+        # get_queryset() annotates average_rating/rating_count for the
+        # retrieve action -- an aggregate that survives into (pk, modified)
+        # values_list() as an extra JOIN + GROUP BY even though neither
+        # annotated field is selected. This lookup only needs the row's own
+        # pk/modified, so skip the annotation entirely.
+        return Issue.objects.all()
+
     def get_queryset(self):
         if self.action == "list":
             return Issue.objects.select_related("series", "series__series_type")
@@ -642,6 +685,16 @@ class SeriesViewSet(
     cache_model_label = ModelLabel.SERIES
     # Series list embeds num_issues, which changes on every Issue write.
     cache_dependent_labels = (ModelLabel.ISSUE,)
+    # Series retrieve embeds its Publisher/Imprint name, which don't cascade
+    # a `modified` bump onto this Series when renamed.
+    cache_detail_dependent_labels = (ModelLabel.PUBLISHER, ModelLabel.IMPRINT)
+
+    def get_modified_queryset(self):
+        # get_queryset() annotates num_issues for list/retrieve -- an
+        # aggregate that survives into (pk, modified) values_list() as an
+        # extra JOIN + GROUP BY even though the annotated field isn't
+        # selected. This lookup only needs the row's own pk/modified.
+        return Series.objects.all()
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -721,6 +774,9 @@ class TeamViewSet(
     filterset_class = ComicVineFilter
     cache_model_label = ModelLabel.TEAM
     parser_classes = (MultiPartParser, FormParser)
+    # issue_list embeds fields from Issue rows that don't cascade a
+    # `modified` bump onto this Team except on M2M add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE,)
 
     def get_queryset(self):
         queryset = super().get_queryset()
diff --git a/comicsdb/apps.py b/comicsdb/apps.py
index 9e18e8f1..854e74b7 100644
--- a/comicsdb/apps.py
+++ b/comicsdb/apps.py
@@ -15,6 +15,7 @@
     update_arc_modified,
     update_character_modified,
     update_issue_modified_on_credit_change,
+    update_issue_modified_on_credit_role_change,
     update_series_modified_on_issue_delete,
     update_series_modified_on_issue_save,
     update_team_modified,
@@ -128,3 +129,8 @@ def ready(self):
             sender=credits_,
             dispatch_uid="post_delete_credit_issue_modified",
         )
+        m2m_changed.connect(
+            update_issue_modified_on_credit_role_change,
+            sender=credits_.role.through,
+            dispatch_uid="m2m_changed_credit_role_modified",
+        )
diff --git a/comicsdb/signals.py b/comicsdb/signals.py
index 676e4a07..446be9ed 100644
--- a/comicsdb/signals.py
+++ b/comicsdb/signals.py
@@ -3,6 +3,8 @@
 from django.utils import timezone
 from sorl.thumbnail import delete
 
+from api.cache import ModelLabel, bump_model_version
+
 LOGGER = logging.getLogger(__name__)
 
 
@@ -16,7 +18,6 @@ def pre_delete_credit(sender, instance, **kwargs):
 
 
 def update_series_modified_on_issue_save(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Series  # noqa: PLC0415
 
     Series.objects.filter(pk=instance.series_id).update(modified=timezone.now())
@@ -25,7 +26,6 @@ def update_series_modified_on_issue_save(sender, instance, **kwargs):
 
 
 def update_series_modified_on_issue_delete(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Series  # noqa: PLC0415
 
     Series.objects.filter(pk=instance.series_id).update(modified=timezone.now())
@@ -50,7 +50,6 @@ def update_related_modified(parent_model, instance, action, pk_set):
 
 
 def update_arc_modified(sender, instance, action, pk_set, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Arc  # noqa: PLC0415
 
     update_related_modified(Arc, instance, action, pk_set)
@@ -59,7 +58,6 @@ def update_arc_modified(sender, instance, action, pk_set, **kwargs):
 
 
 def update_character_modified(sender, instance, action, pk_set, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Character  # noqa: PLC0415
 
     update_related_modified(Character, instance, action, pk_set)
@@ -68,7 +66,6 @@ def update_character_modified(sender, instance, action, pk_set, **kwargs):
 
 
 def update_team_modified(sender, instance, action, pk_set, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Team  # noqa: PLC0415
 
     update_related_modified(Team, instance, action, pk_set)
@@ -80,56 +77,42 @@ def update_issue_modified_on_credit_change(sender, instance, **kwargs):
     """Credits changes aren't reflected on the parent Issue's `modified` by
     default; bump it explicitly so the issue's cached detail response
     (which embeds credits) invalidates."""
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
     from comicsdb.models import Issue  # noqa: PLC0415
 
     Issue.objects.filter(pk=instance.issue_id).update(modified=timezone.now())
     bump_model_version(ModelLabel.ISSUE)
 
 
-def bump_arc_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
-
-    bump_model_version(ModelLabel.ARC)
-
-
-def bump_character_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
-
-    bump_model_version(ModelLabel.CHARACTER)
-
-
-def bump_creator_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
-
-    bump_model_version(ModelLabel.CREATOR)
-
-
-def bump_imprint_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
-
-    bump_model_version(ModelLabel.IMPRINT)
-
-
-def bump_publisher_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
-
-    bump_model_version(ModelLabel.PUBLISHER)
-
-
-def bump_series_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
+def update_issue_modified_on_credit_role_change(sender, instance, action, pk_set, **kwargs):
+    """Credits.role is a M2M -- .add()/.remove()/.set() don't call
+    Credits.save(), so update_issue_modified_on_credit_change (a post_save
+    hook) never fires for role-only changes. In particular,
+    CreditSerializer.create() calls Credits.objects.create() (bumping
+    Issue.modified with an empty role list) and only then calls
+    credit.role.add(...): without this second bump, a request landing in
+    that window could cache the issue with an empty role list under a
+    `modified` key that's never touched again."""
+    if action not in ("post_add", "post_remove", "post_clear"):
+        return
 
-    bump_model_version(ModelLabel.SERIES)
+    from comicsdb.models import Issue  # noqa: PLC0415
 
+    Issue.objects.filter(pk=instance.issue_id).update(modified=timezone.now())
+    bump_model_version(ModelLabel.ISSUE)
 
-def bump_team_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
 
-    bump_model_version(ModelLabel.TEAM)
+def _make_cache_bumper(label):
+    def bump_cache(sender, instance, **kwargs):
+        bump_model_version(label)
 
+    return bump_cache
 
-def bump_universe_cache(sender, instance, **kwargs):
-    from api.cache import ModelLabel, bump_model_version  # noqa: PLC0415
 
-    bump_model_version(ModelLabel.UNIVERSE)
+bump_arc_cache = _make_cache_bumper(ModelLabel.ARC)
+bump_character_cache = _make_cache_bumper(ModelLabel.CHARACTER)
+bump_creator_cache = _make_cache_bumper(ModelLabel.CREATOR)
+bump_imprint_cache = _make_cache_bumper(ModelLabel.IMPRINT)
+bump_publisher_cache = _make_cache_bumper(ModelLabel.PUBLISHER)
+bump_series_cache = _make_cache_bumper(ModelLabel.SERIES)
+bump_team_cache = _make_cache_bumper(ModelLabel.TEAM)
+bump_universe_cache = _make_cache_bumper(ModelLabel.UNIVERSE)
diff --git a/tests/comicsdb/test_api_response_caching.py b/tests/comicsdb/test_api_response_caching.py
index 2775f184..78996258 100644
--- a/tests/comicsdb/test_api_response_caching.py
+++ b/tests/comicsdb/test_api_response_caching.py
@@ -155,6 +155,73 @@ def test_publisher_series_list_reflects_new_series(
     assert resp.json()["count"] == 2
 
 
+def test_arc_issue_list_reflects_issue_field_edit(
+    api_client_with_staff_credentials, issue_with_arc, fc_arc, local_cache
+):
+    """issue_list is cached under the parent Arc's own `modified`, which
+    only bumps on M2M add/remove/clear -- not on a plain field edit to one
+    of the linked issues. cache_action_dependent_labels ties the cache key
+    to the Issue model's version counter too, so an edit like this should
+    still be visible without waiting for the parent's `modified` to catch
+    up."""
+    url = reverse("api:arc-issue-list", kwargs={"pk": fc_arc.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["results"][0]["number"] == "1"
+
+    issue_with_arc.number = "2"
+    issue_with_arc.save()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["results"][0]["number"] == "2"
+
+
+def test_series_retrieve_after_publisher_rename_is_not_stale(
+    api_client_with_staff_credentials, fc_series, dc_comics, local_cache
+):
+    """Series retrieve embeds the Publisher's name, but renaming a
+    Publisher doesn't bump the owning Series' `modified`.
+    cache_detail_dependent_labels ties the Series detail cache key to the
+    Publisher model's version counter too, so the rename should show up
+    without waiting on the Series' own `modified`."""
+    url = reverse("api:series-detail", kwargs={"pk": fc_series.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["publisher"]["name"] == "DC Comics"
+
+    dc_comics.name = "DC Comics Renamed"
+    dc_comics.save()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["publisher"]["name"] == "DC Comics Renamed"
+
+
+def test_credit_role_add_after_create_does_not_stick_empty_roles(
+    api_client_with_staff_credentials, basic_issue, john_byrne, writer, local_cache
+):
+    """CreditSerializer.create() calls Credits.objects.create() (bumping
+    Issue.modified once) and only then credit.role.add(...). A request
+    landing between those two steps -- simulated here by fetching before
+    role.add() runs -- must not permanently cache the issue with an empty
+    role list: the m2m_changed bump on role.add() has to produce a new
+    `modified` value so the stale entry is orphaned rather than reused."""
+    url = reverse("api:issue-detail", kwargs={"pk": basic_issue.pk})
+
+    credit = Credits.objects.create(issue=basic_issue, creator=john_byrne)
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["credits"][0]["role"] == []
+
+    credit.role.add(writer)
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["credits"][0]["role"][0]["name"] == "Writer"
+
+
 def test_user_scoped_viewsets_are_not_list_cached():
     """CollectionViewSet/PullListViewSet/WishListViewSet are user-scoped
     (get_queryset filters by request.user) -- they must never use

From 42569ad3f025e4fceb29e45a89a3c5c54152dd31 Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Thu, 20 Aug 2026 07:40:31 -0400
Subject: [PATCH 3/5] Close remaining cache-staleness gaps and restore
 series_list existence check

Issue retrieve embeds Series/Publisher/Imprint names, and Imprint/Universe retrieve embed their Publisher's name, none of which cascade a `modified` bump onto the cached object. Arc/Character/Team issue_list also nests each issue's Series name without depending on it. All now mix in the relevant model's version counter, same pattern already used for Series. Character/
Team retrieve still omit Creator/Universe on purpose -- those are edited far more often than Publisher/Imprint, so tracking them would tank thecache hit rate for comparatively little benefit; documented inline.

Also fixes PublisherViewSet.series_list, which skipped get_object() (and therefore the existence/permission check) entirely on a cache hit -- a deleted publisher's cached series list kept returning 200 instead of 404 until the 2min list-cache TTL caught up.
---
 api/views.py                                | 70 ++++++++++++++++---
 tests/comicsdb/test_api_response_caching.py | 74 +++++++++++++++++++++
 2 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/api/views.py b/api/views.py
index 31e5fc8c..ca66ca2d 100644
--- a/api/views.py
+++ b/api/views.py
@@ -197,7 +197,17 @@ def _retrieve_last_modified(self, *args, **kwargs):
     def _cached_retrieve(self, request, *args, **kwargs):
         """Reached only once rest_framework_condition's last_modified() has
         already ruled out a 304 -- i.e. exactly where a cache lookup is
-        worth doing."""
+        worth doing.
+
+        Note: a cache hit returns without calling get_object(), so
+        check_object_permissions() never runs on that path. Every
+        permission class in this project is view-level only (none override
+        has_object_permission), so this is currently inert -- but a future
+        object-level permission class on a cache_model_label-enabled
+        viewset would need get_object_modified() (or this method) to also
+        enforce it explicitly, since DRF's default only calls it from
+        get_object().
+        """
         pk, modified = self.get_object_modified()
         if not self.cache_model_label or modified is None:
             return mixins.RetrieveModelMixin.retrieve(self, request, *args, **kwargs)
@@ -343,9 +353,10 @@ class ArcViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.ARC
-    # issue_list embeds fields from Issue rows that don't cascade a
-    # `modified` bump onto this Arc except on M2M add/remove/clear.
-    cache_action_dependent_labels = (ModelLabel.ISSUE,)
+    # issue_list embeds fields from Issue rows (and their Series) that
+    # don't cascade a `modified` bump onto this Arc except on M2M
+    # add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE, ModelLabel.SERIES)
 
     def get_serializer_class(self):
         match self.action:
@@ -378,9 +389,18 @@ class CharacterViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.CHARACTER
-    # issue_list embeds fields from Issue rows that don't cascade a
-    # `modified` bump onto this Character except on M2M add/remove/clear.
-    cache_action_dependent_labels = (ModelLabel.ISSUE,)
+    # retrieve also embeds Creator/Universe names (CharacterReadSerializer)
+    # that don't cascade a `modified` bump onto this Character either, but
+    # those are deliberately left as bounded (TTL-limited) staleness rather
+    # than a cache_detail_dependent_labels dependency -- Creators in
+    # particular are edited/added far more often than Publishers/Imprints,
+    # so tying this key to CREATOR's version would invalidate every cached
+    # Character detail on essentially any creator edit anywhere.
+    #
+    # issue_list embeds fields from Issue rows (and their Series) that
+    # don't cascade a `modified` bump onto this Character except on M2M
+    # add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE, ModelLabel.SERIES)
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -477,6 +497,9 @@ class ImprintViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.IMPRINT
+    # Imprint retrieve embeds its Publisher's name, which doesn't cascade a
+    # `modified` bump onto this Imprint when renamed.
+    cache_detail_dependent_labels = (ModelLabel.PUBLISHER,)
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -517,6 +540,14 @@ class IssueViewSet(
     filterset_class = IssueFilter
     parser_classes = (JSONParser, MultiPartParser, FormParser)
     cache_model_label = ModelLabel.ISSUE
+    # Issue retrieve embeds its Series/Publisher/Imprint names, which don't
+    # cascade a `modified` bump onto this Issue when renamed. Arc/
+    # Character/Team/Universe/Creator names are also embedded but
+    # deliberately excluded here -- those are edited/created far more
+    # often, and mixing them in would invalidate every cached issue detail
+    # response on essentially every catalog edit anywhere, not just ones
+    # affecting this issue.
+    cache_detail_dependent_labels = (ModelLabel.PUBLISHER, ModelLabel.IMPRINT, ModelLabel.SERIES)
 
     def get_modified_queryset(self):
         # get_queryset() annotates average_rating/rating_count for the
@@ -616,6 +647,12 @@ def series_list(self, request, pk=None):
         """
         Returns a list of series for a publisher.
         """
+        # get_object() (existence + permission check) first, same as any
+        # other detail-scoped action -- a cache hit must not bypass either
+        # of those, and must not keep serving a 200 once the publisher
+        # itself has been deleted.
+        publisher = self.get_object()
+
         # series_list's payload depends on the Series/Issue graph, not on
         # Publisher.modified (adding a series, or issues under one, doesn't
         # touch the publisher row) -- so this uses the version-counter list
@@ -631,7 +668,6 @@ def series_list(self, request, pk=None):
         if cached is not None:
             return Response(cached)
 
-        publisher = self.get_object()
         queryset = (
             publisher.series.select_related("series_type")
             .annotate(num_issues=Count("issues", distinct=True))
@@ -774,9 +810,18 @@ class TeamViewSet(
     filterset_class = ComicVineFilter
     cache_model_label = ModelLabel.TEAM
     parser_classes = (MultiPartParser, FormParser)
-    # issue_list embeds fields from Issue rows that don't cascade a
-    # `modified` bump onto this Team except on M2M add/remove/clear.
-    cache_action_dependent_labels = (ModelLabel.ISSUE,)
+    # retrieve also embeds Creator/Universe names (TeamReadSerializer) that
+    # don't cascade a `modified` bump onto this Team either, but those are
+    # deliberately left as bounded (TTL-limited) staleness rather than a
+    # cache_detail_dependent_labels dependency -- Creators in particular
+    # are edited/added far more often than Publishers/Imprints, so tying
+    # this key to CREATOR's version would invalidate every cached Team
+    # detail on essentially any creator edit anywhere.
+    #
+    # issue_list embeds fields from Issue rows (and their Series) that
+    # don't cascade a `modified` bump onto this Team except on M2M
+    # add/remove/clear.
+    cache_action_dependent_labels = (ModelLabel.ISSUE, ModelLabel.SERIES)
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -816,6 +861,9 @@ class UniverseViewSet(
     filterset_class = UniverseFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.UNIVERSE
+    # Universe retrieve embeds its Publisher's name, which doesn't cascade a
+    # `modified` bump onto this Universe when renamed.
+    cache_detail_dependent_labels = (ModelLabel.PUBLISHER,)
 
     def get_queryset(self):
         queryset = super().get_queryset()
diff --git a/tests/comicsdb/test_api_response_caching.py b/tests/comicsdb/test_api_response_caching.py
index 78996258..a44f36a1 100644
--- a/tests/comicsdb/test_api_response_caching.py
+++ b/tests/comicsdb/test_api_response_caching.py
@@ -155,6 +155,23 @@ def test_publisher_series_list_reflects_new_series(
     assert resp.json()["count"] == 2
 
 
+def test_publisher_series_list_404s_after_publisher_deleted(
+    api_client_with_staff_credentials, dc_comics, fc_series, local_cache
+):
+    """series_list must call get_object() (existence check) before serving
+    a cache hit -- otherwise a deleted publisher's cached series list would
+    keep returning 200 instead of 404 until the list cache TTL expires."""
+    url = reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+
+    fc_series.delete()
+    dc_comics.delete()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_404_NOT_FOUND
+
+
 def test_arc_issue_list_reflects_issue_field_edit(
     api_client_with_staff_credentials, issue_with_arc, fc_arc, local_cache
 ):
@@ -222,6 +239,63 @@ def test_credit_role_add_after_create_does_not_stick_empty_roles(
     assert resp.json()["credits"][0]["role"][0]["name"] == "Writer"
 
 
+def test_issue_retrieve_after_series_rename_is_not_stale(
+    api_client_with_staff_credentials, basic_issue, fc_series, local_cache
+):
+    """Issue retrieve embeds its Series' name, but renaming a Series
+    doesn't bump the owning Issue's `modified`. cache_detail_dependent_labels
+    ties the Issue detail cache key to the Series model's version counter
+    too, so the rename should show up without waiting on the Issue's own
+    `modified`."""
+    url = reverse("api:issue-detail", kwargs={"pk": basic_issue.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["series"]["name"] == "Final Crisis"
+
+    fc_series.name = "Final Crisis Renamed"
+    fc_series.save()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["series"]["name"] == "Final Crisis Renamed"
+
+
+def test_imprint_retrieve_after_publisher_rename_is_not_stale(
+    api_client_with_staff_credentials, vertigo_imprint, dc_comics, local_cache
+):
+    """Imprint retrieve embeds its Publisher's name, but renaming a
+    Publisher doesn't bump the owning Imprint's `modified`."""
+    url = reverse("api:imprint-detail", kwargs={"pk": vertigo_imprint.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["publisher"]["name"] == "DC Comics"
+
+    dc_comics.name = "DC Comics Renamed"
+    dc_comics.save()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["publisher"]["name"] == "DC Comics Renamed"
+
+
+def test_arc_issue_list_reflects_series_rename(
+    api_client_with_staff_credentials, issue_with_arc, fc_arc, fc_series, local_cache
+):
+    """issue_list nests each issue's Series name; renaming the Series
+    doesn't bump the parent Arc's `modified`."""
+    url = reverse("api:arc-issue-list", kwargs={"pk": fc_arc.pk})
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["results"][0]["series"]["name"] == "Final Crisis"
+
+    fc_series.name = "Final Crisis Renamed"
+    fc_series.save()
+
+    resp = api_client_with_staff_credentials.get(url)
+    assert resp.status_code == status.HTTP_200_OK
+    assert resp.json()["results"][0]["series"]["name"] == "Final Crisis Renamed"
+
+
 def test_user_scoped_viewsets_are_not_list_cached():
     """CollectionViewSet/PullListViewSet/WishListViewSet are user-scoped
     (get_queryset filters by request.user) -- they must never use

From 6aaa8be11984aeb772c9e5dc93dad48e182d4920 Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Thu, 20 Aug 2026 08:10:05 -0400
Subject: [PATCH 4/5] Simplify cache-bump wiring and batch Redis version
 lookups

ModelLabel is now a StrEnum instead of a plain string-constant class, so a typo'd label is caught by type checking instead of silently creating a version counter that's never invalidated.

list_cache_key()/detail_cache_key() now fetch all dependent labels' version counters in one cache.get_many() instead of one cache.get() per label, and get_model_version()'s cold-start path drops from 3 Redis round trips to 2 (1 for the winning first caller) by checking cache.add()'s return value instead of re-reading afterward.

comicsdb/apps.py's 16 near-identical post_save/post_delete .connect() calls for the 8 "just bump my version counter" models (Arc, Character, Creator, Imprint, Publisher, Series, Team, Universe) collapse into a table-driven loop backed by a single bump_cache() receiver in signals.py, replacing the 8 near-identical wrapper functions it used to require.
weak=False is required here since the loop's functools.partial receivers have no other strong reference; verified all 8 models still bump their counter on save and delete before trusting it.
---
 api/cache.py        | 32 ++++++++++++++----
 comicsdb/apps.py    | 79 +++++++++++++++++++--------------------------
 comicsdb/signals.py | 22 ++++---------
 3 files changed, 65 insertions(+), 68 deletions(-)

diff --git a/api/cache.py b/api/cache.py
index eba992bd..4dca6d2b 100644
--- a/api/cache.py
+++ b/api/cache.py
@@ -15,6 +15,7 @@
 
 import hashlib
 from collections.abc import Iterable
+from enum import StrEnum
 from typing import Any
 
 from django.core.cache import cache
@@ -25,7 +26,7 @@
 _VERSION_KEY_PREFIX = "cachever"
 
 
-class ModelLabel:
+class ModelLabel(StrEnum):
     """Stable cache-key labels shared between signal handlers and views."""
 
     ARC = "arc"
@@ -53,7 +54,8 @@ def detail_cache_key(model_label: str, pk: Any, modified, *dependent_labels: str
     """
     key = f"api:detail:{model_label}:{pk}:{modified.timestamp()}"
     if dependent_labels:
-        versions = "-".join(str(get_model_version(lbl)) for lbl in dependent_labels)
+        version_map = get_model_versions(dependent_labels)
+        versions = "-".join(str(version_map[lbl]) for lbl in dependent_labels)
         key = f"{key}:{versions}"
     return key
 
@@ -63,10 +65,24 @@ def get_model_version(model_label: str) -> int:
     it to 1 on first use."""
     key = f"{_VERSION_KEY_PREFIX}:{model_label}"
     version = cache.get(key)
-    if version is None:
-        cache.add(key, 1, timeout=None)
-        version = cache.get(key) or 1
-    return version
+    if version is not None:
+        return version
+    if cache.add(key, 1, timeout=None):
+        return 1
+    # Lost the initialization race to another caller -- read back what they set.
+    return cache.get(key) or 1
+
+
+def get_model_versions(model_labels: Iterable[str]) -> dict[str, int]:
+    """Batch form of get_model_version(): one Redis round trip (get_many)
+    for the common case where every counter already exists, instead of one
+    round trip per label."""
+    labels = list(dict.fromkeys(model_labels))  # de-dupe, preserve order
+    keys = {lbl: f"{_VERSION_KEY_PREFIX}:{lbl}" for lbl in labels}
+    cached = cache.get_many(keys.values())
+    return {
+        lbl: cached[key] if key in cached else get_model_version(lbl) for lbl, key in keys.items()
+    }
 
 
 def bump_model_version(model_label: str) -> None:
@@ -97,7 +113,9 @@ def list_cache_key(
     repeated params (e.g. IssueFilter's `role_id`), which would let distinct
     multi-value requests collide on the same key.
     """
-    versions = "-".join(str(get_model_version(lbl)) for lbl in (model_label, *dependent_labels))
+    labels = (model_label, *dependent_labels)
+    version_map = get_model_versions(labels)
+    versions = "-".join(str(version_map[lbl]) for lbl in labels)
     normalized = "&".join(f"{k}={v}" for k, v in sorted(query))
     digest = hashlib.sha256(normalized.encode()).hexdigest()[:16]
     return f"api:list:{model_label}:{scope}:{versions}:{digest}"
diff --git a/comicsdb/apps.py b/comicsdb/apps.py
index 854e74b7..c9c03359 100644
--- a/comicsdb/apps.py
+++ b/comicsdb/apps.py
@@ -1,15 +1,11 @@
+from functools import partial
+
 from django.apps import AppConfig
 from django.db.models.signals import m2m_changed, post_delete, post_save, pre_delete
 
+from api.cache import ModelLabel
 from comicsdb.signals import (
-    bump_arc_cache,
-    bump_character_cache,
-    bump_creator_cache,
-    bump_imprint_cache,
-    bump_publisher_cache,
-    bump_series_cache,
-    bump_team_cache,
-    bump_universe_cache,
+    bump_cache,
     pre_delete_credit,
     pre_delete_image,
     update_arc_modified,
@@ -29,26 +25,12 @@ class ComicsdbConfig(AppConfig):
     def ready(self):
         arc = self.get_model("Arc")
         pre_delete.connect(pre_delete_image, sender=arc, dispatch_uid="pre_delete_arc")
-        post_save.connect(bump_arc_cache, sender=arc, dispatch_uid="post_save_arc_cache")
-        post_delete.connect(bump_arc_cache, sender=arc, dispatch_uid="post_delete_arc_cache")
 
         character = self.get_model("Character")
         pre_delete.connect(pre_delete_image, sender=character, dispatch_uid="pre_delete_character")
-        post_save.connect(
-            bump_character_cache, sender=character, dispatch_uid="post_save_character_cache"
-        )
-        post_delete.connect(
-            bump_character_cache, sender=character, dispatch_uid="post_delete_character_cache"
-        )
 
         creator = self.get_model("Creator")
         pre_delete.connect(pre_delete_image, sender=creator, dispatch_uid="pre_delete_creator")
-        post_save.connect(
-            bump_creator_cache, sender=creator, dispatch_uid="post_save_creator_cache"
-        )
-        post_delete.connect(
-            bump_creator_cache, sender=creator, dispatch_uid="post_delete_creator_cache"
-        )
 
         issue = self.get_model("Issue")
         pre_delete.connect(pre_delete_image, sender=issue, dispatch_uid="pre_delete_issue")
@@ -79,40 +61,16 @@ def ready(self):
         )
 
         imprint = self.get_model("Imprint")
-        post_save.connect(
-            bump_imprint_cache, sender=imprint, dispatch_uid="post_save_imprint_cache"
-        )
-        post_delete.connect(
-            bump_imprint_cache, sender=imprint, dispatch_uid="post_delete_imprint_cache"
-        )
 
         publisher = self.get_model("Publisher")
         pre_delete.connect(pre_delete_image, sender=publisher, dispatch_uid="pre_delete_publisher")
-        post_save.connect(
-            bump_publisher_cache, sender=publisher, dispatch_uid="post_save_publisher_cache"
-        )
-        post_delete.connect(
-            bump_publisher_cache, sender=publisher, dispatch_uid="post_delete_publisher_cache"
-        )
 
         series = self.get_model("Series")
-        post_save.connect(bump_series_cache, sender=series, dispatch_uid="post_save_series_cache")
-        post_delete.connect(
-            bump_series_cache, sender=series, dispatch_uid="post_delete_series_cache"
-        )
 
         team = self.get_model("Team")
         pre_delete.connect(pre_delete_image, sender=team, dispatch_uid="pre_delete_team")
-        post_save.connect(bump_team_cache, sender=team, dispatch_uid="post_save_team_cache")
-        post_delete.connect(bump_team_cache, sender=team, dispatch_uid="post_delete_team_cache")
 
         universe = self.get_model("Universe")
-        post_save.connect(
-            bump_universe_cache, sender=universe, dispatch_uid="post_save_universe_cache"
-        )
-        post_delete.connect(
-            bump_universe_cache, sender=universe, dispatch_uid="post_delete_universe_cache"
-        )
 
         variant = self.get_model("Variant")
         pre_delete.connect(pre_delete_image, sender=variant, dispatch_uid="pre_delete_variant")
@@ -134,3 +92,32 @@ def ready(self):
             sender=credits_.role.through,
             dispatch_uid="m2m_changed_credit_role_modified",
         )
+
+        # Models whose cache invalidation is *only* "bump my own version
+        # counter" on every save/delete -- see bump_cache() in
+        # comicsdb/signals.py. Everything above this point wires up
+        # handlers with model-specific behavior (image cleanup, modified
+        # cascades); this is the uniform remainder.
+        cache_bump_models = (
+            (arc, ModelLabel.ARC),
+            (character, ModelLabel.CHARACTER),
+            (creator, ModelLabel.CREATOR),
+            (imprint, ModelLabel.IMPRINT),
+            (publisher, ModelLabel.PUBLISHER),
+            (series, ModelLabel.SERIES),
+            (team, ModelLabel.TEAM),
+            (universe, ModelLabel.UNIVERSE),
+        )
+        for model, label in cache_bump_models:
+            bumper = partial(bump_cache, label)
+            # weak=False: `bumper` is a local `partial` with no other
+            # strong reference, so Django's default weak-reference
+            # receiver storage would let it be garbage-collected right
+            # after this loop iteration ends, silently dropping the
+            # connection.
+            post_save.connect(
+                bumper, sender=model, weak=False, dispatch_uid=f"post_save_{label}_cache"
+            )
+            post_delete.connect(
+                bumper, sender=model, weak=False, dispatch_uid=f"post_delete_{label}_cache"
+            )
diff --git a/comicsdb/signals.py b/comicsdb/signals.py
index 446be9ed..8e454a51 100644
--- a/comicsdb/signals.py
+++ b/comicsdb/signals.py
@@ -101,18 +101,10 @@ def update_issue_modified_on_credit_role_change(sender, instance, action, pk_set
     bump_model_version(ModelLabel.ISSUE)
 
 
-def _make_cache_bumper(label):
-    def bump_cache(sender, instance, **kwargs):
-        bump_model_version(label)
-
-    return bump_cache
-
-
-bump_arc_cache = _make_cache_bumper(ModelLabel.ARC)
-bump_character_cache = _make_cache_bumper(ModelLabel.CHARACTER)
-bump_creator_cache = _make_cache_bumper(ModelLabel.CREATOR)
-bump_imprint_cache = _make_cache_bumper(ModelLabel.IMPRINT)
-bump_publisher_cache = _make_cache_bumper(ModelLabel.PUBLISHER)
-bump_series_cache = _make_cache_bumper(ModelLabel.SERIES)
-bump_team_cache = _make_cache_bumper(ModelLabel.TEAM)
-bump_universe_cache = _make_cache_bumper(ModelLabel.UNIVERSE)
+def bump_cache(label, sender, instance, **kwargs):
+    """Generic post_save/post_delete receiver for the models whose cache
+    invalidation is *only* "bump my own version counter" -- Arc, Character,
+    Creator, Imprint, Publisher, Series, Team, Universe. Wired up via
+    functools.partial(bump_cache, label) in comicsdb/apps.py so one
+    function covers all eight instead of eight near-identical wrappers."""
+    bump_model_version(label)

From 8a310e9928e04cfc84a7712381247fada6ca8411 Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Thu, 20 Aug 2026 08:25:16 -0400
Subject: [PATCH 5/5] Add audit_response_cache management command

Deciding whether to raise or lower DETAIL_CACHE_TTL/LIST_CACHE_TTL needs actual production data, not guesswork: how much of Redis the API cache accounts for, how it's split across models, and whether Redis is already evicting under memory pressure before the TTL ever gets a say.

Scans the full keyspace once (cursor-based, safe against a large production keyspace), buckets keys by api:detail:<model>/api:list:<model>/cachever/other, and estimates per-bucket memory by sampling MEMORY USAGE rather than calling it on every key. Also reports global hit rate and evicted_keys from Redis INFO. Meant to be run manually against production after this caching work deploys, and periodically afterward to compare.
---
 .../commands/audit_response_cache.py          | 166 ++++++++++++++++++
 tests/api/test_audit_response_cache.py        |  82 +++++++++
 2 files changed, 248 insertions(+)
 create mode 100644 api/management/commands/audit_response_cache.py
 create mode 100644 tests/api/test_audit_response_cache.py

diff --git a/api/management/commands/audit_response_cache.py b/api/management/commands/audit_response_cache.py
new file mode 100644
index 00000000..ee457a9a
--- /dev/null
+++ b/api/management/commands/audit_response_cache.py
@@ -0,0 +1,166 @@
+import random
+import time
+from collections import defaultdict
+
+from django.core.cache import cache
+from django.core.management.base import BaseCommand
+
+_DETAIL_PREFIX = "api:detail:"
+_LIST_PREFIX = "api:list:"
+_VERSION_PREFIX = "cachever:"
+_BYTES_PER_UNIT = 1024
+
+
+class Command(BaseCommand):
+    help = (
+        "One-off audit of the Redis-backed API response cache -- key counts and "
+        "estimated memory footprint per category (api:detail:<model>, "
+        "api:list:<model>, cachever, everything else), plus global hit-rate and "
+        "eviction stats. Meant to be run against production shortly after the "
+        "caching PR deploys (and again later) to see whether DETAIL_CACHE_TTL/"
+        "LIST_CACHE_TTL (api/cache.py) need tuning, rather than guessing."
+    )
+
+    def add_arguments(self, parser) -> None:
+        parser.add_argument(
+            "--sample-size",
+            type=int,
+            default=300,
+            help=(
+                "Max keys to sample per category for MEMORY USAGE, so the "
+                "estimate doesn't require calling it on every key in a large "
+                "keyspace (default: 300)"
+            ),
+        )
+        parser.add_argument(
+            "--scan-count",
+            type=int,
+            default=1000,
+            help="COUNT hint passed to Redis SCAN per iteration (default: 1000)",
+        )
+
+    def handle(self, *args, **options) -> None:
+        # Django's generic cache API has no SCAN/MEMORY USAGE/INFO -- those
+        # require the raw redis-py client the RedisCache backend wraps.
+        client = cache._cache.get_client()
+
+        self._print_global_stats(client)
+        categories, total_keys, elapsed = self._scan_and_categorize(client, options["scan_count"])
+        self.stdout.write(f"\nScanned {total_keys:,} keys in {elapsed:.1f}s.")
+        self._print_category_report(client, categories, options["sample_size"])
+
+    def _print_global_stats(self, client) -> None:
+        try:
+            memory = client.info("memory")
+            stats = client.info("stats")
+        except Exception as exc:  # noqa: BLE001 -- best-effort diagnostics
+            self.stdout.write(
+                self.style.WARNING(f"INFO command unavailable ({exc}); skipping global stats.")
+            )
+            return
+
+        hits = stats.get("keyspace_hits", 0)
+        misses = stats.get("keyspace_misses", 0)
+        total = hits + misses
+        hit_rate = f"{hits / total:.1%}" if total else "n/a"
+
+        self.stdout.write(
+            self.style.MIGRATE_HEADING("Redis instance (global -- all keys, not just ours)")
+        )
+        self.stdout.write(f"  used_memory:       {memory.get('used_memory_human', '?')}")
+        maxmemory = memory.get("maxmemory", 0)
+        unbounded = "  (unbounded -- no eviction policy in effect)" if not maxmemory else ""
+        self.stdout.write(f"  maxmemory:         {memory.get('maxmemory_human', '?')}{unbounded}")
+        self.stdout.write(f"  maxmemory_policy:  {memory.get('maxmemory_policy', '?')}")
+        self.stdout.write(f"  mem_fragmentation: {memory.get('mem_fragmentation_ratio', '?')}")
+        self.stdout.write(f"  keyspace hit rate: {hit_rate} ({hits:,} hits / {misses:,} misses)")
+
+        evicted = stats.get("evicted_keys", 0)
+        evicted_line = f"  evicted_keys:      {evicted:,}"
+        if evicted:
+            evicted_line = self.style.WARNING(
+                f"{evicted_line}  <-- Redis is evicting under memory pressure; "
+                "TTLs alone aren't controlling memory here, lower them or add memory"
+            )
+        self.stdout.write(evicted_line)
+
+    def _scan_and_categorize(self, client, scan_count: int):
+        categories: dict[str, list[bytes]] = defaultdict(list)
+        total_keys = 0
+        cursor = 0
+        start = time.monotonic()
+        while True:
+            cursor, keys = client.scan(cursor=cursor, count=scan_count)
+            for raw_key in keys:
+                total_keys += 1
+                key = raw_key.decode() if isinstance(raw_key, bytes) else raw_key
+                categories[self._categorize(key)].append(raw_key)
+            if cursor == 0:
+                break
+        return categories, total_keys, time.monotonic() - start
+
+    @staticmethod
+    def _categorize(key: str) -> str:
+        # Substring match rather than startswith(): robust to however Django's
+        # RedisCache backend wraps the logical key (e.g. a ":<version>:"
+        # prefix), without needing to know its exact format.
+        if _DETAIL_PREFIX in key:
+            model = key.split(_DETAIL_PREFIX, 1)[1].split(":", 1)[0]
+            return f"api:detail:{model}"
+        if _LIST_PREFIX in key:
+            model = key.split(_LIST_PREFIX, 1)[1].split(":", 1)[0]
+            return f"api:list:{model}"
+        if _VERSION_PREFIX in key:
+            return "cachever"
+        return "other (Select2, throttling, etc.)"
+
+    def _print_category_report(self, client, categories: dict, sample_size: int) -> None:
+        self.stdout.write(self.style.MIGRATE_HEADING("\nBy category"))
+        if not categories:
+            self.stdout.write("  No keys found.")
+            return
+
+        rows = []
+        for label, keys in categories.items():
+            count = len(keys)
+            avg_bytes = self._sample_avg_memory(client, keys, sample_size)
+            est_total = avg_bytes * count if avg_bytes is not None else None
+            rows.append((label, count, avg_bytes, est_total))
+        rows.sort(key=lambda row: row[3] or 0, reverse=True)
+
+        for label, count, avg_bytes, est_total in rows:
+            avg_str = f"~{avg_bytes:,.0f} B/key" if avg_bytes is not None else "n/a"
+            total_str = self._human_bytes(est_total)
+            self.stdout.write(f"  {label:<32} {count:>8,} keys   {avg_str:>14}   est. {total_str}")
+
+        known = sum(est for *_ignored, est in rows if est is not None)
+        self.stdout.write(
+            f"\n  Estimated total across sampled categories: {self._human_bytes(known)}"
+        )
+        self.stdout.write(
+            "  (Estimates extrapolate from a random sample's MEMORY USAGE -- "
+            "re-run periodically and compare, not just once.)"
+        )
+
+    @staticmethod
+    def _sample_avg_memory(client, keys: list, sample_size: int) -> float | None:
+        sample = keys if len(keys) <= sample_size else random.sample(keys, sample_size)
+        sizes = []
+        for key in sample:
+            try:
+                size = client.memory_usage(key)
+            except Exception:  # noqa: BLE001, S112 -- best-effort; key may have expired mid-scan
+                continue
+            if size is not None:
+                sizes.append(size)
+        return sum(sizes) / len(sizes) if sizes else None
+
+    @staticmethod
+    def _human_bytes(n: float | None) -> str:
+        if n is None:
+            return "n/a"
+        for unit in ("B", "KB", "MB", "GB"):
+            if n < _BYTES_PER_UNIT:
+                return f"{n:,.1f} {unit}"
+            n /= _BYTES_PER_UNIT
+        return f"{n:,.1f} TB"
diff --git a/tests/api/test_audit_response_cache.py b/tests/api/test_audit_response_cache.py
new file mode 100644
index 00000000..37b405cc
--- /dev/null
+++ b/tests/api/test_audit_response_cache.py
@@ -0,0 +1,82 @@
+import io
+import uuid
+from unittest.mock import patch
+
+from django.core.cache import cache
+from django.core.management import call_command
+
+from api.management.commands.audit_response_cache import Command
+
+COMMAND = "audit_response_cache"
+
+
+def test_categorize_detail_key_extracts_model_label():
+    key = ":1:api:detail:issue:42:1704085200.0"
+    assert Command._categorize(key) == "api:detail:issue"
+
+
+def test_categorize_detail_key_with_dependent_labels_still_extracts_model():
+    key = ":1:api:detail:series:7:1704085200.0:12-3"
+    assert Command._categorize(key) == "api:detail:series"
+
+
+def test_categorize_list_key_extracts_model_label():
+    key = ":1:api:list:arc::4-2:abcdef0123456789"
+    assert Command._categorize(key) == "api:list:arc"
+
+
+def test_categorize_cachever_key():
+    assert Command._categorize(":1:cachever:issue") == "cachever"
+
+
+def test_categorize_unrelated_key_falls_back_to_other():
+    assert Command._categorize(":1:django_select2:some-widget-id") == (
+        "other (Select2, throttling, etc.)"
+    )
+
+
+def test_human_bytes_formats_across_units():
+    assert Command._human_bytes(512) == "512.0 B"
+    assert Command._human_bytes(2048) == "2.0 KB"
+    assert Command._human_bytes(5 * 1024 * 1024) == "5.0 MB"
+    assert Command._human_bytes(None) == "n/a"
+
+
+def test_report_counts_and_sizes_a_seeded_category():
+    """Seed a handful of keys under a category unique to this test run (a
+    random model label) so the assertion is immune to whatever other keys
+    real Redis happens to hold -- other test workers/dev usage share this
+    same Redis instance, and the command intentionally scans everything."""
+    label = f"audit-test-{uuid.uuid4().hex[:8]}"
+    keys = [f"api:detail:{label}:{i}:1704085200.0" for i in range(3)]
+    for key in keys:
+        cache.set(key, {"payload": "x" * 50}, 60)
+
+    out = io.StringIO()
+    try:
+        call_command(COMMAND, stdout=out)
+    finally:
+        cache.delete_many(keys)
+
+    output = out.getvalue()
+    assert f"api:detail:{label}" in output
+    assert "3 keys" in output
+    assert "Redis instance (global" in output
+    assert "By category" in output
+
+
+def test_info_failure_is_reported_without_crashing():
+    class _BrokenClient:
+        def info(self, *_args, **_kwargs):
+            raise ConnectionError("redis unavailable")
+
+        def scan(self, cursor=0, **_kwargs):
+            return 0, []
+
+    with patch("django.core.cache.cache._cache.get_client", return_value=_BrokenClient()):
+        out = io.StringIO()
+        call_command(COMMAND, stdout=out)
+
+    output = out.getvalue()
+    assert "INFO command unavailable" in output
+    assert "redis unavailable" in output