Do not hit db for each user in get_users_with_perms with attach_perms=True

[pld]

As noted in this todo, with attach_perms=True the get_users_with_perms with shortcut hits the db once for each user!

Modify get_users_with_perms so it hits the db once for all users. Maybe we can call get_perms_for_model on User and then merge the two?

(being tracked for our app here, onaio/onadata#474)

[lukaszb]

That's perfectly valid issue. However I'm pretty sure I won't have time for it in foreseeable future and am waiting for the community to provide a solution.

[troygrosfield]

The code is question is:

# TODO: Do not hit db for each user!
users = {}
for user in get_users_with_perms(obj,
        with_group_users=with_group_users):
    users[user] = sorted(get_perms(user, obj))
return users

It would be great if we could also add pagination in here so the entire list doesn't have to be retrieved. Anyway, why not do:

from guardian.models import UserObjectPermission
from guardian.compat import get_user_model

user_model = get_user_model()
users = {}

# get all users with permissions to this object
users_with_perms = get_users_with_perms(
    obj, 
    with_group_users=with_group_users
)
# TODO: do paging on queryset here. This would likely
# be from additional params passed in (page, page_size)

user_ids = users_with_perms.values_list('id', flat=True)

# then get all permissions for these users
permissions_for_users = UserObjectPermission.objects.filter(
    user_id__in=user_ids,
    content_type_id=ContentType.objects.get_for_model(obj),
    object_pk=obj.id
)

# add the user keys for users who have access
for user in users_with_perms:
    users[user] = []

# sort through all the permissions for users
for perm in permissions_for_users:
    users[user_model(perm.user_id)].append(perm)

[troygrosfield]

@lukaszb, thoughts?

[lukaszb]

@troygrosfield thanks for this, yeah, make sense.
However, looks like group object perms are omitted in the resulting dict. Moreover, at resulting dict keys should be full instances, not only User(id=123) I guess.

Could you provide a pull request with changes? It's always easier to see diffs.

[troygrosfield]

@lukaszb, the keys are the user objects. You can reference a key by a mock object as a way to access it in a dict:

$ users = {User(id=1, first_name='Bob'): [], User(id=2, first_name='John'): [], User(id=3, first_name='Doe'): []}
$ users[User(id=2)]
[]
$ users[User(id=2)].append('testing')
$ users
{<User: >: [], <User: >: ['testing'], <User: >: []}
$ users[User(id=2)]
['testing']

I'll get the pull request submitted in a bit.

[cancan101]

There are a few of the permission shortcuts whose DB querying can be cleaned up: #189, #269, and #270.

[troygrosfield]

@lukaszb, see PR: #272

[troygrosfield]

The PR didn't account for group permissions as well. I'm closing the PR for the time being.

[mikeengland]

👍 This would be very useful