Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ class ReadingListItemsPagination(PageNumberPagination):
page_size = 50


def _mark_cache_status(response: Response, *, hit: bool) -> Response:
"""Tag a response with whether it came from the Redis response cache,
so cache behavior can be checked in production with `curl -I` instead
of inspecting Redis directly. Only called on the paths that actually
went through a cache.get()/cache.set() -- a viewset/action with caching
disabled (no cache_model_label) gets no header at all, rather than a
misleading MISS."""
response["X-Cache"] = "HIT" if hit else "MISS"
return response


class CachedObjectMixin:
#: Set on concrete viewsets to enable response caching; None disables it
#: (fail-open -- behaves exactly as before this attribute existed).
Expand Down Expand Up @@ -217,12 +228,12 @@ def _cached_retrieve(self, request, *args, **kwargs):
)
cached = cache.get(key)
if cached is not None:
return Response(cached)
return _mark_cache_status(Response(cached), hit=True)

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
return _mark_cache_status(response, hit=False)


class UserTrackingMixin:
Expand Down Expand Up @@ -254,12 +265,12 @@ def list(self, request, *args, **kwargs):
)
cached = cache.get(key)
if cached is not None:
return Response(cached)
return _mark_cache_status(Response(cached), hit=True)

response = super().list(request, *args, **kwargs)
if response.status_code == status.HTTP_200_OK:
cache.set(key, response.data, LIST_CACHE_TTL)
return response
return _mark_cache_status(response, hit=False)


class CachedDetailActionMixin(CachedObjectMixin):
Expand Down Expand Up @@ -287,7 +298,7 @@ def _cached_paginated_action(self, *, build_queryset, serializer_class):
)
cached = cache.get(key)
if cached is not None:
return Response(cached)
return _mark_cache_status(Response(cached), hit=True)

obj = self.get_object()
queryset = build_queryset(obj)
Expand All @@ -298,6 +309,7 @@ def _cached_paginated_action(self, *, build_queryset, serializer_class):
response = self.get_paginated_response(serializer.data)
if key is not None:
cache.set(key, response.data, DETAIL_CACHE_TTL)
response = _mark_cache_status(response, hit=False)
return response


Expand Down Expand Up @@ -666,7 +678,7 @@ def series_list(self, request, pk=None):
)
cached = cache.get(key)
if cached is not None:
return Response(cached)
return _mark_cache_status(Response(cached), hit=True)

queryset = (
publisher.series.select_related("series_type")
Expand All @@ -679,7 +691,7 @@ def series_list(self, request, pk=None):
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
return _mark_cache_status(response, hit=False)


class RoleViewset(mixins.ListModelMixin, viewsets.GenericViewSet):
Expand Down
57 changes: 57 additions & 0 deletions tests/comicsdb/test_api_response_caching.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,63 @@ def test_arc_issue_list_reflects_series_rename(
assert resp.json()["results"][0]["series"]["name"] == "Final Crisis Renamed"


def test_issue_retrieve_x_cache_header(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
assert resp["X-Cache"] == "MISS"

resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "HIT"


def test_arc_list_x_cache_header(api_client_with_credentials, wwh_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["X-Cache"] == "MISS"

resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "HIT"


def test_arc_issue_list_x_cache_header(
api_client_with_credentials, issue_with_arc, fc_arc, local_cache
):
url = reverse("api:arc-issue-list", kwargs={"pk": fc_arc.pk})
resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "MISS"

resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "HIT"


def test_publisher_series_list_x_cache_header(
api_client_with_credentials, dc_comics, fc_series, local_cache
):
url = reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk})
resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "MISS"

resp = api_client_with_credentials.get(url)
assert resp.status_code == status.HTTP_200_OK
assert resp["X-Cache"] == "HIT"


def test_uncached_viewset_gets_no_x_cache_header(api_client_with_credentials, local_cache):
"""PullListViewSet uses plain mixins.ListModelMixin, not
CachedListModelMixin -- confirms the header is only added on the paths
that actually go through the response cache, not stamped unconditionally."""
resp = api_client_with_credentials.get(reverse("api:pull_list-list"))
assert resp.status_code == status.HTTP_200_OK
assert "X-Cache" not in resp


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
Expand Down
Loading