Task #11732 (closed)
@login_required __name__
Reported by: | wmoore | Owned by: | drussell-x |
---|---|---|---|
Priority: | major | Milestone: | 5.0.0-rc1 |
Component: | Web | Version: | n.a. |
Keywords: | n.a. | Cc: | cxallan, atarkowska, jamoore, cneves, douglas.russell@… |
Resources: | n.a. | Referenced By: | n.a. |
References: | n.a. | Remaining Time: | n.a. |
Sprint: | n.a. |
Description
If you want to use the class-based login_required decorator to decorate class-based views using Django's @method_decorator() https://docs.djangoproject.com/en/1.5/topics/class-based-views/intro/#decorating-the-class
like
@method_decorator(login_required()) def dispatch(self, *args, **kwargs):
Then this fails with:
Traceback: File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/core/handlers/base.py" in get_response 101. request.path_info) File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/core/urlresolvers.py" in resolve 252. sub_match = pattern.resolve(new_path) File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/core/urlresolvers.py" in resolve 250. for pattern in self.url_patterns: File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/core/urlresolvers.py" in _get_url_patterns 279. patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module) File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/core/urlresolvers.py" in _get_urlconf_module 274. self._urlconf_module = import_module(self.urlconf_name) File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/utils/importlib.py" in import_module 35. __import__(name) File "/Users/dpwrussell/Checkout/openmicroscopy/components/tools/OmeroWeb/omeroweb/../omeroweb/webadmin/urls.py" in <module> 31. from omeroweb.webadmin import views File "/Users/dpwrussell/Checkout/openmicroscopy/components/tools/OmeroWeb/omeroweb/../omeroweb/webadmin/views.py" in <module> 821. class AnnounceView(FormView): File "/Users/dpwrussell/Checkout/openmicroscopy/components/tools/OmeroWeb/omeroweb/../omeroweb/webadmin/views.py" in AnnounceView 845. @method_decorator(login_required()) File "/Users/dpwrussell/Checkout/openmicroscopy/dist/lib/python/django/utils/decorators.py" in method_decorator 40. update_wrapper(_dec, decorator) File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/functools.py" in update_wrapper 33. setattr(wrapper, attr, getattr(wrapped, attr)) Exception Type: AttributeError at /webadmin/announce/ Exception Value: 'login_required' object has no attribute '__name__'
But works with
lr = login_required() lr.__name__ = "login_required" @method_decorator(lr) def dispatch(self, *args, **kwargs):
So, we simply need to add the name attribute to the login_required class.
Change History (10)
comment:1 Changed 10 years ago by wmoore
comment:2 Changed 10 years ago by drussell-x
I couldn't understand why this error wasn't being hit by anyone else, and I think I've figured out why. Initially we were looking at why the function returned by login_required did not have the expected name method. It does. The problem is that the login_required is initialised, but not called at that point.
When wrapping the object, the functools update_wrapper is trying to copy all the important stuff from the decorator it is passed. Which is login_required(). When this decorator is used, it will execute call and correctly return the true wrapper. But when constructing the class, the method_decorator will do this:
https://github.com/django/django/blob/master/django/utils/decorators.py#L39
which will be looking at login_required, not the method defined when called. Thus it is missing name.
Test cases here: https://gist.github.com/dpwrussell/7573003 (test1 is non-working, test2 has a bodge to make it work)
Still experimenting with the best way to solve this.
comment:3 Changed 10 years ago by wmoore
Hi Douglas,
After playing with your test2 a little bit, I think that your 'bodge' is the only real way to solve it. The reason that the other examples you've seen are working OK is because the class itself, that you're passing to method_decorator() does have a name:
MyDecorator.__name__
but in our case, the instance of the class doesn't (by default) have a name
MyDecorator().__name__
so we have to provide one.
In neither case does the name match the name of the function we're actually wrapping - so I don't think it matters too much.
Using update_wrapper(wrapped, f) as you've done seems like a good idea too, although it's not related to the error we're seeing.
I would just go with your solution in test2, with the name matching the class name, E.g. "MyDecorator?" or "login_required". Works fine for me.
comment:4 Changed 10 years ago by drussell-x
Alternatively, try this:
https://github.com/dpwrussell/openmicroscopy/tree/admin_utils
I haven't replace login_required, I've added login_required_dpwr so both can be used side by side to test this out. I tried it on my CBV and on a function, seems to work fine in both cases as either:
@login_required_dpwr def load_drivespace(request, conn=None, **kwargs):
or
class AnnounceView(FormView): # [...] @method_decorator(login_required_dpwr) def dispatch(self, *args, **kwargs): return super(AnnounceView, self).dispatch(*args, **kwargs)
I can't see how it is functionality any different from what is happening at the moment, except that it works with @method_decorator and simplifies the login_required decorator a bit.
comment:5 Changed 10 years ago by wmoore
With the alternative approach above, you can't configure the decorator differently for each view method.
Your init is
def __init__(self, func, useragent='OMERO.web', isAdmin=False, isGroupOwner=False, doConnectionCleanup=True, omero_group='-1', allowPublic=None):
but how are you going to specify the optional parameters, E.g. isAdmin = True
E.g. in webclient views.py we have:
@login_required(setGroupContext=True) @render_response() def load_data(....): .... @login_required(doConnectionCleanup=False) def get_original_file(request, fileId, conn=None, **kwargs): .... @login_required(ignore_login_fail=True) def keepalive_ping(request, conn=None, **kwargs): .... @login_required(login_redirect='webindex') def logout(request, conn=None, **kwargs): ....
comment:6 Changed 10 years ago by drussell-x
Ok, I finally get it. I hadn't recognised that decorators with arguments work in a fundamentally different way than ones without. This post is an excellent explanation: http://www.artima.com/weblogs/viewpost.jsp?thread=240845
I think my initial bodge is the way to go for now, but I will take this up with django as I think the method_decorator should be able to handle this case automatically.
comment:7 Changed 10 years ago by drussell-x
Bug is already opened: https://code.djangoproject.com/ticket/13879
comment:8 Changed 10 years ago by drussell-x
- Owner changed from wmoore to drussell-x
- Status changed from new to accepted
comment:9 Changed 10 years ago by drussell-x
- Resolution set to fixed
- Status changed from accepted to closed
comment:10 Changed 10 years ago by drussell-x
Closing in favour of: https://github.com/openmicroscopy/openmicroscopy/pull/1820
From Douglas