Opened 10 years ago

Closed 9 years ago

#1840 closed defect (duplicate)

django's decorators don't assign necessary attributes to the wrapper

Reported by: russblau@… Owned by: nobody
Component: Core (Other) Version: master
Severity: minor Keywords:
Cc: gary.wilson@…, myer0052@…, ben@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

If a view function is wrapped with a Python decorator, the admin interface will show that decorator in place of the actual view (in admin/doc/views). For example:

/admin/doc/views/
View function: django.contrib.admin.views.decorators._checklogin

The actual view function is 'django.contrib.admin.views.doc.view_index', but it's wrapped with the staff_member_required() decorator, which in turn returns the _checklogin function object.

Attachments (3)

admin-doc-views.diff (2.7 KB) - added by dummy@… 10 years ago.
this maybe a solution
RaviKumar2.doc (54.5 KB) - added by anonymous 10 years ago.
admindocdecoratorfix.diff (1.0 KB) - added by dan.ferrante at gmail 10 years ago.
SVN patch for this ticket that changes decorator code to report the decorated function to admin.doc views

Download all attachments as: .zip

Change History (23)

comment:2 Changed 10 years ago by Adrian Holovaty

priority: normallow
Severity: normalminor

I'm not sure what we can do to fix this...We'd have to somehow peek into the function to see whether it was being decorated or not. Either that, or each view decorator would have to mark itself as a decorator somehow, and the admin view-doc system would have to keep an eye out for that special mark. Hmmmm. Any other ideas?

comment:3 Changed 10 years ago by Luke Plant

You could make the decorators set the name of the function they return to be the name of function they take:

def mydecorator(myfunc):
    def wrapper(*args, **kwargs):
        # do something here, then...
        return myfunc(*args, **kwargs)
    wrapper.__name__ = myfunc.__name__
    wrapper.__doc__ = myfunc.__doc__
    return wrapper

Then everything would be lovely. Of course, you'd have to do that on every decorator. Alternatively, you could use the module I posted a link to above, which does that automatically, and a lot more (including making 'wrapped' have the same function signature as 'myfunc', so you can introspect it nicely).

comment:4 Changed 10 years ago by Luke Plant

Ah, except that it doesn't work. If you include module as well, then you get the whole name correct, but for some reason the documentation doesn't show up. And the 'decorator' module I linked seems to work even less (I'm sure I found that module from a reputable source like Ian Bicking or someone). I'll investigate it a bit.

comment:5 Changed 10 years ago by Luke Plant

OK, the first part of my last comment was nonsense -- the method described does work (I was getting mixed up between difference view functions).

As for using the 'decorator' module, there is a very easy one line fix to apply (he just forgot to do the wrapper.__module__ = myfunc.__module__ bit for some reason), then it works for the documentation. However, I'm getting bizarro messages when actually trying to *use* the views.

comment:6 Changed 10 years ago by mattimustang@…

This is how i'm doing it in one of my decorators that adds extra context to a generic view:

class GenericViewContextAdder(object):    
    """
    Add extra context to a generic view function.
    Example usage:
        from django.views.generic import list_detail
        from nong.common.views import GenericViewContextAdder
        foo_list = GenericViewContextAdder(list_detail.object_list, 'title', bar=lambda original: mymodels.get_object(name__exact=original))
    """
    def __init__(self, method_to_wrap, *arguments_to_steal,
        **arguments_to_transform):
        self.method_to_wrap = method_to_wrap
        self.arguments_to_steal = arguments_to_steal
        self.arguments_to_transform = arguments_to_transform
        doc = ""
        for l in self.method_to_wrap.__doc__.splitlines():
            doc += l.replace('    ', '', 1)
            doc += '\n'
        doc = doc.replace('Context:', '\nContext:\n\n', 1)
        doc = doc.replace('Templates:', '\nTemplates:\n\n', 1)
        self.method_to_wrap.__doc__ = 'This view is a wrapper around the ' \
            + doc
        self.method_to_wrap.__doc__ += '\nExtra context:\n\n'
        for stolen_argument in self.arguments_to_steal:
            self.method_to_wrap.__doc__ += '\n\t%s\n' % stolen_argument
        for stolen_argument, transformation in self.arguments_to_transform:
            self.method_to_wrap.__doc__ += '\n\t%s=%s\n' % \
                (stolen_argument, transformation)

    def __call__(self, *args, **kwargs):
        extra_context = {}
        for stolen_argument in self.arguments_to_steal:
            extra_context[stolen_argument] = kwargs.pop(stolen_argument)
        for stolen_argument, transformation in self.arguments_to_transform:
            original_value = kwargs.pop(stolen_argument)
            transformed_value = transformation(original_value)
            extra_context[stolen_argument] = transformed_value
        kwargs.setdefault('extra_context', {}).update(extra_context)
        return self.method_to_wrap(*args, **kwargs)

    @property
    def __name__(self):
        return self.method_to_wrap.__name__

    @property
    def __module__(self):
        return self.method_to_wrap.__module__

This works for me.

comment:7 Changed 10 years ago by Adrian Holovaty

Luke -- which bizarro messages are you getting when you try to use the views? I've added the __doc__, __name__ and __module__ assignments in my local copy of django.utils.decorators.decorator_from_middleware, and the problem is fixed with no side effects.

comment:8 Changed 10 years ago by Luke Plant

Adrian -- the __doc__, __name__ and __module__ assignments should be fine. I was using the 'decorator' module, which does that for you and also creates 'signature preserving' decorators, so you can use getargspec etc on them. However, I've just discovered that I was using it incorrectly, and using it correctly works very well.

The advantages of using the module are:

  • You don't have to repeat the __doc__, __name__ etc assignments (although I guess you could put them in a utility function)
  • You preserve the signature of the original functions, so you can introspect them.

The disadvantages are:

  • Adding more code to Django (120 lines, which includes doctests)
  • The changes you have to make are not quite as trivial as I thought before -- I thought it was just adding staff_member_required = decorator(staff_member_required . In reality, you have to change the decorator function like this:
def staff_member_required(view_func):
    def _wrapped(request, *args **kwargs):
        # Do stuff with request
        return view_func(request, *args, **kwargs)

to

@decorator.decorator
def staff_member_required(view_func, request, *args, **kwargs):
    # Do stuff with request
    return view_func(request, *args, **kwargs)

The transformation is very straightforward in each case -- write a 'caller' (which takes a function and the arguments to pass to it) instead of a typical decorator, and then use 'decorator' to make it into a very well-behaved decorator. In some sense it is a simplification -- you have a simple higher order function instead of higher order function plus closure. On the other hand, it is a slight obfuscation, since staff_member_required() looks like it takes several arguments, but it ends up as a callable that takes a single argument.

Overall I think I'm in favour of using it -- it would also solve the problem that Gary Wilson just mentioned in django-devs about not being able to use decorators with filters.

comment:9 Changed 10 years ago by hi-world cup

Cc: hi-world cup added
Keywords: rthml tab space editor js added
Summary: admin/doc/views shows decorator instead of actual view functionhi-world cup

comment:10 Changed 10 years ago by Adrian Holovaty

Cc: hi-world cup removed
Summary: hi-world cupadmin/doc/views shows decorator instead of actual view function

comment:11 Changed 10 years ago by anonymous

Summary: admin/doc/views shows decorator instead of actual view function[patch] admin/doc/views shows decorator instead of actual view function
Version: SVN

Changed 10 years ago by dummy@…

Attachment: admin-doc-views.diff added

this maybe a solution

comment:12 Changed 10 years ago by Gary Wilson <gary.wilson@…>

We could require python2.5 and use the update_wrapper and wraps functions of the new functools module :)

Or, if we don't want to require 2.5, try to import the function(s) from the functools module, and if that fails then use a fake function/decorator that does nothing. Only people using 2.5 would see the benefit, but on the bright side it would require less code and would be the future.

comment:13 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Cc: gary.wilson@… added
Component: Admin interfaceCore framework
Keywords: rthml tab space editor js removed
Summary: [patch] admin/doc/views shows decorator instead of actual view functiondjango's decorators don't assign necessary attributes to the wrapper

There are more decorators than just in the admin interface. Also removing patch from title since it's invalid.

comment:14 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Here is the description of the update_wrapper function:

Update a wrapper function to look like the wrapped function. The optional arguments are tuples to specify which attributes of the original function are assigned directly to the matching attributes on the wrapper function and which attributes of the wrapper function are updated with the corresponding attributes from the original function. The default values for these arguments are the module level constants WRAPPER_ASSIGNMENTS (which assigns to the wrapper function's name, module and documentation string) and WRAPPER_UPDATES (which updates the wrapper function's instance dictionary).

The main intended use for this function is in decorator functions which wrap the decorated function and return the wrapper. If the wrapper function is not updated, the metadata of the returned function will reflect the wrapper definition rather than the original function definition, which is typically less than helpful.

Changed 10 years ago by anonymous

Attachment: RaviKumar2.doc added

comment:15 Changed 10 years ago by Robert Myers <myer0052@…>

Cc: myer0052@… added

comment:16 Changed 10 years ago by Michael Radziej <mir@…>

Triage Stage: UnreviewedDesign decision needed

comment:17 Changed 10 years ago by dan.ferrante at gmail

Has patch: set
Needs tests: set

Hi. I made a really simple patch that is horribly simple and works fine AFAIK. All it does is add the following code, within the decorator functions in django.contrib.auth.decorators and django.contrib.admin.decorators:

_checklogin.__name__ = view_func.__name__
_checklogin.__module__ = view_func.__module__

This provides the appropriate variables to the admin.view_index view to make everything work swimmingly, and shouldn't mess with anything else.

Changed 10 years ago by dan.ferrante at gmail

Attachment: admindocdecoratorfix.diff added

SVN patch for this ticket that changes decorator code to report the decorated function to admin.doc views

comment:18 in reply to:  17 Changed 10 years ago by James Bennett

Replying to dan.ferrante at gmail:

__name__ is read-only in Python 2.3, so this would break compatibility.

comment:19 Changed 9 years ago by anonymous

Cc: ben@… added

comment:20 Changed 9 years ago by Gary Wilson

Resolution: duplicate
Status: newclosed

Marking this a dup of #5701 since we have a patch there.

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