Skip to content

Commit

Permalink
Improve performance of get_objects_for_user, removing sets (#637)
Browse files Browse the repository at this point in the history
* Improve performance of get_objects_for_user, removing sets

* Fixing wrong character for some variables

* Adding order by id for return the elements in the same order of creation

* Changing cast from IntegerField to BigIntegerField
  • Loading branch information
woakas authored and brianmay committed Jan 13, 2020
1 parent 1fae6e2 commit 7803d0c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 11 deletions.
57 changes: 47 additions & 10 deletions guardian/shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import Count, Q, QuerySet
from django.shortcuts import _get_queryset

from django.db.models.functions import Cast
from django.db.models import (
IntegerField,
AutoField,
BigIntegerField,
PositiveIntegerField,
PositiveSmallIntegerField,
SmallIntegerField,
)
from guardian.core import ObjectPermissionChecker
from guardian.ctypes import get_content_type
from guardian.exceptions import MixedContentTypeError, WrongAppError, MultipleIdentityAndObjectError
Expand Down Expand Up @@ -609,14 +617,27 @@ def get_objects_for_user(user, perms, klass=None, use_groups=True, any_perm=Fals
user_obj_perms_queryset = counts.filter(
object_pk_count__gte=len(codenames))

values = user_obj_perms_queryset.values_list(user_fields[0], flat=True)
if user_model.objects.is_generic():
values = set(values)
is_cast_integer = _is_cast_integer_pk(queryset)

field_pk = user_fields[0]
values = user_obj_perms_queryset
if is_cast_integer:
values = values.annotate(
obj_pk=Cast(field_pk, BigIntegerField())
)
field_pk = 'obj_pk'

values = values.values_list(field_pk, flat=True)
q = Q(pk__in=values)
if use_groups:
values = groups_obj_perms_queryset.values_list(group_fields[0], flat=True)
if group_model.objects.is_generic():
values = set(values)
field_pk = group_fields[0]
values = groups_obj_perms_queryset
if is_cast_integer:
values = values.annotate(
obj_pk=Cast(field_pk, BigIntegerField())
)
field_pk = 'obj_pk'
values = values.values_list(field_pk, flat=True)
q |= Q(pk__in=values)

return queryset.filter(q)
Expand Down Expand Up @@ -762,7 +783,23 @@ def get_objects_for_group(group, perms, klass=None, any_perm=False, accept_globa
objects = queryset.filter(pk__in=pk_list)
return objects

values = groups_obj_perms_queryset.values_list(fields[0], flat=True)
if group_model.objects.is_generic():
values = list(values)
is_cast_integer = _is_cast_integer_pk(queryset)

field_pk = fields[0]
values = groups_obj_perms_queryset

if is_cast_integer:
values = values.annotate(
obj_pk=Cast(field_pk, BigIntegerField())
)
field_pk = 'obj_pk'

values = values.values_list(field_pk, flat=True)
return queryset.filter(pk__in=values)


def _is_cast_integer_pk(queryset):
return isinstance(queryset.model._meta.pk, (
IntegerField, AutoField, BigIntegerField,
PositiveIntegerField, PositiveSmallIntegerField,
SmallIntegerField))
2 changes: 1 addition & 1 deletion guardian/testapp/tests/test_shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ def test_any_of_multiple_perms_to_check(self):
['contenttypes.change_contenttype',
'contenttypes.delete_contenttype'], any_perm=True)
self.assertTrue(isinstance(objects, QuerySet))
self.assertEqual([obj for obj in objects.order_by('app_label')],
self.assertEqual([obj for obj in objects.order_by('app_label', 'id')],
[self.obj1, self.obj3])

def test_results_for_different_groups_are_correct(self):
Expand Down

0 comments on commit 7803d0c

Please sign in to comment.