Opened 9 years ago

Last modified 4 days ago

#26626 new New feature

Update decorator_from_middleware to work with new-style middleware

Reported by: Tim Graham Owned by:
Component: HTTP handling Version: dev
Severity: Normal Keywords:
Cc: Adam Johnson, Tanishq Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

@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.

Change History (9)

comment:1 by Carl Meyer, 9 years ago

Owner: changed from nobody to Carl Meyer
Status: newassigned

comment:2 by Tim Graham, 9 years ago

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 on Reddit. I've asked if that poster can give a use case here.

comment:3 by Andreas Pelme, 9 years ago

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.

comment:4 by Andreas Pelme, 9 years ago

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)

in reply to:  4 comment:5 by David Svenson, 8 years ago

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

comment:6 by Mariusz Felisiak, 4 years ago

Owner: Carl Meyer removed
Status: assignednew

comment:7 by Adam Johnson, 12 months ago

Cc: Adam Johnson added

comment:8 by Tanishq, 2 weeks ago

Hi Tim Graham,
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 awaitin 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!

Last edited 12 days ago by Tanishq (previous) (diff)

comment:9 by Tanishq, 4 days ago

Cc: Tanishq added
Note: See TracTickets for help on using tickets.
Back to Top