I agree entirely. The point of mixins is not that they are a particularly
healthy abstraction, just that they are one way of exposing the Django
permissions system, which is currently only done in full form (that is,
with is_active and is_superuser guards) in AbsractUser and User. If you
want to use Django permissions with AbstractBaseUser, you have to roll your
own, and/or depend on underscore-prefixed functions from
contrib.auth.models.
I would be +1 for a solution that exposed those permissions checking
functions so that they can be used with less boilerplate. In general I
think having an explicit contract for User models in particular contexts is
a good thing, better than hiding behind an opaque mixin. That is,
class MyUser(AbstractBaseUser):
is_active = BooleanField(...)
is_staff = BooleanField(...)
@property
def is_superuser(self):
return False
def has_perm(self, perm, obj=None):
return user_has_perm(self, perm, obj)
def has_perms(self, perm_list, obj=None):
return user_has_perms(self, perm_list, obj)
def has_module_perms(self, package_name)
return user_has_module_perms(self, package_name)
is arguably preferable to
class MyUser(AbstractBaseUser, PermissionsMixin):
pass
The advantages of the former is that it is more explicit, and perhaps more
newbie friendly, but it is currently impossible to do without duplicating
logic since user_has_perm etc. don't exist. The advantage of the latter is
that it involves considerably less boilerplate, but it freezes is_superuser
and is_staff as boolean fields on the ORM (though this could be removed
from the mixin if people prefer implementing these themselves as Florian
suggested).
The point isn't to go mixin-crazy, it's to find the best way to expose
permissions-checking (the most boilerplate-heavy of the requirements of an
admin-compatible user). The other contracts are probably better exposed as
tests in contrib.admin than as little piece-wise mixins.
Best,
Alex Ogier
On Thu, Nov 8, 2012 at 7:12 AM, Russell Keith-Magee <[email protected]
> wrote:
>
> Ok - so, I've been following this thread, and I should probably shed some
> light on the other side of the decision making process.
>
> I've got a history with Mixins. I was responsible for the final commit of
> Django's class-based views, which are very mixin heavy… and haven't been
> universally well received.
>
> There was a lot of very incoherent "this sucks" criticism (which -- as an
> aside -- is hideously demotivating). However, some did find a way to
> express their criticism constructively; one good example that explains the
> bigger problem is from one of Django's core team:
>
> http://lukeplant.me.uk/blog/posts/class-based-views-and-dry-ravioli/
>
> tl;dr - The root of the objection is that yes, introducing a stack of
> mixins decreases repetition - but at the cost of a greater learning curve.
> And as a result, it's harder for newcomers to get started. This isn't an
> inherently bad thing, but it *is* a tradeoff.
>
> So, I've been burned. While I still believe mixins have their place… I'm
> not rushing to go overboard again.
>
> Where does that leave swappable Users?
>
> Something like a PermissionMixin? I can certainly see a place for that.
> Django's permissions model is a very specific beast, and the current User
> model implements them in a way that could definitely be refactored for
> reuse. I can certainly see the value in splitting that out so that you
> don't have to repeat the same implementation, Truth be told, it's about 10
> lines of code to reimplement if you import the helper functions, but if
> someone were to work up a patch for this, I'd be inclined to add it.
>
> Something like AdminMixin? I'm not as convinced. The Admin User API
> interface is duck typing at it's best. Admin doesn't care *how* is_staff is
> implemented -- it just cares that the attribute is available. Splitting out
> a mixin class to make it easier to put an is_staff attribute on a model
> seems like a whole lot of unnecessary complexity
>
> Something like PersonalDataModelMixin? Definitely unconvinced on this one.
> Implementations of first_name and last_name aren't *that* hard, and
> flexibility to include or not include these attributes was one of the
> driving motivators behind swappable users in the first place.
>
> Now, ok - in the case of the latter two, you could argue that *any*
> repetition is bad repetition. However, there's repetition and there's
> repetition. Being forced to repeat hundreds of lines of complex business
> logic… sure - that's bad. Being forced to explicitly declare that your user
> model has a boolean flag to define the staff status of a user? I'm sorry,
> but that's really not an onerous burden, and the CBV experience has shown
> that it can be beneficial for this sort of thing to be clearly declared,
> not masked in a superclass or mixin.
>
> As for the argument that having the mixins makes it easier to build
> interoperable components? In this case, I don't buy that at all. Again,
> duck typing is one of Python's strengths. Interfaces are important, but
> they don't have to be strong interfaces. It's enough to say "you must have
> an attribute called X"; you don't have to force a base class with a
> particular implementation to make that happen.
>
> Regarding the specifics -- I'm inclined to agree with Florian on the scope
> of the PermissionsMixin -- it needs to be constrained in scope, otherwise
> you're just importing all of AbstractUser into the mixin. So, if someone
> wants to pull that together (or make an impassioned case the other way),
> feel free, and I'll see about getting it into trunk before the 1.5 beta
> freeze.
>
> Yours,
> Russ Magee %-)
>
>
> On Thu, Nov 8, 2012 at 6:21 PM, Ludwig Kraatz <[email protected]> wrote:
>
>> If you ask me this just points out to some point i mentioned in the
>> original Custom UserModel Thread. I'm trying to reframe it again.
>> I think the current django.contrib.auth *app* somehow behaves like some
>> mixture of
>> django.core._mixins_everyone_should_use_to_make_apps_interoperability and
>> contrib.auth
>>
>> As Ross pointed out in this original thread: it is usefull that everyone
>> uses the AbstractBaseUser - and i'm interpreting now - *because it would be
>> better for interoperability and security*
>> I think its possible and not bad designed at all to have this kinda stuff
>> as core material.
>>
>> So - most apps use authorization features as .*has_perm()* or .*
>> is_superuser*...
>>
>> If decoupling it into clear interfaces this could make custom development
>> much easier without loosing interoperability. i would suggest there
>> shouldn't be a default user attributes like .*is_superuser* because
>> thats very restrictive and nasty to workaround. And its clearly an
>> authorization and no authentication feature.
>>
>> So why not having it somehow like this? (pythonic pseudocode)
>>
>> core.mixins.authentication:
>>
>> *AuthModelMixin*:
>> -abstract-
>>
>> *UniqueIdentifier*
>>
>> *get_authentication_id*(): return UniqueIdentifier
>> *get_long_name*(): return UniqueIdentifier
>> *get_short_name*(): return UniqueIdentifier
>>
>> *DjangoAuthModelMixin*(AuthModelMixin):
>> -abstract-
>>
>> *UniqueIdentifier*
>> *password*
>> *last_login* #(for password reset token etc.)
>>
>> *check_password*() ..
>> *set_password*()...
>>
>>
>> core.mixins.personalyzation:
>>
>> *PersonalDataModelMixin*:
>> -abstract-
>> *first_name*
>> *last_name*
>>
>> *get_long_name*(): return first_name+last_name
>> *get_short_name*(): return first_name
>>
>>
>> core.mixins.authorization
>>
>> *PermissionModelMixin*:
>> IS_SUPERUSER = "not_as_an_user_attribute_necessarily__is_superuser"
>> IS_STAFF = "not_as_an_user_attribute_necessarily__is_staff"
>>
>> -abstractmethod-
>> *has_perm*():
>>
>> # or maybe even as property for backwards compatibility
>> *is_staff*():return False
>> *is_superuser*(): return False
>>
>>
>> *DjangoPermissionModelMixin*(PermissionMixin):
>> *has_perm*(permission): return self.is_superuser() or
>> self._check_for_permission(permission)
>> *_check_for_permission*(permission):
>> if permission == IS_STAFF:
>> return self.is_staff()
>> else:
>> super(django1.5a.contrib.auth.User,self).has_perm(permission)
>>
>>
>> and contrib.auth:
>>
>> *User*(Mixina,b,c,d):
>> *date_joined*
>>
>> .....
>>
>>
>> Didn't think abt every detail - just ment to point out what i would
>> suggest to possibly fit all needs.
>>
>> Best regards and sry for this *little* novel
>>
>> ludwig
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers" group.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msg/django-developers/-/36x8Ecpj9scJ.
>>
>> 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.
>>
>
> --
> 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.
>
--
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.
