On 3 February 2011 18:06, Jari Pennanen <[email protected]> wrote: > If everyone is happy with class decorators we should do it. But bear > in mind that removing the ability to override them practicly means > that only Final class should define them.
Which I think is actually a good thing from my experience, but I may be biased. > > That could lead to following annoying situations: Base classes could > e.g. enforce using some requirement for User.user_permission and > subclass could require using per object permission. What would happen > now? The programmer could not subclass from that class only because > the permissions checking is enforced in super class but otherwise a > good class, not exactly a good solution. Which is an argument against putting permission checks into generic reusable views, just like the current views don't use the TemplateMixin by default (well, they do, but they have a Base* duplicates). I'm not saying, that having a permission checking mixin is a bad idea, but I don't like the proposed API. For starters, you mentioned "object permissions" - how do I check that in your implementation, if all checkers are evaluated in dispatch() (and without fetching the object multiply times) ? > So I think the overriding ability in subclasses is a good to have. Ok, but I rather override some methods then a list. > > On Feb 3, 6:15 pm, Lukasz Rekucki <[email protected]> wrote: >> On 3 February 2011 16:09, Jari Pennanen <[email protected]> wrote: >> If they're not decorators, then that's even worse, 'cause you need to >> duplicate all existing permission checking decorators. > > In fact current decorator permission checking mechanism is wrong, it > does two things in one function: Checking permissions from request > (what I propose to separate) and being decorator at the same time. > They break the principle of one function one task, and should be > breaken apart to checker functions and decorators. Similarly as > validator functions are separated and reusable. It's actually a shortcut for user_passes_test() decorator factory and a very short lambda, so there is not much to reuse (apart from the redirection code maybe). > Making the attribute to *add only* is possible (but I don't support > that because of need to override I said above), by iterating over > classes (in __mro__*) and combining the checker functions to one list. Yes, I know it's possible and I even showed you that exact implementation. It's not exactly "add only" as you can always override `get_decorators()` to filter out or replace the list entirely. > However I find that would be odd behavior for class/instance > attribute. Also programmer should be able to remember all the checker > functions in all superclasses, and the overriding the earlier values > becomes impossible (or requires other way to do it). Without the add semantics, every time I want to add a check, I need to repeat all the checks in base classes, which is not very DRY if you ask me. If the checks come from a 3rd party app and it ever changes, I would need to go through all my code and update the lists. Maybe your mixin could provide a method to run the checks (so the view can decide when to invoke it) and some other methods which do the actual checks that can be overridden. Just a thought. -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
