auth.models.User.get_djangouser_for_user can be improved

[GoogleCodeExporter]

The current implementation of auth.models.User.get_djangouser_for_user 
performs a filter lookup for the current user with:

User.all().filter("user =", user)

This can be done more efficiently (and also supports keeping the same django 
User object when a user's email changes) by using a key_name based on the 
current user.user_id().

See attached patch for details (note: this patch would break backwards 
compatibility as is, not sure if we'd want to hack something up to support 
the legacy lookup, or rely on users to update their datastore to the new 
key_name style.)

Original issue reported on code.google.com by mmcdon...@google.com on 18 Feb 2010 at 8:00

Attachments:

• get_djangouser_for_user.patch

[GoogleCodeExporter]

If not done this way, at least please remove the unnecessary count(). It's 
slowing us
down!

Original comment by pwnfact...@gmail.com on 14 Apr 2010 at 4:20

Attachments:

• stats.png

[GoogleCodeExporter]

The concept sounds good but I'm wary of breaking backwards compatibility. 

Can you easily add support for the legacy style lookups as well?

Original comment by mattbrow...@gmail.com on 16 Apr 2010 at 11:07

[GoogleCodeExporter]

It seems like adding in a hack for maintaining legacy support would make it 
similarly 
inefficient. 

Original comment by pwnfact...@gmail.com on 20 Apr 2010 at 5:13

[GoogleCodeExporter]

Not necessarily (see attached patch with legacy User support)

Original comment by mmcdon...@google.com on 29 Apr 2010 at 12:03

Attachments:

• get_djangouser_for_user.patch

[GoogleCodeExporter]

Patch looks good, and seems effectively as efficient as without legacy support.
One question:

legacy_django_user = cls.all().filter('user =', user).get()

Is this guaranteed to be the *old* User object?

Original comment by pwnfact...@gmail.com on 29 Apr 2010 at 10:55

[GoogleCodeExporter]

Ah, good catch.  Fixed in this patch:

Original comment by mmcdon...@google.com on 29 Apr 2010 at 4:23

Attachments:

• get_djangouser_for_user.patch

[GoogleCodeExporter]

There is an issue with the patch in comment 6 - if you delete the legacy user, 
this 
will break all existing references to the legacy User entity.  

For backwards compatibility, you should leave the legacy User entity in place, 
and 
check for it using the 'user =' filter if the get_by_key_name fails.  If 
neither 
key_name or 'user =' filter matches, then you can create the new User with the 
key_name.  Otherwise, you have to go through your datastore and fixup any 
references 
from the old legacy User entity to the new one you are creating by key_name.

For new projects which do not need to support legacy User entities, you might 
want to 
include a setting to only use the new key_name approach, and then you can use 
the 
get_or_insert method.

Original comment by dherbst on 18 May 2010 at 12:57

[GoogleCodeExporter]

Another good catch.

As for having a config knob for turning off the filter('user', user) check, 
it's only 
going to get called the first time each user visits the site; Subsequent visits 
will 
just fetch by the key_name, so it doesn't seem too big of an issue.

Original comment by mmcdon...@google.com on 26 May 2010 at 7:41

Attachments:

• get_djangouser_for_user.patch

[GoogleCodeExporter]

Woop, noticed a small nit at the end of the previous patch; 2nd try:

Original comment by mmcdon...@google.com on 26 May 2010 at 7:47

Attachments:

• get_djangouser_for_user.patch

[GoogleCodeExporter]

Patch submitted in r103

Original comment by mmcdon...@google.com on 4 Jun 2010 at 5:34

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by mmcdon...@google.com on 19 Nov 2010 at 4:16

• Changed state: Fixed