Fix Issue cache over- and under-invalidation

api/views.py

@@ -365,10 +365,17 @@ class ArcViewSet(
     filterset_class = ComicVineFilter
     parser_classes = (MultiPartParser, FormParser)
     cache_model_label = ModelLabel.ARC
-    # 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)
+    # 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. ModelLabel.ISSUE/SERIES are deliberately NOT used as
+    # cache_action_dependent_labels here: both are bumped by
+    # update_series_modified_on_issue_save() on *every* issue write
+    # anywhere on the site, so tying this 24h-TTL cache to either
+    # invalidates every Arc's issue_list on essentially any issue edit
+    # site-wide -- confirmed as a live problem for IssueViewSet's own
+    # retrieve cache in production (see its cache_detail_dependent_labels
+    # comment). A plain issue field edit can show stale here for up to
+    # DETAIL_CACHE_TTL; that's the accepted tradeoff.
 
     def get_serializer_class(self):
         match self.action:
@@ -409,10 +416,14 @@ class CharacterViewSet(
     # 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)
+    # 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. ModelLabel.ISSUE/SERIES are deliberately NOT used as
+    # cache_action_dependent_labels here either, for the same reason as
+    # CREATOR above: both are bumped by update_series_modified_on_issue_save()
+    # on *every* issue write anywhere on the site, so tying this 24h-TTL
+    # cache to either invalidates every Character's issue_list on
+    # essentially any issue edit site-wide.
 
     def get_queryset(self):
         queryset = super().get_queryset()
@@ -553,13 +564,22 @@ class IssueViewSet(
     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)
+    # cascade a `modified` bump onto this Issue when renamed. Only
+    # PUBLISHER/IMPRINT are tracked here:
+    # - SERIES is deliberately excluded even though it's also embedded --
+    #   ModelLabel.SERIES is bumped by update_series_modified_on_issue_save()
+    #   on *every* issue write anywhere (PublisherViewSet.series_list needs
+    #   that), so including it here invalidated every cached issue detail
+    #   response on essentially every issue edit site-wide, not just ones
+    #   affecting this issue's own Series (confirmed in production: a
+    #   single unrelated issue write elsewhere flipped an otherwise-stable
+    #   X-Cache HIT to a MISS).
+    # - Arc/Character/Team/Universe/Creator names are also embedded but
+    #   excluded for the same reason -- edited/created far more often than
+    #   Publisher/Imprint, so mixing them in would cost far more cache
+    #   churn than the staleness they'd prevent.
+    # Both cases accept staleness up to DETAIL_CACHE_TTL as the tradeoff.
+    cache_detail_dependent_labels = (ModelLabel.PUBLISHER, ModelLabel.IMPRINT)
 
     def get_modified_queryset(self):
         # get_queryset() annotates average_rating/rating_count for the
@@ -830,10 +850,14 @@ class TeamViewSet(
     # 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)
+    # 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. ModelLabel.ISSUE/SERIES are deliberately NOT used as
+    # cache_action_dependent_labels here either, for the same reason as
+    # CREATOR above: both are bumped by update_series_modified_on_issue_save()
+    # on *every* issue write anywhere on the site, so tying this 24h-TTL
+    # cache to either invalidates every Team's issue_list on essentially
+    # any issue edit site-wide.
 
     def get_queryset(self):
         queryset = super().get_queryset()

comicsdb/apps.py

@@ -12,6 +12,9 @@
     update_character_modified,
     update_issue_modified_on_credit_change,
     update_issue_modified_on_credit_role_change,
+    update_issue_modified_on_reprint_change,
+    update_issue_modified_on_universe_change,
+    update_issue_modified_on_variant_change,
     update_series_modified_on_issue_delete,
     update_series_modified_on_issue_save,
     update_team_modified,
@@ -59,6 +62,16 @@ def ready(self):
             sender=issue.teams.through,
             dispatch_uid="m2m_changed_issue_team_modified",
         )
+        m2m_changed.connect(
+            update_issue_modified_on_universe_change,
+            sender=issue.universes.through,
+            dispatch_uid="m2m_changed_issue_universe_modified",
+        )
+        m2m_changed.connect(
+            update_issue_modified_on_reprint_change,
+            sender=issue.reprints.through,
+            dispatch_uid="m2m_changed_issue_reprint_modified",
+        )
 
         imprint = self.get_model("Imprint")
 
@@ -74,6 +87,16 @@ def ready(self):
 
         variant = self.get_model("Variant")
         pre_delete.connect(pre_delete_image, sender=variant, dispatch_uid="pre_delete_variant")
+        post_save.connect(
+            update_issue_modified_on_variant_change,
+            sender=variant,
+            dispatch_uid="post_save_variant_issue_modified",
+        )
+        post_delete.connect(
+            update_issue_modified_on_variant_change,
+            sender=variant,
+            dispatch_uid="post_delete_variant_issue_modified",
+        )
 
         credits_ = self.get_model("Credits")
         pre_delete.connect(pre_delete_credit, sender=credits_, dispatch_uid="pre_delete_credits")

comicsdb/signals.py

@@ -34,19 +34,33 @@ def update_series_modified_on_issue_delete(sender, instance, **kwargs):
 
 
 def update_related_modified(parent_model, instance, action, pk_set):
-    """Shared logic for M2M post_add/post_remove/post_clear on Arc, Character, Team."""
+    """Shared logic for M2M post_add/post_remove/post_clear on Arc, Character, Team.
+
+    Bumps both sides of the relationship's `modified`: the parent (Arc/
+    Character/Team -- for its own issue_list cache) and the specific
+    Issue(s) involved (for that issue's own cached detail response, which
+    embeds this relationship). Both updates are scoped by pk/pk_set to the
+    objects actually affected -- never a blanket update -- so this doesn't
+    reintroduce the cross-contamination that ModelLabel.ISSUE/SERIES had as
+    global version counters.
+    """
     if action not in ("post_add", "post_remove", "post_clear"):
         return
 
     from comicsdb.models import Issue  # noqa: PLC0415
 
+    now = timezone.now()
     if isinstance(instance, Issue):
+        # issue.arcs.add(...)/.remove()/.clear() -- instance is the Issue.
+        Issue.objects.filter(pk=instance.pk).update(modified=now)
         # pk_set is None for post_clear; skip since affected parents are unknown
         if pk_set:
-            parent_model.objects.filter(pk__in=pk_set).update(modified=timezone.now())
+            parent_model.objects.filter(pk__in=pk_set).update(modified=now)
     else:
         # instance is the parent (e.g. arc.issues.add/clear(...))
-        parent_model.objects.filter(pk=instance.pk).update(modified=timezone.now())
+        parent_model.objects.filter(pk=instance.pk).update(modified=now)
+        if pk_set:
+            Issue.objects.filter(pk__in=pk_set).update(modified=now)
 
 
 def update_arc_modified(sender, instance, action, pk_set, **kwargs):
@@ -73,6 +87,48 @@ def update_team_modified(sender, instance, action, pk_set, **kwargs):
         bump_model_version(ModelLabel.TEAM)
 
 
+def update_issue_modified_on_universe_change(sender, instance, action, pk_set, **kwargs):
+    """Issue.universes is a M2M, but -- unlike arcs/characters/teams --
+    Universe has no issue_list-style action needing its own side bumped,
+    so only the Issue side needs updating here for the issue's cached
+    detail response (which embeds `universes`) to invalidate."""
+    if action not in ("post_add", "post_remove", "post_clear"):
+        return
+
+    from comicsdb.models import Issue  # noqa: PLC0415
+
+    now = timezone.now()
+    if isinstance(instance, Issue):
+        Issue.objects.filter(pk=instance.pk).update(modified=now)
+    elif pk_set:
+        Issue.objects.filter(pk__in=pk_set).update(modified=now)
+
+
+def update_issue_modified_on_reprint_change(sender, instance, action, pk_set, **kwargs):
+    """Issue.reprints is a symmetric self-referential M2M -- both sides of
+    a reprints.add()/.remove() are Issue instances, so bump both the issue
+    the change was made through and the affected issue(s) on the other
+    side; both cached detail responses embed `reprints`."""
+    if action not in ("post_add", "post_remove", "post_clear"):
+        return
+
+    from comicsdb.models import Issue  # noqa: PLC0415
+
+    now = timezone.now()
+    Issue.objects.filter(pk=instance.pk).update(modified=now)
+    if pk_set:
+        Issue.objects.filter(pk__in=pk_set).update(modified=now)
+
+
+def update_issue_modified_on_variant_change(sender, instance, **kwargs):
+    """Variant changes aren't reflected on the parent Issue's `modified` by
+    default; bump it explicitly so the issue's cached detail response
+    (which embeds variants) invalidates."""
+    from comicsdb.models import Issue  # noqa: PLC0415
+
+    Issue.objects.filter(pk=instance.issue_id).update(modified=timezone.now())
+
+
 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

tests/comicsdb/test_api_arc.py

@@ -90,6 +90,11 @@ def test_unauthorized_detail_view_url(api_client, wwh_arc):
 
 def test_arc_issue_list_view(api_client_with_credentials, fc_arc, issue_with_arc):
     resp = api_client_with_credentials.get(reverse("api:arc-issue-list", kwargs={"pk": fc_arc.pk}))
+    # The fixture's .arcs.add()/.characters.add() calls now also bump this
+    # issue's own `modified` (see update_related_modified), via a raw
+    # .update() that doesn't touch the in-memory instance -- refresh before
+    # comparing against the API response.
+    issue_with_arc.refresh_from_db()
     serializer = IssueListSerializer(issue_with_arc)
     assert resp.data["count"] == 1
     assert resp.data["next"] is None