[Django] #26626: Update decorator_from_middleware to work with new-style middleware
Django
-----------------------------------------+------------------------
Reporter: timgraham | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
@carljm is working on it: "I did work on updating
`decorator_from_middleware` yesterday and have an in-progress patch. I
don't think we should plan on the patch for alpha (it's nontrivial and
will need review), but if it's ok to do it as a follow-up cleanup fix for
beta, then yeah I think it's be good to have it for 1.10.
--
Ticket URL: <https://code.djangoproject.com/ticket/26626>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: timgraham | Owner: carljm
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* status: new => assigned
* owner: nobody => carljm
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:1>
Django
Reporter: Tim Graham | Owner: Carl Meyer
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Absent a compelling use case for this update and due to some limitations
in how effectively `decorator_from_middleware` can make the
transformation, we decided not to move forward with this. A query about it
has been raised
[https://www.reddit.com/r/django/comments/5s052v/decorator_from_middleware_doesnt_work_with_the/
on Reddit]. I've asked if that poster can give a use case here.
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:2>
Django
Reporter: Tim Graham | Owner: Carl Meyer
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andreas Pelme):
A use case for this is tests. I have a middleware that handles temporary
unavailability by showing a loading page/returning HTTP 503 for certain
exceptions.
This particular middleware uses custom logic in `__call__` and
`process_exception`. `decorator_from_middleware` makes it easy to test
this middleware:
{{{
@override_settings(MAINTENANCE_MODE=True)
def test_maintenance_mode(self):
@decorator_from_middleware(TemporaryUnavailable)
def view():
raise AssertionError("don't call")
response = view(RequestFactory().get('/')
assert response.status_code == 503
}}}
Of course, I could craft my own get_response, initiate the middleware and
call `__call__` or `process_exception` directly but that is ugly and easy
to get some details wrong, especially when the test involves exceptions.
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:3>
Django
Reporter: Tim Graham | Owner: Carl Meyer
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andreas Pelme):
Another use case:
We have a custom authentication mechanism that is not tied to
contrib.admin for our main site.
However, we use contrib.auth for administrative accounts. To make it very
clear which views needs a real user, we have disabled the auth middleware
globally and use a custom admin site that selectively enables auth:
{{{
from django.contrib import admin
from django.contrib.auth.middleware import AuthenticationMiddleware
from django.utils.decorators import decorator_from_middleware
auth_decorator = decorator_from_middleware(AuthenticationMiddleware)
class AdminSite(admin.AdminSite):
def admin_view(self, view, cacheable=False):
super_wrapper = super().admin_view(view, cacheable=cacheable)
return auth_decorator(super_wrapper)
}}}
(This use case is fine and works fine in Django 1.10 since Django's built
in `AuthenticationMiddleware` is both old and new-style, but I just wanted
to highlight that `decorator_from_middleware` is useful in different
contexts)
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:4>
Django
Reporter: Tim Graham | Owner: Carl Meyer
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Svenson):
The following function takes a new-style middleware class and makes a view
decorator out of it. It's used in code that me and @pelme are working on,
though not as a decorator.
{{{
def decorator_from_middleware_new(new_middleware_cls):
def view_decorator(view_function):
def view(request, *args, **kwargs):
def get_response(request_inner):
assert request is request_inner
try:
return view_function(request, *args, **kwargs)
except Exception as e:
new_middleware.process_exception(request, e)
return HttpResponseServerError()
new_middleware = new_middleware_cls(get_response)
return new_middleware(request)
return view
return view_decorator
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:5>
Django
Reporter: Tim Graham | Owner: (none)
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Carl Meyer => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:6>
Django
Reporter: Tim Graham | Owner: (none)
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Johnson):
* cc: Adam Johnson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:7>
Django
Reporter: Tim Graham | Owner: (none)
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tanishq):
Hi all,
Just checking in—ticket #26626 appears to still be open, If it's still
relevant for the dev branch, I've prepared a fix based on the use cases in
comments 3–5.
The patch updates
{{{make_middleware_decorator}}} in {{{django/utils/decorators.py}}} to
support new-style middleware by passing a proper
{{{get_response}}} callable and invoking{{{__call__}}}. It refactors async
handling to avoid event loop conflicts (using new loops for sync contexts
and {{{await}}}in async ones), resolving un awaited coroutine warnings via
proper exception handling, while maintaining compatibility with old-style
middleware ({{{process_request}}}, {{{process_response}}}, etc.). Tests
were added/updated in {{{tests/utils_tests/test_decorators.py}}}
to cover sync/async{{{__call__}}}, {{{process_view}}},
{{{process_exception}}},and {{{process_template_response}}}, all passing
on Python 3.12. Verification was done using a reproduction script,
reproduce_26626.py, which replicates the issue and confirms the fix.
I’ll can submit a PR shortly with these changes and the reproduction
script for review. Happy to address any feedback, including on whether to
keep
{{{reproduce_26626.py}}}!
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:8>
Django
Reporter: Tim Graham | Owner: (none)
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tanishq):
* cc: Tanishq (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26626#comment:9>