Opened 3 years ago
Last modified 22 months ago
#33588 assigned Bug
@never_cache and @cache_page decorators are applied out of order for TemplateResponse.
Reported by: | Jacek Wojna | Owned by: | noneNote |
---|---|---|---|
Component: | Core (Cache system) | Version: | 4.0 |
Severity: | Normal | Keywords: | cache, never_cache, cache_page, TemplateResponse, make_middleware_decorator |
Cc: | Carlton Gibson | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The never_cache
decorator is a simple decorator, which applies add_never_cache_headers
on a response. On the other hand cache_page
decorator is based on CacheMiddleware
by using decorator_from_middleware_with_args
adapter. The impact of this issue is not applying cache_page
functionality to some of the Django views.
If both are used simultaneously, everything behaves fine for a regular django view, which returns HttpResponse
.
In the case of working with TemplateResponse
, the decorators are invoked in the correct order but applied out of order.
The difference between the responses is that the TemplateResponse
has a callable render
method and the decorator (based on CacheMiddleware) has a process_response
method.
Because of that, we register a new post_render_callback
here:
https://github.com/django/django/blob/fdf209eab8949ddc345aa0212b349c79fc6fdebb/django/utils/decorators.py#L145
# Defer running of process_response until after the template # has been rendered: if hasattr(middleware, "process_response"): def callback(response): return middleware.process_response(request, response) response.add_post_render_callback(callback)
In comparison with never_cache
, which is not based on middleware, we don't postpone applying the decorator, thus it's always applied before cache_page
regardless of their order.
It means, the following definitions are returning the same output, spite of the different decorators' order:
# I used here Wagtail's Page model as an example, because their views return `TemplateResponse` @method_decorator(never_cache, name="serve") @method_decorator(cache_page(settings.CACHE_TIME), name="serve") class SomePage(Page): ... # or @method_decorator(cache_page(settings.CACHE_TIME), name="serve") @method_decorator(never_cache, name="serve") class SomePage(Page): ...
This issue was originally posted on Wagtail project, but after figuring out what the actual issue is, I decided to post it here. For reference and described debugging process, please go to:
https://github.com/wagtail/wagtail/issues/7666
Workaround
To temporarily solve the issue, I decided to create a workaround by redefining what never_cache
is:
class NeverCacheMiddleware: def process_response(self, request, response): add_never_cache_headers(response) return response never_cache = decorator_from_middleware(NeverCacheMiddleware)
As you can see, it tries to mimic how cache_page
was implemented. Because of this it will also add_post_render_callback
and postpone applying add_never_cache_headers
.
Tests
views.py
from django.http import HttpResponse from django.template.response import TemplateResponse from django.views.decorators.cache import cache_page, never_cache @never_cache @cache_page(3600) def never_cache_first_http_response(request): return HttpResponse("never_cache_first_http_response") @cache_page(3600) @never_cache def cache_page_first_http_response(request): return HttpResponse("cache_page_first_http_response") @never_cache @cache_page(3600) def never_cache_first_template_response(request): return TemplateResponse(request, "app/template.html") @cache_page(3600) @never_cache def cache_page_first_template_response(request): return TemplateResponse(request, "app/template.html")
test_views.py
from django.test import Client, SimpleTestCase from freezegun import freeze_time from email.utils import parsedate_to_datetime @freeze_time("2022-01-14T10:00:00Z") class TestHttpResponse(SimpleTestCase): def test_different_expires_header_value(self): client = Client() first_never_cache = client.get("/never-cache-first-http-response") first_cache_page = client.get("/cache-page-first-http-response") assert first_never_cache.headers['Expires'] != first_cache_page.headers['Expires'] first_never_cache_datetime = parsedate_to_datetime(first_never_cache.headers['Expires']) first_cache_page_datetime = parsedate_to_datetime(first_cache_page.headers['Expires']) # Check that `cache_page` correcty sets the Expires header (to be +1 hour) assert (first_never_cache_datetime - first_cache_page_datetime).seconds == 3600 @freeze_time("2022-01-14T10:00:00Z") class TestTemplateResponse(SimpleTestCase): def test_different_expires_header_value(self): client = Client() first_never_cache = client.get("/never-cache-first-template-response") first_cache_page = client.get("/cache-page-first-template-response") # Fails, because headers have the same value assert first_never_cache.headers['Expires'] != first_cache_page.headers['Expires']
fails with:
E AssertionError: assert 'Fri, 14 Jan 2022 10:00:00 GMT' != 'Fri, 14 Jan 2022 10:00:00 GMT' app/test_views.py:28: AssertionError
Change History (5)
comment:1 by , 3 years ago
Version: | 3.2 → 4.0 |
---|
comment:2 by , 3 years ago
Cc: | added |
---|---|
Summary: | never_cache and cache_page applied out of order for TemplateResponse → @never_cache and @cache_page decorators are applied out of order for TemplateResponse. |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 22 months ago
Owner: | changed from | to
---|
comment:5 by , 22 months ago
@never_cache @cache_page(3600) def tem_view(request): r = random.randint(1000, 9999) return TemplateResponse(request, 'index.html', context=dict(value=r))
Here is what going on at 1st request,
never_cache -> CacheMiddleware.process_request[no cache found] -> response.add_post_render_callback(CacheMiddleware.process_response) -> add_never_cache_headers(response) -> after render calling CacheMiddleware.process_response but returning response without setting cache because of this,
if "private" in response.get("Cache-Control", ()): return response
Suppose we somehow set cache, Here, what going to happen on at 2nd request,
never_cache -> CacheMiddleware.process_request[cache found] -> add_never_cache_headers(response)
What is the problem:
1) never_cache, cache_control, last_modified setting the headers immediately while the cache is going to be set after render(), Thus cache-Control, and other headers are present at the time of setting the cache
2) there is no good way of detecting @decorator's orders and numbers, which means we can not detect which and how many decorator users applied before CacheMiddleware
3) deferring CacheMiddleware.process_response execution making whole @decorator's orders meaningless.
Here is what I suggest as, a solution:
We already have add_post_render_callback so how about we add an add_apply_last_callback to HttpResponse / HttpResponseBase,
apply_last_callback is going to execute just before we send the final response.
It is a multipurpose solution, a user also can use it from view to modify the response object at end of the chain
@last_modified(latest_entry, apply_last=True) @never_cache(apply_last=True) @cache_page(3600) def tem_view(request): r = random.randint(1000, 9999) return TemplateResponse(request, 'index.html', context=dict(value=r))
Please Let me Know, your views on this.
Thanks, sounds valid. Unfortunately other decorators are probably also affected e.g.
@cache_control
(see also #11479 about@last_modified
). As far as I'm aware, we could fix both tickets by adding a few hooks toFetchFromCacheMiddleware
andUpdateCacheMiddleware
and use them in the@cache_page
decorator instead of relying directly on theCacheMiddleware
.