Merge pull request #1680 from anoek/performance

hosts and roles query performance improvements

awx/api/serializers.py

@@ -1443,8 +1443,13 @@ class RoleSerializer(BaseSerializer):
 
     class Meta:
         model = Role
-        fields = ('*', 'description', 'name')
+        read_only_fields = ('id', 'role_field', 'description', 'name')
-        read_only_fields = ('description', 'name')
+
+    def to_representation(self, obj):
+        ret = super(RoleSerializer, self).to_representation(obj)
+        ret.pop('created')
+        ret.pop('modified')
+        return ret
 
     def get_related(self, obj):
         ret = super(RoleSerializer, self).get_related(obj)

awx/api/views.py

@@ -814,8 +814,10 @@ class TeamRolesList(SubListCreateAttachDetachAPIView):
     relationship='member_role.children'
 
     def get_queryset(self):
-        team = Team.objects.get(pk=self.kwargs['pk'])
+        team = get_object_or_404(Team, pk=self.kwargs['pk'])
-        return team.member_role.children.filter(id__in=Role.visible_roles(self.request.user))
+        if not self.request.user.can_access(Team, 'read', team):
+            raise PermissionDenied()
+        return Role.filter_visible_roles(self.request.user, team.member_role.children.all())
 
     # XXX: Need to enforce permissions
     def post(self, request, *args, **kwargs):
@@ -1081,8 +1083,10 @@ class UserRolesList(SubListCreateAttachDetachAPIView):
     permission_classes = (IsAuthenticated,)
 
     def get_queryset(self):
-        #u = User.objects.get(pk=self.kwargs['pk'])
+        u = get_object_or_404(User, pk=self.kwargs['pk'])
-        return Role.visible_roles(self.request.user).filter(members__in=[int(self.kwargs['pk']), ])
+        if not self.request.user.can_access(User, 'read', u):
+            raise PermissionDenied()
+        return Role.filter_visible_roles(self.request.user, u.roles.all())
 
     def post(self, request, *args, **kwargs):
         # Forbid implicit role creation here

awx/main/access.py

@@ -379,12 +379,16 @@ class HostAccess(BaseAccess):
 
     def get_queryset(self):
         inv_qs = Inventory.accessible_objects(self.user, 'read_role')
-        group_qs = Group.accessible_objects(self.user, 'read_role')
+        group_qs = Group.accessible_objects(self.user, 'read_role').exclude(inventory__in=inv_qs)
-        qs = (self.model.objects.filter(inventory=inv_qs) | self.model.objects.filter(groups=group_qs)).distinct()
+        if group_qs.count():
-        #qs = qs.select_related('created_by', 'modified_by', 'inventory',
+            qs = self.model.objects.filter(Q(inventory__in=inv_qs) | Q(groups__in=group_qs))
-        #                       'last_job__job_template',
+        else:
-        #                       'last_job_host_summary__job')
+            qs = self.model.objects.filter(inventory__in=inv_qs)
-        #return qs.prefetch_related('groups').all()
+
+        qs = qs.select_related('created_by', 'modified_by', 'inventory',
+                               'last_job__job_template',
+                               'last_job_host_summary__job')
+        qs =qs.prefetch_related('groups').all()
         return qs
 
     def can_read(self, obj):
@@ -491,7 +495,7 @@ class InventorySourceAccess(BaseAccess):
     def get_queryset(self):
         qs = self.model.objects.all()
         qs = qs.select_related('created_by', 'modified_by', 'group', 'inventory')
-        inventory_ids = set(self.user.get_queryset(Inventory).values_list('id', flat=True))
+        inventory_ids = self.user.get_queryset(Inventory)
         return qs.filter(Q(inventory_id__in=inventory_ids) |
                          Q(group__inventory_id__in=inventory_ids))
 

awx/main/migrations/0008_v300_rbac_changes.py

@@ -129,6 +129,10 @@ class Migration(migrations.Migration):
             name='ancestors',
             field=models.ManyToManyField(related_name='descendents', through='main.RoleAncestorEntry', to='main.Role'),
         ),
+        migrations.AlterIndexTogether(
+            name='role',
+            index_together=set([('content_type', 'object_id')]),
+        ),
         migrations.AlterIndexTogether(
             name='roleancestorentry',
             index_together=set([('ancestor', 'content_type_id', 'object_id'), ('ancestor', 'content_type_id', 'role_field'), ('ancestor', 'descendent')]),

awx/main/models/rbac.py

@@ -8,7 +8,6 @@ import contextlib
 
 # Django
 from django.db import models, transaction, connection
-from django.db.models import Q
 from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy as _
 from django.contrib.contenttypes.models import ContentType
@@ -106,6 +105,9 @@ class Role(models.Model):
         app_label = 'main'
         verbose_name_plural = _('roles')
         db_table = 'main_rbac_roles'
+        index_together = [
+            ("content_type", "object_id")
+        ]
 
     role_field = models.TextField(null=False)
     singleton_name = models.TextField(null=True, default=None, db_index=True, unique=True)
@@ -348,7 +350,43 @@ class Role(models.Model):
 
     @staticmethod
     def visible_roles(user):
-        return Role.objects.filter(Q(descendents__in=user.roles.filter()) | Q(ancestors__in=user.roles.filter()))
+        sql_params = {
+            'ancestors_table': Role.ancestors.through._meta.db_table,
+            'parents_table': Role.parents.through._meta.db_table,
+            'roles_table': Role._meta.db_table,
+            'ids': ','.join(str(x) for x in user.roles.values_list('id', flat=True))
+        }
+
+        qs = Role.objects.extra(
+            where = ['''
+                    %(roles_table)s.id IN (
+                        SELECT descendent_id FROM %(ancestors_table)s WHERE ancestor_id IN (%(ids)s)
+                        UNION
+                        SELECT ancestor_id FROM %(ancestors_table)s WHERE descendent_id IN (%(ids)s)
+                    )
+                    ''' % sql_params]
+        )
+        return qs
+
+    @staticmethod
+    def filter_visible_roles(user, roles_qs):
+        sql_params = {
+            'ancestors_table': Role.ancestors.through._meta.db_table,
+            'parents_table': Role.parents.through._meta.db_table,
+            'roles_table': Role._meta.db_table,
+            'ids': ','.join(str(x) for x in user.roles.all().values_list('id', flat=True))
+        }
+
+        qs = roles_qs.extra(
+            where = ['''
+                EXISTS (
+                    SELECT 1 FROM
+                    %(ancestors_table)s
+                    WHERE (descendent_id = %(roles_table)s.id AND ancestor_id IN (%(ids)s))
+                       OR (ancestor_id = %(roles_table)s.id AND descendent_id IN (%(ids)s))
+                ) ''' % sql_params]
+        )
+        return qs
 
     @staticmethod
     def singleton(name):

awx/main/tests/functional/test_rbac_api.py

@@ -96,6 +96,7 @@ def test_user_view_other_user_roles(organization, inventory, team, get, alice, b
     'Users can see roles for other users, but only the roles that that user has access to see as well'
     organization.member_role.members.add(alice)
     organization.admin_role.members.add(bob)
+    organization.member_role.members.add(bob)
     custom_role = Role.objects.create(name='custom_role-test_user_view_admin_roles_list')
     organization.member_role.children.add(custom_role)
     team.member_role.members.add(bob)