[RHCLOUD-36102] - Slow loading of data from roles endpoint perfo…

rbac/management/role/serializer.py

@@ -92,6 +92,8 @@ class RoleSerializer(serializers.ModelSerializer):
     access = AccessSerializer(many=True)
     policyCount = serializers.IntegerField(read_only=True)
     accessCount = serializers.IntegerField(read_only=True)
+    groups_in_count = serializers.IntegerField(read_only=True)
+    groups_in = serializers.ListField(read_only=True)
     applications = serializers.SerializerMethodField()
     system = serializers.BooleanField(read_only=True)
     platform_default = serializers.BooleanField(read_only=True)
@@ -113,6 +115,8 @@ class Meta:
             "access",
             "policyCount",
             "accessCount",
+            "groups_in_count",
+            "groups_in",
             "applications",
             "system",
             "platform_default",

rbac/management/role/view.py

@@ -25,7 +25,7 @@
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
-from django.db.models import Q
+from django.db.models import F, Q
 from django.db.models.aggregates import Count
 from django.http import Http404
 from django.utils.translation import gettext as _
@@ -34,6 +34,8 @@
 from management.models import AuditLog, Permission
 from management.notifications.notification_handlers import role_obj_change_notification_handler
 from management.permissions import RoleAccessPermission
+from management.principal.model import Principal
+from management.principal.proxy import PrincipalProxy
 from management.querysets import get_role_queryset, user_has_perm
 from management.relation_replicator.relation_replicator import DualWriteException, ReplicationEventType
 from management.role.relation_api_dual_write_handler import (
@@ -46,9 +48,10 @@
 from rest_framework.filters import OrderingFilter
 from rest_framework.response import Response
 
-from api.models import Tenant
+from api.common.pagination import StandardResultsSetPagination
+from api.models import Tenant, User
 from rbac.env import ENVIRONMENT
-from .model import Role
+from .model import ExtTenant, Role
 from .serializer import RoleSerializer
 
 TESTING_APP = os.getenv("TESTING_APPLICATION")
@@ -141,6 +144,7 @@ class RoleViewSet(
     filterset_class = RoleFilter
     ordering_fields = ("name", "display_name", "modified", "policyCount")
     ordering = ("name",)
+    default_limit = StandardResultsSetPagination.default_limit
 
     def get_queryset(self):
         """Obtain queryset for requesting user based on access and action."""
@@ -324,7 +328,210 @@ def list(self, request, *args, **kwargs):
             }
 
         """
-        return super().list(request=request, args=args, kwargs=kwargs)
+        public_tenant = Tenant.objects.get(tenant_name="public")
+        # Filtering
+        query_params = {
+            "external_tenant": request.query_params.get("external_tenant", None),
+            "system": request.query_params.get("system", None),
+            "application": request.query_params.get("application", None),
+            "display_name": request.query_params.get("display_name", None),
+            "name_match": request.query_params.get("name_match", None),
+            "permission": request.query_params.get("permission", None),
+            "name": request.query_params.get("name", None),
+            "platform_default": request.query_params.get("platform_default", None),
+            "admin_default": request.query_params.get("admin_default", None),
+            "username": request.query_params.get("username", None),
+            "limit": request.query_params.get("limit", 10),
+            "offset": request.query_params.get("offset", 0),
+            "add_fields": request.query_params.get("add_fields", ""),
+        }
+        add_fields = query_params["add_fields"]
+        add_fields_split = add_fields.split(",")
+
+        base_queryset = (
+            Role.objects.prefetch_related("access", "ext_relation")
+            .filter(tenant__in=[request.tenant, public_tenant])
+            .annotate(
+                policyCount=Count("policies", distinct=True),
+                accessCount=Count("access", distinct=True),
+            )
+        )
+
+        # Dynamic annotations
+        if "groups_in_count" in add_fields_split:
+            base_queryset = base_queryset.annotate(groups_in_count=Count("policies__group"))
+        elif "groups_in" in add_fields_split:
+            base_queryset = base_queryset.annotate(groups_in=F("policies__group__name"))
+        else:
+            base_queryset = base_queryset
+
+        filters = []
+        limit = query_params["limit"]
+        offset = int(query_params["offset"])
+        path = request.path
+
+        previous_offset = 0
+        if offset - limit > 0:
+            previous_offset = offset - limit
+
+        if query_params:
+            filtered_queryset = base_queryset
+            system_value = str(query_params["system"]).lower()
+
+            # Check if "add_fields" is a valid field
+            additional_fields = {"access", "groups_in", "groups_in_count"}
+            if query_params["add_fields"]:
+                split_fields = query_params["add_fields"].split(",")
+                invalid_field = [field for field in split_fields if field not in additional_fields]
+                if invalid_field:
+                    return Response(
+                        status=status.HTTP_400_BAD_REQUEST,
+                        data={
+                            "errors": [
+                                {
+                                    "detail": "Invalid additional field passed in query",
+                                    "source": "add_fields invalid parameter",
+                                    "status": status.HTTP_400_BAD_REQUEST,
+                                }
+                            ]
+                        },
+                    )
+
+            # Username filter
+            if query_params["username"]:
+                princ = Principal.objects.filter(username=query_params["username"]).first()
+                if not princ:
+                    return Response(
+                        status=status.HTTP_400_BAD_REQUEST,
+                        data={
+                            "errors": [
+                                {
+                                    "detail": "Principal not found for this username",
+                                    "source": "Invalid username query parameter",
+                                    "status": status.HTTP_400_BAD_REQUEST,
+                                }
+                            ]
+                        },
+                    )
+
+                proxy = PrincipalProxy
+                results = proxy.request_filtered_principals(
+                    query_params["username"],
+                    org_id=request.user.org_id,
+                )
+                results_exist = results["data"]
+                if results_exist:
+                    principal = results["data"][0]
+
+                    # Convert from principal to User Model to add to request object
+                    org_id = principal.get("org_id")
+                    user_id = principal.get("user_id")
+                    username = principal.get("username")
+                    account_number = principal.get("account_number")
+                    is_active = principal.get("is_active")
+
+                    # Map principal fields to User fields
+                    user = User()
+                    user.org_id = org_id
+                    user.user_id = user_id
+                    user.username = username
+                    user.account = account_number
+                    user.is_active = is_active
+                    request.user_from_query = user
+
+            # External tenant filter
+            if query_params["external_tenant"]:
+                ext_tenant = ExtTenant.objects.get(name=query_params["external_tenant"])
+                if ext_tenant:
+                    filters.append(Q(ext_relation__ext_tenant=ext_tenant))
+            # System value filter
+            if system_value == "false":
+                filters.append(Q(system=False))
+            elif system_value == "true":
+                filters.append(Q(system=True))
+            # Application filter
+            if query_params["application"]:
+                applications = query_params["application"].split(",")
+
+                external_tenant = ExtTenant.objects.filter(name=query_params["application"]).first()
+
+                filtered_queryset = filtered_queryset.filter(access__permission__application__in=applications)
+                # If a external tenant exists with the name passed to application query parameter
+                # return the roles for that external tenant
+                if external_tenant:
+                    ext_tenant = ExtTenant.objects.get(name=query_params["application"])
+                    filtered_queryset = base_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate(
+                        external_tenant=F("ext_relation__ext_tenant__name")
+                    )
+            # Display_name & name_match filter
+            if query_params["display_name"]:
+                if query_params["name_match"] == "partial":
+                    filters.append(Q(display_name__contains=query_params["display_name"]))
+                elif query_params["name_match"] == "exact":
+                    filters.append(Q(display_name__exact=query_params["display_name"]))
+                elif not query_params["name_match"]:
+                    filters.append(Q(display_name__contains=query_params["display_name"]))
+                else:
+                    return Response(
+                        status=status.HTTP_400_BAD_REQUEST,
+                        data={
+                            "errors": [
+                                {
+                                    "detail": "Invalid name match value provided",
+                                    "source": "name_match query parameter",
+                                    "status": status.HTTP_400_BAD_REQUEST,
+                                }
+                            ]
+                        },
+                    )
+            # name & name_match filter
+            if query_params["name"]:
+                if query_params["name_match"] == "partial":
+                    filters.append(Q(name__contains=query_params["name"]))
+                elif query_params["name_match"] == "exact":
+                    filters.append(Q(name__exact=query_params["name"]))
+                elif not query_params["name_match"]:
+                    filters.append(Q(name__contains=query_params["name"]))
+                else:
+                    return Response(
+                        status=status.HTTP_400_BAD_REQUEST,
+                        data={
+                            "errors": [
+                                {
+                                    "detail": "Invalid name match value provided",
+                                    "source": "name_match query parameter",
+                                    "status": status.HTTP_400_BAD_REQUEST,
+                                }
+                            ]
+                        },
+                    )
+            # Permission filter
+            if query_params["permission"]:
+                permissions = query_params["permission"].split(",")
+                filters.append(Q(access__permission__permission__in=permissions))
+
+            # Apply the filters
+            filtered_queryset = filtered_queryset.filter(*filters)
+
+            # Serialize the queryset for response
+            serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request})
+
+            # Metadata
+            meta = {}
+            if query_params.get("username"):
+                meta["count"] = sum(len(group.roles()) for group in princ.group.all())
+            else:
+                meta["count"] = filtered_queryset.count()
+
+            # Pagination
+            links = {
+                "first": f"{path}?limit={limit}&offset=0",
+                "next": f"{path}?limit={limit}&offset={offset + limit}",
+                "previous": f"{path}?limit={limit}&offset={previous_offset}",
+                "last": f"{path}?limit={limit}&offset={meta['count'] - limit if (meta['count'] - limit) >= 0 else 0}",
+            }
+
+            return Response({"meta": meta, "links": links, "data": serializer.data})
 
     def retrieve(self, request, *args, **kwargs):
         """Get a role.

tests/internal/integration/test_integration_views.py

@@ -428,6 +428,7 @@ def test_roles_for_org(self, mock_request):
             **self.request.META,
             follow=True,
         )
+
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data.get("meta").get("count"), 1)
 

tests/management/role/test_view.py

@@ -151,6 +151,7 @@ def setUp(self):
             "display_name",
             "system",
             "created",
+            "access",
             "policyCount",
             "accessCount",
             "modified",
@@ -663,6 +664,7 @@ def test_read_role_list_success(self):
             self.assertIsNotNone(iterRole.get("name"))
             # fields displayed are same as defined
             self.assertEqual(self.display_fields, set(iterRole.keys()))
+
             if iterRole.get("name") == role_name:
                 self.assertEqual(iterRole.get("accessCount"), 2)
                 role = iterRole
@@ -674,6 +676,7 @@ def test_get_role_by_application_single(self):
         url = "{}?application={}".format(URL, "app")
         client = APIClient()
         response = client.get(url, **self.headers)
+
         self.assertEqual(response.data.get("meta").get("count"), 1)
         self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name)
 
@@ -682,6 +685,7 @@ def test_get_role_by_application_using_ext_tenant(self):
         url = "{}?application={}".format(URL, "foo")
         client = APIClient()
         response = client.get(url, **self.headers)
+
         self.assertEqual(response.data.get("meta").get("count"), 1)
         self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name)
 
@@ -1112,6 +1116,7 @@ def test_list_role_with_groups_in_fields_for_admin_scope_success(self):
 
         response_data = response.data.get("data")
         for iterRole in response_data:
+            print(iterRole.get("groups_in"))
             # fields displayed are same as defined, groupsInCount is added
             self.assertEqual(new_display_fields, set(iterRole.keys()))
             self.assertIsNotNone(iterRole.get("groups_in")[0]["name"])
@@ -1183,7 +1188,6 @@ def test_list_role_with_additional_fields_username_success(self, mock_request):
         url = "{}?add_fields={},{}&username={}".format(URL, field_1, field_2, self.principal.username)
         client = APIClient()
         response = client.get(url, **self.headers)
-
         self.assertEqual(len(response.data.get("data")), 5)
 
         role = response.data.get("data")[0]
@@ -1205,6 +1209,7 @@ def test_list_role_with_additional_fields_principal_success(self):
         self.assertEqual(len(response.data.get("data")), 5)
 
         role = response.data.get("data")[0]
+        print(role)
         self.assertEqual(new_display_fields, set(role.keys()))
         self.assertEqual(role["groups_in_count"], 1)
 
@@ -1732,7 +1737,6 @@ def test_external_tenant_filter(self):
         response = client.get(URL, **self.headers)
 
         self.assertEqual(len(response.data.get("data")), 5)
-
         url = f"{URL}?external_tenant=foo"
         client = APIClient()
         response = client.get(url, **self.headers)
@@ -1751,7 +1755,6 @@ def test_list_role_admin_platform_default_groups(self):
         url = f"{URL}?display_name=platform_admin_default_display&add_fields=groups_in_count%2Cgroups_in"
         client = APIClient()
         response = client.get(url, **self.headers)
-
         self.assertEqual(len(response.data.get("data")), 1)
         role = response.data.get("data")[0]
         self.assertEqual(role.get("groups_in_count"), 2)
@@ -1954,6 +1957,7 @@ def test_list_roles_with_User_Access_Admin_success(self):
         response = client.get(url, **self.headers_user_based_principal)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         expected_count = self.system_roles_count + self.non_system_roles_count + 1
+
         self.assertEqual(len(response.data.get("data")), expected_count)
 
         response = client.get(url, **self.headers_service_account_principal)