Hi,
On Sep 16, 2011, at 12:11 AM, Łukasz Rekucki wrote:
Hi,
On 15 September 2011 22:44, Jacob Kaplan-Moss <ja...@jacobian.org>
wrote:
#14512 proposes a adding another view-decorator-factory for
decorating
class-based views, which would turn the above into::
@class_view_decorator(login_required)
class MyView(View):
...
This makes me less sad, but still sad. Factory functions. Ugh.
As the ticket creator I feel obligated to reply :)
Me (as the poster of the latest patch) too :)
Thinking about it now, it does look kinda ugly. It's mainly because I
was focus on how to reuse existing decorators in CBV context. It
shouldn't be to hard to make the "view_decorator" a meta-decorator
(or a factory function as you call it) that turns login_required to
something that is both a class and function decorator. Methods are
still out-of-bounds for non-runtime introspection, but after almost 1
year of using CBV, I never needed to decorate a method.
I would like to also comment on the new approach in that ticket.
Making a shallow copy of a class is *MAGIC* to me. It breaks the basic
invariant "issubsclass(decorator(A), A) == True". This is important if
you're planing to use this as "B = decorator()(A)" (and you are,
'cause the whole point of not modifying the original class is to allow
safely doing this), as you quickly end up with weird alternate
hierarchies. So, please don't do that :)
I agree that in an ideal world we'd have a better solution. On the
other hand, the cbv-decorator using inheritance just didn't work, and
the one using shallow copies does (if you don't use B=decorator()(A),
indeed).
As Donald mentioned, the more elegant solution for classes is to use
base classes (mixins) instead of class decorators. I'm not sure though
if every decorator can be replaced by a mixin, and if the order of
these mixins can be arbitrary in that case (which I believe is
desirable). Can anybody comment on that?
I believe, however, I've figured out a different technique to make
this work: don't try to detect bound versus unbound methods, but
instead look for the HttpRequest object. It'll either be args[0] if
the view's a function, or args[1] if the view's a method. This
technique won't work for any old decorator, but it *will* work (I
think) for any decorator *designed to be applied only to views*.
+1 on this. I would be a bit concerned about this vs. future
implementation of generator-based-views that allow some kind of
response streaming (is someone working on that?).
I've written a proof-of-concept patch to @login_required (well,
@user_passes_test, actually):
https://gist.github.com/1220375
Did I already mention creating a subclass in the class decorator
breaks super ;) [https://gist.github.com/643536]
Can I get some thoughts on this technique and some feedback on
whether
it's OK to apply to every decorator built into Django?
It would be great to have that meta-decorator, so everyone else could
upgrade their decorators just by decorating them.
If this doesn't exist, then the view_decorator still has a right to
live.
So what would have to be created is a meta-decorator
universal_decorator, replacing or using view_decorator, such that
universal_decorator(login_required) is the actual universal decorator.
This would be applied to all django-decorators, but I think it would
be good to also allow applying universal_decorator to an already
universal decorator, such that:
# after this...
login_required = universal_decorator(login_required)
# ... this will hold
assert login_required == universal_decorator(login_required)
--
You received this message because you are subscribed to the Google Groups "Django
developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
