Improve API perf

CHANGELOG.md

@@ -6,6 +6,8 @@
 - Add field "enabled" in ApprovalWorkflow
 - Remove "operations" field in ApprovalWorkflow form when editing
 - Add the number of items displayed at the end of lists
+- Improve API performance for /api/service-catalog/request/ and /api/service-catalog/instance/
+- Improve performance in homepage when several requests where displayed in "To be reviewed"
 
 ## Feature
 

profiles/api/serializers/scope_serializer.py

@@ -14,7 +14,7 @@ class Meta:
         fields = '__all__'
 
 
-class ScopeSerializer(AbstractScopeSerializer):
+class ScopeSerializerManager(ModelSerializer):
     organization = SerializerMethodField()
     team = SerializerMethodField()
 
@@ -40,6 +40,16 @@ def get_team(self, obj):
             return {}
 
 
+class ScopeSerializerNested(ScopeSerializerManager):
+    pass
+
+
+class ScopeSerializer(AbstractScopeSerializer, ScopeSerializerManager):
+    class Meta:
+        model = Scope
+        fields = '__all__'
+
+
 class AbstractScopeCreateRBACSerializer(ModelSerializer):
     roles = MultipleChoiceField(
         choices=[],

profiles/api/serializers/user_serializers.py

@@ -12,6 +12,15 @@ class Meta:
                   'request_notification_filters', 'instance_notification_filters']
 
 
+class UserSerializerNested(ModelSerializer):
+    class Meta:
+        model = User
+        fields = ['id', 'username', 'email', 'first_name', 'last_name', 'is_staff', 'is_superuser',
+                  'is_active']
+        read_only_fields = ['id', 'username', 'email', 'first_name', 'last_name', 'is_staff',
+                            'is_superuser', 'is_active']
+
+
 class UserSerializer(ModelSerializer):
     class Meta:
         model = User

service_catalog/api/serializers/instance_serializer.py

@@ -1,23 +1,23 @@
 from rest_framework import serializers
 
-from profiles.api.serializers import UserSerializer, ScopeSerializer
+from profiles.api.serializers import ScopeSerializerNested, UserSerializerNested
 from resource_tracker_v2.api.serializers.resource_serializer import ResourceSerializer
 from service_catalog.models import Instance, InstanceState
 
 
 class InstanceReadSerializer(serializers.ModelSerializer):
     state = serializers.ChoiceField(choices=InstanceState.choices)
     resources = ResourceSerializer(many=True, read_only=True)
-    requester = UserSerializer(read_only=True)
-    quota_scope = ScopeSerializer(read_only=True)
+    requester = UserSerializerNested()
+    quota_scope = ScopeSerializerNested(read_only=True)
 
     class Meta:
         model = Instance
         fields = '__all__'
 
 
 class RestrictedInstanceReadSerializer(InstanceReadSerializer):
-    requester = UserSerializer(read_only=True)
+    requester = UserSerializerNested(read_only=True)
 
     class Meta:
         model = Instance

service_catalog/api/serializers/request_serializers.py

@@ -5,7 +5,7 @@
 from rest_framework.serializers import ModelSerializer, CharField
 
 from Squest.utils.squest_encoder import SquestEncoder
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers.user_serializers import UserSerializerNested
 from profiles.models import Scope
 from service_catalog.api.serializers import DynamicSurveySerializer, InstanceReadSerializer
 from service_catalog.models.instance import Instance
@@ -125,7 +125,7 @@ class Meta:
         read_only = True
 
     instance = InstanceReadSerializer(read_only=True)
-    user = UserSerializer(read_only=True)
+    user = UserSerializerNested(read_only=True)
 
 
 class AdminRequestSerializer(ModelSerializer):
@@ -134,4 +134,4 @@ class Meta:
         exclude = ['periodic_task', 'periodic_task_date_expire', 'failure_message']
 
     instance = InstanceReadSerializer(read_only=True)
-    user = UserSerializer(read_only=True)
+    user = UserSerializerNested(read_only=True)

service_catalog/api/views/instance_api_views.py

@@ -15,7 +15,9 @@ class InstanceList(SquestListCreateAPIView):
     filterset_class = InstanceFilter
 
     def get_queryset(self):
-        return Instance.get_queryset_for_user(self.request.user, 'service_catalog.view_instance')
+        return Instance.get_queryset_for_user(self.request.user, 'service_catalog.view_instance').prefetch_related(
+            "requester", "requester__profile", "resources", "requester__groups", "quota_scope", "service",
+        )
 
     def get_serializer_class(self):
         if self.request.method in ["POST"]:

service_catalog/api/views/request_api_views.py

@@ -17,7 +17,12 @@ class RequestList(SquestListAPIView):
     filterset_class = RequestFilter
 
     def get_queryset(self):
-        return Request.get_queryset_for_user(self.request.user, 'service_catalog.view_request')
+        return Request.get_queryset_for_user(self.request.user, 'service_catalog.view_request').prefetch_related(
+                "user", "operation", "instance__requester","instance__requester__profile","instance__resources","instance__requester__groups" ,"instance__quota_scope", "instance__service",
+                "operation__service", "approval_workflow_state", "approval_workflow_state__approval_workflow",
+                "approval_workflow_state__current_step",
+                "approval_workflow_state__current_step__approval_step", "approval_workflow_state__approval_step_states"
+            )
 
     def get_serializer_class(self):
         if self.request.user.has_perm("service_catalog.view_admin_survey"):

tests/test_service_catalog/test_api/test_request/test_request_details.py

@@ -1,8 +1,7 @@
 from rest_framework import status
 from rest_framework.reverse import reverse
 
-from profiles.api.serializers import ScopeSerializer
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers import ScopeSerializerNested, UserSerializerNested
 from profiles.models import Permission
 from service_catalog.models import Request
 from tests.test_service_catalog.base_test_request import BaseTestRequestAPI
@@ -51,7 +50,7 @@ def setUp(self):
             'tower_job_id': self.test_request_standard_user_1.tower_job_id,
             'state': self.test_request_standard_user_1.state,
             'operation': self.test_request_standard_user_1.operation.id,
-            'user': UserSerializer(instance=self.test_request_standard_user_1.user).data
+            'user': UserSerializerNested(instance=self.test_request_standard_user_1.user).data
         }
         self.expected_instance = {
             'id': self.test_request_standard_user_1.instance.id,
@@ -60,8 +59,8 @@ def setUp(self):
             'spec': self.test_request_standard_user_1.instance.spec,
             'user_spec': self.test_request_standard_user_1.instance.user_spec,
             'service': self.test_request_standard_user_1.instance.service.id,
-            'requester': UserSerializer(self.test_request_standard_user_1.instance.requester).data,
-            'quota_scope': ScopeSerializer(self.test_request_standard_user_1.instance.quota_scope).data
+            'requester': UserSerializerNested(self.test_request_standard_user_1.instance.requester).data,
+            'quota_scope': ScopeSerializerNested(self.test_request_standard_user_1.instance.quota_scope).data
         }
         self.expected_data_list = [self.expected_data, self.expected_instance]
 

tests/test_service_catalog/test_api/test_request/test_request_patch.py

@@ -1,7 +1,7 @@
 from rest_framework import status
 from rest_framework.reverse import reverse
 
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers.user_serializers import UserSerializerNested
 from service_catalog.models import Request, RequestState
 from tests.test_service_catalog.base_test_request import BaseTestRequestAPI
 from tests.utils import check_data_in_dict
@@ -37,7 +37,7 @@ def setUp(self):
             'tower_job_id': self.test_request_standard_user_1.tower_job_id,
             'state': RequestState.NEED_INFO,
             'operation': self.update_operation_test.id,
-            'user': UserSerializer(self.test_request_standard_user_1.user).data
+            'user': UserSerializerNested(self.test_request_standard_user_1.user).data
         }
 
     def test_admin_patch_request(self):

tests/test_service_catalog/test_api/test_request/test_request_put.py

@@ -1,7 +1,7 @@
 from rest_framework import status
 from rest_framework.reverse import reverse
 
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers.user_serializers import UserSerializerNested
 from service_catalog.models import Request, RequestState
 from tests.test_service_catalog.base_test_request import BaseTestRequestAPI
 from tests.utils import check_data_in_dict
@@ -39,7 +39,7 @@ def test_admin_put_on_request(self):
         response = self.client.put(self.get_request_details_url, data=self.put_data, format="json")
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         expected = self.put_data
-        expected['user'] = UserSerializer(instance=self.standard_user).data
+        expected['user'] = UserSerializerNested(instance=self.standard_user).data
         check_data_in_dict(self, [expected], [response.data])
 
     def test_admin_cannot_put_on_request_not_full(self):

tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_process.py

@@ -2,8 +2,8 @@
 
 from django.urls import reverse
 
-from profiles.api.serializers import ScopeSerializer
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers import ScopeSerializerNested
+from profiles.api.serializers.user_serializers import UserSerializerNested
 from service_catalog.models import RequestState, InstanceState
 from tests.test_service_catalog.base_test_request import BaseTestRequestAPI
 
@@ -40,8 +40,8 @@ def _process(self, status=200, expected_instance_state=InstanceState.PROVISIONIN
                 'spec': {},
                 'state': expected_instance_state,
                 'service': self.test_request.operation.service.id,
-                'quota_scope': ScopeSerializer(self.test_quota_scope_org).data,
-                'requester': UserSerializer(self.test_request.instance.requester).data
+                'quota_scope': ScopeSerializerNested(self.test_quota_scope_org).data,
+                'requester': UserSerializerNested(self.test_request.instance.requester).data
             }
             self.test_instance.refresh_from_db()
             self.assertEqual(self.test_instance.state, expected_instance_state)

tests/test_service_catalog/test_api/test_serializers/test_request_serializer.py

@@ -54,5 +54,3 @@ def test_request_serializer_field_content(self):
                          self.local_test_request.user.is_superuser)
         self.assertEqual(serializer.data['user']['is_staff'],
                          self.local_test_request.user.is_staff)
-        self.assertEqual(serializer.data['user']['profile']['request_notification_enabled'],
-                         self.local_test_request.user.profile.request_notification_enabled)

tests/test_service_catalog/test_models/test_approval_workflow_reset.py

@@ -1,3 +1,4 @@
+from profiles.models import Organization
 from service_catalog.models import ApprovalWorkflow, Request, RequestState
 from tests.setup import SetupRequest
 from tests.utils import TransactionTestUtils
@@ -45,6 +46,18 @@ def _create_approval_for_org2_team1(self):
         aw.scopes.set([self.team1org2])
         return aw
 
+    def _create_approval_for_org3_team1(self):
+        ##################################
+        # Approval workflow for Org2 - Team 1
+        ##################################
+        aw = ApprovalWorkflow.objects.create(
+            name="AW - Org3 - Team 1",
+            operation=self.operation_create_1,
+            enabled=True
+        )
+        aw.scopes.set([self.team1org3])
+        return aw
+
     def assertRequestUseWorkflow(self, qs, workflow=None):
 
         if not isinstance(qs, QuerySet):
@@ -270,7 +283,8 @@ def test_scenario3(self):
 
         self.assertQuerysetEqualID(other_requests, wf_for_all._get_all_requests_that_should_use_workflow())
         self.assertQuerysetEqualID(request_of_org2, wf_for_org2._get_all_requests_that_should_use_workflow())
-        self.assertQuerysetEqualID(request_of_org2_team1, wf_for_org2_team1._get_all_requests_that_should_use_workflow())
+        self.assertQuerysetEqualID(request_of_org2_team1,
+                                   wf_for_org2_team1._get_all_requests_that_should_use_workflow())
 
         wf_for_org2_team1.reset_all_approval_workflow_state()
         # Org        |  1   |  2   |  2   |  2   |  2   |  2   |  3   |  3   |
@@ -402,3 +416,55 @@ def test_scenario4(self):
         self.assertRequestDontUseWorkflow(other_requests.all())
         self.assertRequestDontUseWorkflow(request_of_org2.all())
         self.assertRequestUseWorkflow(request_of_org2_team1.all(), wf_for_org2_team1)
+
+    def test_scenario5(self):
+
+        # Scenario: setup a workflow for org3_team1 then change scopes and reset
+        request_of_org3_team1 = Request.objects.filter(
+            id__in=[
+                # self.request_1_org1.id,
+                # self.request_2_team1org2.id,
+                # self.request_3_team1org2.id,
+                # self.request_4_team2org2.id,
+                # self.request_5_team2org2.id,
+                # self.request_6_org2.id,
+                self.request_7_team1org3.id,
+                # self.request_8_org3.id,
+            ]
+        ).all()
+        request_of_org1 = Request.objects.filter(
+            id__in=[
+                self.request_1_org1.id,
+                # self.request_2_team1org2.id,
+                # self.request_3_team1org2.id,
+                # self.request_4_team2org2.id,
+                # self.request_5_team2org2.id,
+                # self.request_6_org2.id,
+                # self.request_7_team1org3.id,
+                # self.request_8_org3.id,
+            ]
+        ).all()
+
+        # Workflow for org3 team1
+        aw = self._create_approval_for_org3_team1()
+        aw.scopes.add(self.org1)
+        self.assertQuerysetEqualID(aw._get_request_to_reset(), request_of_org1 | request_of_org3_team1)
+        aw.reset_all_approval_workflow_state()
+        self.assertQuerysetEqualID(request_of_org1 | request_of_org3_team1, aw._get_request_using_workflow())
+
+        # Org        |  1   |  2   |  2   |  2   |  2   |  2   |  3   |  3   |
+        # Team       | None |  1   |  1   |  2   |  2   | None |  1   | None |
+        # Request ID |  1   |  2   |  3   |  4   |  5   |  6   |  7   |  8   |
+        # -----------|------|------|------|------|------|------|------|------|
+        # WF ID      |  1   | None | None | None | None | None |  1   | None |
+
+        aw.scopes.remove(self.team1org3)
+        self.assertQuerysetEqualID(aw._get_request_to_reset(), request_of_org3_team1)
+        aw.reset_all_approval_workflow_state()
+        # Org        |  1   |  2   |  2   |  2   |  2   |  2   |  3   |  3   |
+        # Team       | None |  1   |  1   |  2   |  2   | None |  1   | None |
+        # Request ID |  1   |  2   |  3   |  4   |  5   |  6   |  7   |  8   |
+        # -----------|------|------|------|------|------|------|------|------|
+        # WF ID      |  1   | None | None | None | None | None | None | None |
+        self.assertQuerysetEqualID(request_of_org1, aw._get_request_using_workflow())
+        self.assertRequestDontUseWorkflow(request_of_org3_team1)

tests/test_service_catalog/test_models/test_request.py

@@ -9,8 +9,8 @@
 from django_celery_beat.models import PeriodicTask, IntervalSchedule, CrontabSchedule
 from django_fsm import can_proceed
 
-from profiles.api.serializers import ScopeSerializer
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers import ScopeSerializerNested
+from profiles.api.serializers.user_serializers import UserSerializerNested
 from service_catalog.models import ApprovalWorkflow, ApprovalStep
 from service_catalog.models.instance import InstanceState, Instance
 from service_catalog.models.request import RequestState, Request
@@ -41,18 +41,18 @@ def _check_instance_state_after_process(self, expected_state):
                 'id': self.test_request.id,
                 'state': RequestState.PROCESSING,
                 'operation': self.test_request.operation.id,
-                'user': UserSerializer(self.test_request.user).data
+                'user': UserSerializerNested(self.test_request.user).data
             }
             expected_instance = {
                 'id': self.test_instance.id,
                 'name': 'test_instance_1',
                 'spec': {},
                 'state': expected_state,
                 'service': self.test_request.operation.service.id,
-                'quota_scope': ScopeSerializer(self.test_quota_scope_org).data,
-                'requester': UserSerializer(self.test_request.instance.requester).data
+                'quota_scope': ScopeSerializerNested(self.test_quota_scope_org).data,
+                'requester': UserSerializerNested(self.test_request.instance.requester).data
             }
-            expected_user = UserSerializer(self.test_request.user).data
+            expected_user = UserSerializerNested(self.test_request.user).data
             mock_job_execute.assert_called()
             kwargs = mock_job_execute.call_args[1]
             expected_data_list = [expected_extra_vars, expected_request, expected_instance, expected_user]
@@ -162,18 +162,18 @@ def _check_state_after_accept(self, expected_instance_state, expected_request_st
                     'id': self.test_request.id,
                     'state': RequestState.PROCESSING,
                     'operation': self.test_request.operation.id,
-                    'user': UserSerializer(self.test_request.user).data
+                    'user': UserSerializerNested(self.test_request.user).data
                 }
                 expected_instance = {
                     'id': self.test_instance.id,
                     'name': 'test_instance_1',
                     'spec': {},
                     'state': expected_instance_state,
                     'service': self.test_request.operation.service.id,
-                    'quota_scope': ScopeSerializer(self.test_quota_scope_org).data,
-                    'requester': UserSerializer(self.test_request.instance.requester).data
+                    'quota_scope': ScopeSerializerNested(self.test_quota_scope_org).data,
+                    'requester': UserSerializerNested(self.test_request.instance.requester).data
                 }
-                expected_user = UserSerializer(self.test_request.user).data
+                expected_user = UserSerializerNested(self.test_request.user).data
                 mock_job_execute.assert_called()
                 kwargs = mock_job_execute.call_args[1]
                 expected_data_list = [expected_extra_vars, expected_request, expected_instance, expected_user]

tests/test_service_catalog/test_views/test_admin/test_request/test_admin_request_view.py

@@ -2,8 +2,7 @@
 
 from django.urls import reverse
 
-from profiles.api.serializers import ScopeSerializer
-from profiles.api.serializers.user_serializers import UserSerializer
+from profiles.api.serializers import ScopeSerializerNested, UserSerializerNested
 from profiles.models import Role, Permission, GlobalScope
 from service_catalog.models import Request, RequestMessage, ExceptionServiceCatalog
 from service_catalog.models.instance import InstanceState
@@ -326,8 +325,8 @@ def _process_with_expected_instance_state(self, expected_instance_state,
                 'spec': {},
                 'state': expected_instance_state,
                 'service': self.test_request.operation.service.id,
-                'quota_scope': ScopeSerializer(self.test_request.instance.quota_scope).data,
-                'requester': UserSerializer(self.test_request.instance.requester).data
+                'quota_scope': ScopeSerializerNested(self.test_request.instance.quota_scope).data,
+                'requester': UserSerializerNested(self.test_request.instance.requester).data
             }
             self.test_instance.refresh_from_db()
             self.assertEqual(self.test_instance.state, expected_instance_state)