Opened 3 weeks ago
Last modified 13 hours ago
#37065 assigned Bug
@method_decorator can't be used on async view at class-level with name="dispatch"
| Reported by: | Jacob Walls | Owned by: | Vishy Algo |
|---|---|---|---|
| Component: | Utilities | Version: | 5.2 |
| Severity: | Normal | Keywords: | async |
| Cc: | Carlton Gibson | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
@method_decorator(..., name="dispatch") doesn't work for async views that don't override dispatch(), because dispatch() is sync, which is enough to fool the logic in #35083.
-
tests/decorators/test_cache.py
diff --git a/tests/decorators/test_cache.py b/tests/decorators/test_cache.py index 1aca6967e0..c3b1668a7c 100644
a b 1 from http import HTTPStatus 1 2 from inspect import iscoroutinefunction 2 3 from unittest import mock 3 4 … … from django.http import HttpRequest, HttpResponse 5 6 from django.test import SimpleTestCase 6 7 from django.utils.decorators import method_decorator 7 8 from django.views.decorators.cache import cache_control, cache_page, never_cache 9 from django.views.generic import View 8 10 9 11 10 12 class HttpRequestProxy: … … class NeverCacheDecoratorTest(SimpleTestCase): 217 219 with self.assertRaisesMessage(TypeError, msg): 218 220 await MyClass().async_view(HttpRequestProxy(request)) 219 221 222 async def test_never_cache_method_decorator_http_request_async_view(self): 223 @method_decorator(never_cache, name="dispatch") 224 class MyClass(View): 225 async def get(self, request): 226 return HttpResponse() 227 228 request = HttpRequest() 229 request.method = "GET" 230 response = await MyClass().dispatch(request) 231 self.assertEqual(response.status_code, HTTPStatus.OK) 232 220 233 def test_never_cache_decorator_http_request_proxy(self): 221 234 class MyClass: 222 235 @method_decorator(never_cache)
ERROR: test_never_cache_method_decorator_http_request_async_view (decorators.test_cache.NeverCacheDecoratorTest.test_never_cache_method_decorator_http_request_async_view) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/jwalls/my314/lib/python3.14/site-packages/asgiref/sync.py", line 325, in __call__ return call_result.result() ~~~~~~~~~~~~~~~~~~^^ File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/concurrent/futures/_base.py", line 443, in result return self.__get_result() ~~~~~~~~~~~~~~~~~^^ File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/concurrent/futures/_base.py", line 395, in __get_result raise self._exception File "/Users/jwalls/my314/lib/python3.14/site-packages/asgiref/sync.py", line 365, in main_wrap result = await awaitable ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/tests/decorators/test_cache.py", line 230, in test_never_cache_method_decorator_http_request_async_view response = await MyClass().dispatch(request) ~~~~~~~~~~~~~~~~~~^^^^^^^^^ File "/Users/jwalls/django/django/utils/decorators.py", line 47, in _wrapper return bound_method(*args, **kwargs) File "/Users/jwalls/django/django/views/decorators/cache.py", line 80, in _view_wrapper add_never_cache_headers(response) ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/jwalls/django/django/utils/cache.py", line 294, in add_never_cache_headers patch_response_headers(response, cache_timeout=-1) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/utils/cache.py", line 285, in patch_response_headers if not response.has_header("Expires"): ^^^^^^^^^^^^^^^^^^^ AttributeError: 'coroutine' object has no attribute 'has_header'
Moving the decorator to the method instead of the class works. We probably will prefer a test inside the test class added in #35083 rather than merging my sketched test.
Change History (9)
comment:1 by , 3 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:2 by , 3 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 6 days ago
| Has patch: | set |
|---|
comment:4 by , 5 days ago
| Patch needs improvement: | set |
|---|
comment:5 by , 22 hours ago
comment:6 by , 17 hours ago
| Cc: | added |
|---|
comment:7 by , 16 hours ago
This is a doozy. I'm not convinced it's tractable... (at reasonable cost/complexity to method_decorator).
If the method being decorated is not async then it's not 100% clear to me that method_decorator should do any further introspection to determine if the return value should be marked as a coroutine.
We're asking it to decorate a method, and #35083 was about allowing those methods to be async. Both conjuncts work.
Moving the decorator to the method instead of the class works.
A further know significant details about the particulars of View strikes me as out of scope for method_decorator.
If I had to decorate dispatch for an async view class, I think this is probably the best option:
@method_decorator(never_cache, name="dispatch")
class MyClass(View):
async def dispatch(self, *args, **kwargs):
return super().dispatch(*args, **kwargs)
async def get(self, request):
return HttpResponse()
(Not tested. You get the idea)
This somewhat mitigates using the name="dispatch" — so I'd probably put it on the method directly, and I'd only need to do that if we also had say async def post as well.
But I think trying to automagically our way forward here is mistake. It's complex. It just is.
So I think a docs tweak is probably the way to go.
comment:8 by , 14 hours ago
I completely agree, inspecting the return value and patching feels more like over-engineering than necessary.
But would async get dispatch cause any issues, if handled incorrectly! Cause, when just overriding dispatch (without http methods) also seems to be a mistake.
Probably as_view should also be overridden (again when all http methods are sync, view_is_async is False).
comment:9 by , 13 hours ago
It makes no sense to override dispatch to be async except in this kind of case.
Hi Jacob,
Please take a look at the PR and let me know whether patching
Viewwith__init_subclass__would be an appropriate fix, or if it would be considered out of scope for this ticket.