Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#29253 closed Bug (fixed)

method_decorator() with list argument doesn't copy over attributes of the decorator function

Reported by: Chris Jerdonek Owned by: Chris Jerdonek
Component: Utilities Version: 2.0
Severity: Normal Keywords: decorators
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The docs say that the "list" form of @method_decorator is equivalent to invoking it multiple times:

decorators = [never_cache, login_required]

@method_decorator(decorators, name='dispatch')
class ProtectedView(TemplateView):
    template_name = 'secret.html'

@method_decorator(never_cache, name='dispatch')
@method_decorator(login_required, name='dispatch')
class ProtectedView(TemplateView):
    template_name = 'secret.html'

However, it appears there is a slight difference in behavior. For example:

from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_exempt

def my_decorator(func):
    def new_func(*args, **kwargs):
        func(*args, **kwargs)

    return new_func

@method_decorator(my_decorator, name='dispatch')
@method_decorator(csrf_exempt, name='dispatch')
class View1:

    def dispatch(self):
        pass

@method_decorator([my_decorator, csrf_exempt], name='dispatch')
class View2:

    def dispatch(self):
        pass

print(hasattr(View1.dispatch, 'csrf_exempt'))
print(hasattr(View2.dispatch, 'csrf_exempt'))

results in the output--

True
False

It appears this is because method_decorator() takes a short cut when processing a list. It doesn't carry over the attributes of the decorated function like it does in the normal case.

Change History (8)

comment:1 by Chris Jerdonek, 7 years ago

The behavior difference described in this ticket is actually noted in the discussion of the patch here, but it was thought not important. I do think it's important though because, for example, it can cause decorators to have or not have an effect depending on which invocation style is used.

comment:2 by Tim Graham, 7 years ago

Summary: method_decorator behaves differently with list argumentmethod_decorator() with list argument doesn't copy over attributes of the decorator function
Triage Stage: UnreviewedAccepted

comment:3 by Chris Jerdonek, 7 years ago

Owner: changed from nobody to Chris Jerdonek
Status: newassigned

comment:4 by Chris Jerdonek, 7 years ago

Has patch: set

I prepared a pull request here: https://github.com/django/django/pull/9819

comment:5 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In fdc936c:

Fixed #29253 -- Made method_decorator(list) copy attributes.

comment:6 by Tim Graham, 6 years ago

PR 10091 fixes a regression where @method_decorator(transaction.non_atomic_requests) crashes.

comment:7 by Tim Graham <timograham@…>, 6 years ago

In f434f5b:

Refs #29253 -- Fixed method_decorator() crash if decorator sets a new attribute.

Regression in fdc936c9130cf4fb5d59869674b9a31cc79a7999.

comment:8 by Tim Graham <timograham@…>, 6 years ago

In da46599:

[2.1.x] Refs #29253 -- Fixed method_decorator() crash if decorator sets a new attribute.

Regression in fdc936c9130cf4fb5d59869674b9a31cc79a7999.

Backport of f434f5b84f7fcea9a76a551621ecce70786e2899 from master

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