Opened 9 years ago

Closed 7 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@… 9 years ago.
this maybe a solution
RaviKumar2.doc (54.5 KB) - added by anonymous 8 years ago.
admindocdecoratorfix.diff (1.0 KB) - added by dan.ferrante at gmail 8 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 9 years ago by adrian

  • priority changed from normal to low
  • Severity changed from normal to minor

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 9 years ago by lukeplant

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 9 years ago by lukeplant

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 9 years ago by lukeplant

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 9 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 9 years ago by adrian

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 9 years ago by lukeplant

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 9 years ago by hi-world cup

  • Cc hi-world cup added
  • Keywords rthml tab space editor js added
  • Summary changed from admin/doc/views shows decorator instead of actual view function to hi-world cup

comment:10 Changed 9 years ago by adrian

  • Cc hi-world cup removed
  • Summary changed from hi-world cup to admin/doc/views shows decorator instead of actual view function

comment:11 Changed 9 years ago by anonymous

  • Summary changed from admin/doc/views shows decorator instead of actual view function to [patch] admin/doc/views shows decorator instead of actual view function
  • Version set to SVN

Changed 9 years ago by dummy@…

this maybe a solution

comment:12 Changed 9 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 9 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added
  • Component changed from Admin interface to Core framework
  • Keywords rthml tab space editor js removed
  • Summary changed from [patch] admin/doc/views shows decorator instead of actual view function to django'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 9 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 8 years ago by anonymous

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

  • Cc myer0052@… added

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

  • Triage Stage changed from Unreviewed to Design decision needed

comment:17 follow-up: Changed 8 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 8 years ago by dan.ferrante at gmail

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 8 years ago by ubernostrum

Replying to dan.ferrante at gmail:

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

comment:19 Changed 8 years ago by anonymous

  • Cc ben@… added

comment:20 Changed 7 years ago by gwilson

  • Resolution set to duplicate
  • Status changed from new to closed

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

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