Re: RFC: "universal" view decorators

[gavinwahl]

Attempting to make decorators anticipate every usage is the wrong approach. 
In Django, the sole interface to a view is as a callable that accepts an 
`HttpRequest` and turns it into an `HttpResponse` or exception. Django the 
framework doesn't actually need to support class-based views--because CBVs 
are just another way to build view callables.

View callables are an elegant abstraction that keeps the core of Django, 
and the decorators it provides, simple. Trying to anticipate all the ways 
of building view callables is not only complex, it is impossible. Why 
shouldn't I be able to use the built-in decorators with my own 
implementations of class-based views? If the built-in decorators' 
implementations are simple, I will be able to provide an adapter to use 
them with my CBV system. There are many useful decorators built-in to 
Django and in the wild, and my adapter will work with all of them, because 
they treat views as simple callables.

`method_decorator` is the right idea, but it is painful to use. It requires 
a tedious empty `dispatch` method, just to have something to decorate. 
Using decorators to modify views makes sense, because decoration is the 
only way to modify the functionality of a callable. Classes, however, have 
much more powerful ways of adding optional functionality: multiple 
inheritance.


# `DecoratorMixin`

`DecoratorMixin` is a class factory that converts any view decorator into a 
class-based view mixin. It works as you would expect with inheritance--MRO 
provides the same ordering that function decorators do, and multiple 
inheritance allows the mixins to be composed naturally. Inheriting from a 
decorated view decorates the subclass as you would expect. It averts the 
need for the annoying empty `dispatch` method in each class that wants to 
use a decorator.

Its implementation is simple: https://gist.github.com/gavinwahl/5694349. 
This is a complete implementation, and is all that's necessary to use view 
decorators with Django's CBVs. I use this code in all my projects with 
great success. [Here is an 
example](https://github.com/fusionbox/django-authtools/blob/master/authtools/views.py#L94)
 
of its use in a reimplementation of Django's auth views as CBVs.

Using it is easy and intuitive:

LoginRequiredMixin = DecoratorMixin(login_required)
 
class LoginRequiredAndCsrfMixin(DecoratorMixin(login_required),
DecoratorMixin(csrf_protect)):
"""
Inheriting from this class is the same as decorating your view 
thus::

@login_required
@csrf_protect
def someview(request):
pass
"""
pass

class SomeView(LoginRequiredMixin, View):
# ...
   
   
I propose `DecoratorMixin` be added to Django. It retains compatibility 
with all existing decorators, elegantly adapts functional decorators to 
good object-oriented practices, and has a simple implementation. 
'Universal' decorators don't accommodate existing third-party code, contain 
complex magic, and annul one of the most elegant principles of Django: 
views are simply callables.


On Thursday, September 15, 2011 2:44:39 PM UTC-6, Jacob Kaplan-Moss wrote:
>
> Hi folks --
>
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
>
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
>
> class MyView(View):
> @method_decorator(login_required)
> def dispatch(self, *args, **kwargs):
> return super(MyView, self.dispatch(*args, **kwargs)
>
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
>
> #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.
>
> I want @login_required to work for anything::
>
> @login_required
> class MyView(View):
> ...
>
> class Something(object):
> @login_required
> def my

Username Independence in Auth Forms

[gavinwahl]

Some of the built-in auth forms only work on user models whose 
`USERNAME_FIELD` is `username`. It is possible to remove this constraint 
and allow them work on any user model. [django-authtools][1] demonstrates 
this possibility.

The two forms in question are `UserCreationForm` and `UserChangeForm`. 
 Both of them explicitly add a `username` field instead of letting the 
ModelForm add it automatically. For `UserChangeForm`, simply removing the 
`username` field from the form achieves user-model independence. 
`UserCreationForm` is slightly more complicated, due to the 
`clean_username` method. `clean_*` methods have to be named after their 
field, so it's hard to add one when you don't know the name of the field. 
This can be overcome by adding a validator to the field while initializing 
the form [2].

The reason the forms do have a `username` field currently is to change the 
help text, validation error messages, and validators. I don't think this 
should happen in the form, because all of these can be set on the model 
field instead. This could cause a backwards-compatibility concern if 
someone wasn't validating usernames in their custom User model (they are 
already validated in `auth.User`), and relied on the form instead. I don't 
think this is a serious issue--it only occurs if someone is using a custom 
User model, using the built-in forms, and not doing any username validation 
in their model.

If this approach sounds reasonable, I will submit it in the form of a patch.

[1]: 
https://github.com/fusionbox/django-authtools/blob/master/authtools/forms.py
[2]: 
https://github.com/fusionbox/django-authtools/blob/master/authtools/forms.py#L61

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: AbstractUser to get more abstract

[gavinwahl]

> Note that EmailUser *doesn't* have a Meta: swappable definition. There is 
nothing on this model that says "I am swappable", or "I am an appropriate 
substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set 
swappable on its replacement user, too[1]. The two definitions look like 
this:

class User(Model):
username = models.CharField()
USERNAME_FIELD = 'username'
class Meta:
swappable = 'AUTH_USER_MODEL'

class EmailUser(Model):
email = models.CharField()
USERNAME_FIELD = 'email'
class Meta:
   swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to 
the database? 

The existing swappable mechanism takes care of it. Only the model 
referenced by settings.AUTH_USER_MODEL will be installed, not both. 

> we still have a problem -- We can't just say 
"contrib.auth.forms.AuthenticationForm", because this form needs to change 
depending on the model that is in use. We have similar problems with admin, 
and potentially with views and URLs.

It's straightforward to make the forms, views, and URLs work without 
checking what user model is installed, and this is the approach authtools 
takes. We can agree that code like `if settings.AUTH_USER_MODEL == 
auth.EmailUser"` is absolutely awful, so it's nice that it's not required 
to implement forms that work with custom user models. The forms from 
authtools will work with any user model, not just authtools.User and 
auth.User. It doesn't use any ugly switches on the type of the installed 
user model to do it either. (Note: the views and URLs don't actually have 
to change to accommodate EmailUser. authtools ships with the CBV auth views 
simply because #17209[2] has stalled.)

[1]: 
https://github.com/fusionbox/django-authtools/blob/master/authtools/models.py#L86
[2]: https://code.djangoproject.com/ticket/17209

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

Re: AbstractUser to get more abstract

[gavinwahl]

> The intention was to mark a particular model as a something that can be 
replaced.

It's hard to find the original rationale behind swappable, but my mental 
model was always:
  
  A model with `swappable = 'x'` means that the model should only be 
loading if settings.x is set to that same model

> it's still not desirable.

Why not? Swappable seems like exactly what we need here.

> No other User model needs to [set swappable]

This would still be the case. Only models that want to conditionally load 
themselves would set swappable. User models in application code probably 
wouldn't set it, because the project will always use that one user model.

> ... made a special case of "Swappable models in the same app".

I'm not sure where there's a special case. Swappable works for this without 
any modifications, see authtools.

> *Any* model can be swapped in as a substitute user. 

Yep. Nothing needs to change to keep this possible.

> If we were to go down this path, the logical extension (to my mind) would 
be to validate that you're not swapping in a model that hasn't declared 
itself as swappable

Why would you want to validate this? Swappable only controls loading of the 
model its set on, there is no action-at-a-distance on other models.

> Secondly, marking a model as swappable doesn't make it zero cost. 

That's fair. I'll add it to the wiki as a disadvantage.

> So far, the only convincing argument I've heard for putting the EmailUser 
in contrib.auth is that it avoids having to add a value to INSTALLED_APPS.

I don't think we should focus so much on the optional part of the proposal. 
The really important part is to refactor the existing code to work better 
with custom users. Once this happens, projects like authtools can trivially 
add an EmailUser in a few lines of code, so it's not so important that 
there's one in core.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Dynamic AUTH_USER_MODEL based on modules or routes

[gavinwahl]

> The current User model is great for admin panel users but in frontend it 
may become extra overhead for some cases. People try to create another user 
attribute models and use extra joins to be able to keep extra attributes 
(city, ip, locale etc.) for their users.

Use the user-profile 
patternto
 attach extra data to users. Your different user types can use different 
profiles. 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/53a986d8-3b81-40bf-a097-4bce03aa03b1%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: New middleware method for "universal" decoration

[gavinwahl]

This topic was also discussed during the deprecation of 
TransactionMiddleware and the introduction of ATOMIC_REQUESTS. The existing 
middleware semantics can't guarantee that  __exit__ (during 
process_response) will get called no matter what, necessitating the setting 
that invokes BaseHandler.make_view_atomic. make_view_atomic implements 
basically what you're suggesting, for one specific case (DB/ORM code in the 
front controller, ick!).

I can't find the previous discussion, does anyone have a link?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ac2c22dd-258c-4e68-a63c-e947f229176b%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Making assertNumQueries more useful by printing the list of queries executed on failure

[gavinwahl]

Every time I get a failure on one of these types of tests I go in and edit 
the code to print out the queries that were executed. There's no way to 
troubleshoot the problem without know the queries.

On Wednesday, November 13, 2013 11:37:43 PM UTC-7, Dominic Rodger wrote:
>
> Currently, when assertNumQueries fails, the output is perhaps less helpful 
> than it might be:
>
> Traceback (most recent call last):
>   File 
> "/home/dom/.virtualenvs/kanisa/src/kanisa/kanisa/tests/views/public.py", 
> line 31, in test_kanisa_root_view
> [banner1.pk, banner2.pk, banner3.pk, banner5.pk, ])
>   File 
> "/home/dom/.virtualenvs/kanisa/src/kanisa/.tox/py27-1.5.X/local/lib/python2.7/site-packages/django/test/testcases.py",
>  
> line 195, in __exit__
> executed, self.num
> AssertionError: 5 queries executed, 99 expected
>
>
> When an assertNumQueries check fails, the first thing I (and I'd guess 
> everyone else too) want to know is what queries ran, so it'd be awesome if 
> an assertNumQueries failure printed the list of queries. Charlie Denton has 
> a blog post describing how he does this (
> http://meshy.co.uk/posts/debugging-assertnumqueries-tests/), but I'd like 
> to see this included in Django itself - since it seems generally helpful.
>
> I'm very happy to work on this and provide a patch (and a ticket), I just 
> wanted to solicit some feedback before I did too much work on it, in case 
> I'm missing a reason why this hasn't already been done (I appreciate this 
> can be done outside of Django, but I think its of sufficiently general 
> utility to warrant being in core).
>
> Dom
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/37d8ca4c-c8d3-4f46-b5a1-b32000bbf6b3%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Modify get_or_create() to use field lookups parameters for values

[gavinwahl]

I don't think this is possible to do generally. What would count__gt=1 or 
pub_date__month=12 do?

On Friday, June 6, 2014 3:50:08 PM UTC-6, Patrick Bregman wrote:
>
> Hi all,
>
> First of, I'm new to this list so please tell me if there's something that 
> can be done better. It's the best way to learn for me ;)
>
> Recently I've been doing some reworking of an old (think Django 1.1 or 
> 1.2) webapp of mine into the latest and greatest of Django. In the process 
> I modified some models, and thought that *get_or_create* would be perfect 
> to replace boring try / except cases. Except that it didn't really work as 
> planned. I tried to do a *get_or_create(name__iexact=value, 
> defaults={'slug': slugify(value)})*. I expected this to be smart enough 
> to know that it should fill the field *name* based on the *name__iexact* 
> parameter. Apparently it isn't :) In this case you'd need to add a 
> definition for *name* to the *defaults* dict to get the value into the 
> newly created model. I'm not sure, but I personally think this shouldn't be 
> that hard to fix. It's basically checking (and removing) known field 
> lookups or simply everything after a double underscore, and using that as a 
> field name. Or at least, that's my view on it.
>
> The big question is:
> 1. Is this behavior that exotic that I'm the first one to notice, or did I 
> do a wrong search on Google?
> 2. Would this indeed be as easy (or comparably easy) as I think? 
> 3. Is this behavior we actually want in Django?
>
> If the answer to 2 and 3 are yes, I'll look into giving it a try to making 
> a patch for this myself.
>
> Regards,
> Patrick Bregman
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/11c6e6a3-0b73-41a5-a743-83f7ad828817%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: modelformset_factory and unique_together don't always validate unique fields

[gavinwahl]

I, too, struggle with this frequently.

On Wednesday, July 2, 2014 9:07:04 PM UTC-6, Jon Dufresne wrote:
>
> I'm reporting this to the developers list as I feel this shows a 
> shortfall in (to me) expected behavior. I'm not sure this is exactly a 
> bug or simply a use case the unique validation wasn't designed for. 
>
> Basically, sometimes I want to create model formsets that use a 
> limited number of model fields. These model fields may have a unique 
> constraint using unique_together. The other field (or fields) of 
> unique together may not be included in the formset. Upon validating 
> the form, unique_together fields are only checked if all fields are 
> contained in the form. This means, my unique fields will not be 
> validated automatically. To achieve the validation I usually have to 
> copy parts of Django form validation, missing out on DRY. 
>
> I think one solution would be for model formsets to have a "static" or 
> "constant" fields argument. This could be a single value per field 
> that all forms in the formset would share for a particular set of 
> fields. These fields would not be a part of the rendered form, but 
> could be used for validating the forms and creating new models 
> instances. Thoughts? 
>
> An example where I might do this: suppose I have a "container" model. 
> This model has many "item" models created through a FK relationship. 
> Perhaps a field is unique together with the container. This example 
> could apply to any container/item relationship. I might create a 
> formset for all items of just one container for a bulk (unique) 
> rename. I have created a simple small example that illustrates my 
> point: 
>
> models.py 
>
>  
> class Container(models.Model): 
> pass 
>
> class Item(models.Model): 
> container = models.ForeignKey(Container) 
> name = models.CharField(max_length=100) 
>
> class Meta: 
> unique_together = ('container', 'name') 
>
> ItemFormSet = modelformset_factory(model=Item, fields=['name']) 
>  
>
> tests.py 
>  
> class ItemFormSetTestCase(TestCase): 
> def test_unique_item_name(self): 
> container = Container() 
> container.save() 
> item1 = Item(container=container, name='item1') 
> item1.save() 
> item2 = Item(container=container, name='item2') 
> item2.save() 
> data = { 
> 'form-TOTAL_FORMS': 2, 
> 'form-INITIAL_FORMS': 2, 
> 'form-MAX_NUM_FORMS': 2, 
> 'form-0-id': str(item1.pk), 
> 'form-0-name': 'newname', 
> 'form-1-id': str(item2.pk), 
> 'form-1-name': 'newname', 
> } 
> formset = ItemFormSet( 
> data, 
> queryset=Item.objects.filter(container=container)) 
> self.assertFalse(formset.is_valid()) 
> --- 
>
> This test fails because the uniqueness of name is not actually 
> validated. If I were to go ahead an save this "valid" form, I receive 
> the following error: 
>
> --- 
> Traceback (most recent call last): 
>   File "/home/jon/djtest/djtest/myapp/tests.py", line 27, in 
> test_unique_item_name 
> formset.save() 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py", 
> line 636, in save 
> return self.save_existing_objects(commit) + 
> self.save_new_objects(commit) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py", 
> line 753, in save_existing_objects 
> saved_instances.append(self.save_existing(form, obj, commit=commit)) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py", 
> line 623, in save_existing 
> return form.save(commit=commit) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py", 
> line 457, in save 
> construct=False) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py", 
> line 103, in save_instance 
> instance.save() 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py", 
>
> line 590, in save 
> force_update=force_update, update_fields=update_fields) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py", 
>
> line 618, in save_base 
> updated = self._save_table(raw, cls, force_insert, force_update, 
> using, update_fields) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py", 
>
> line 680, in _save_table 
> forced_update) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py", 
>
> line 724, in _do_update 
> return filtered._update(values) > 0 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/query.py",
>  
>
> line 598, in _update 
> return query.get_compiler(self.db).execute_sql(CURSOR) 
>   File 
> "/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/sql/compiler.py",
>  
>
> line 10