Skip to content

Fix response-cache key collisions on pagination and request host - #607

Merged
bpepple merged 3 commits into
Metron-Project:masterfrom
jyggen:caching-fixes
Aug 21, 2026
Merged

Fix response-cache key collisions on pagination and request host#607
bpepple merged 3 commits into
Metron-Project:masterfrom
jyggen:caching-fixes

Conversation

@jyggen

@jyggen jyggen commented Aug 21, 2026

Copy link
Copy Markdown
Member

Description

Fixes two response-cache key collisions found while auditing the recent Redis-backed API caching work (following on from #606's detail/action key collision fix):

  1. Pagination: issue_list (Arc/Character/Series/Team) is a paginated action, but its cache key was built purely from (cache_model_label, action, pk, modified, *dependent_labels), with no component for the request's query string. ?page=1 and ?page=2 computed the identical Redis key, so whichever page was requested first got served back for both — wrong results, wrong next/previous links — until the parent object's modified changed or the 24h TTL expired.

  2. Request host/scheme: every cached serializer embeds a resource_url built via request.build_absolute_uri(), but no cache key varied by the request's scheme/host. A response cached while served under one hostname (ALLOWED_HOSTS can list more than one — apex + www, an internal health-check host, etc.) had that hostname baked into resource_url for every other hostname, for up to DETAIL_CACHE_TTL/LIST_CACHE_TTL.

Checklist

  • Tests pass locally (pytest)
  • Linted (ruff check . --fix && ruff format .)
  • New/changed user-facing strings in templates, forms, and views are wrapped for translation ({% trans %}/{% blocktrans %} in templates, gettext/gettext_lazy in Python), and django-admin makemessages -a was run to update locale/*/LC_MESSAGES/django.po for existing languages
  • Migrations included, if models changed (python manage.py makemigrations)

@bpepple bpepple self-assigned this Aug 21, 2026
@bpepple bpepple added bug Something isn't working api An API bug/feature labels Aug 21, 2026

@bpepple bpepple left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@bpepple
bpepple merged commit 3ddfa0f into Metron-Project:master Aug 21, 2026
2 checks passed
bpepple added a commit that referenced this pull request Aug 21, 2026
rest_framework_condition's condition() decorator re-supplies `self` internally, expecting to wrap an unbound function. Passing an already-bound method (self._cached_retrieve, self._issue_list) double-supplied `self`, silently rebinding the `request` param to the ViewSet instance instead of the real DRF request -- the exact bug papered over with a self.request workaround in #607. Pass the unbound method (type(self).*) instead so `self` binds correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api An API bug/feature bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants