From 894e26116b5a3075c59e012459617d443fa20cfd Mon Sep 17 00:00:00 2001 From: pablohashescobar <118773738+pablohashescobar@users.noreply.github.com> Date: Tue, 17 Jan 2023 01:50:27 +0530 Subject: [PATCH] refactor: performance booster optimization (#176) * refactor: setup multiple select related * chore: upgrade sentry sdk to latest version * refactor: update module and cycle views to increase performance * refactor: remove pagination and make the response simillar to paginated API * fix: update staging to DEBUG True for all logging * refactor: update the query count print statement * refactor: my issues endpoint to remove n+1 * refactor: optimize queries for workspace and project * fix: project member endpoint * fix: revert back workspace members * refactor: update base file to remove workspace and project query and update permission layer accordingly * refactor: update read_only fields in read serializers * fix: read only serializers * chore: update drf package * revert: drf version upgrade * revert: read only fields update * revert: update serializer to old state * chore: update drf to latest version * refactor: update dispatch to display method as well * refactor: optimize cycle and module issue queries * refactor: optimize module endpoint and issue list endpoint * refactor: update prefetch related in modules and cycles * refactor: create permission mapping in permission file --- apiserver/plane/api/permissions/project.py | 16 ++++-- apiserver/plane/api/permissions/workspace.py | 20 +++++-- apiserver/plane/api/serializers/__init__.py | 1 - apiserver/plane/api/serializers/issue.py | 2 +- apiserver/plane/api/views/base.py | 55 ++++--------------- apiserver/plane/api/views/cycle.py | 5 +- apiserver/plane/api/views/issue.py | 58 +++++++++++++++++--- apiserver/plane/api/views/module.py | 20 +++++-- apiserver/plane/api/views/project.py | 15 ++++- apiserver/plane/api/views/workspace.py | 8 +-- apiserver/plane/settings/staging.py | 2 +- apiserver/requirements/base.txt | 4 +- 12 files changed, 126 insertions(+), 80 deletions(-) diff --git a/apiserver/plane/api/permissions/project.py b/apiserver/plane/api/permissions/project.py index 44659a671..f87aaf08e 100644 --- a/apiserver/plane/api/permissions/project.py +++ b/apiserver/plane/api/permissions/project.py @@ -4,6 +4,12 @@ from rest_framework.permissions import BasePermission, SAFE_METHODS # Module import from plane.db.models import WorkspaceMember, ProjectMember +# Permission Mappings +Admin = 20 +Member = 15 +Viewer = 10 +Guest = 5 + class ProjectBasePermission(BasePermission): def has_permission(self, request, view): @@ -22,14 +28,14 @@ class ProjectBasePermission(BasePermission): return WorkspaceMember.objects.filter( workspace__slug=view.workspace_slug, member=request.user, - role__in=[15, 20], + role__in=[Admin, Member], ).exists() ## Only Project Admins can update project attributes return ProjectMember.objects.filter( workspace__slug=view.workspace_slug, member=request.user, - role=20, + role=Admin, project_id=view.project_id, ).exists() @@ -50,14 +56,14 @@ class ProjectMemberPermission(BasePermission): return WorkspaceMember.objects.filter( workspace__slug=view.workspace_slug, member=request.user, - role__in=[15, 20], + role__in=[Admin, Member], ).exists() ## Only Project Admins can update project attributes return ProjectMember.objects.filter( workspace__slug=view.workspace_slug, member=request.user, - role__in=[15, 20], + role__in=[Admin, Member], project_id=view.project_id, ).exists() @@ -80,6 +86,6 @@ class ProjectEntityPermission(BasePermission): return ProjectMember.objects.filter( workspace__slug=view.workspace_slug, member=request.user, - role__in=[15, 20], + role__in=[Admin, Member], project_id=view.project_id, ).exists() diff --git a/apiserver/plane/api/permissions/workspace.py b/apiserver/plane/api/permissions/workspace.py index 510d87ce2..2a2e1d339 100644 --- a/apiserver/plane/api/permissions/workspace.py +++ b/apiserver/plane/api/permissions/workspace.py @@ -2,7 +2,15 @@ from rest_framework.permissions import BasePermission, SAFE_METHODS # Module imports -from plane.db.models import WorkspaceMember, ProjectMember +from plane.db.models import WorkspaceMember + + + +# Permission Mappings +Owner = 20 +Admin = 15 +Member = 10 +Guest = 5 # TODO: Move the below logic to python match - python v3.10 @@ -22,13 +30,15 @@ class WorkSpaceBasePermission(BasePermission): # allow only admins and owners to update the workspace settings if request.method in ["PUT", "PATCH"]: return WorkspaceMember.objects.filter( - member=request.user, workspace=view.workspace, role__in=[15, 20] + member=request.user, + workspace__slug=view.workspace_slug, + role__in=[Owner, Admin], ).exists() # allow only owner to delete the workspace if request.method == "DELETE": return WorkspaceMember.objects.filter( - member=request.user, workspace=view.workspace, role=20 + member=request.user, workspace__slug=view.workspace_slug, role=Owner ).exists() @@ -39,5 +49,7 @@ class WorkSpaceAdminPermission(BasePermission): return False return WorkspaceMember.objects.filter( - member=request.user, workspace=view.workspace, role__in=[15, 20] + member=request.user, + workspace__slug=view.workspace_slug, + role__in=[Owner, Admin], ).exists() diff --git a/apiserver/plane/api/serializers/__init__.py b/apiserver/plane/api/serializers/__init__.py index e2d474901..ba494ec9e 100644 --- a/apiserver/plane/api/serializers/__init__.py +++ b/apiserver/plane/api/serializers/__init__.py @@ -29,7 +29,6 @@ from .issue import ( IssueCommentSerializer, TimeLineIssueSerializer, IssuePropertySerializer, - IssueLabelSerializer, BlockerIssueSerializer, BlockedIssueSerializer, IssueAssigneeSerializer, diff --git a/apiserver/plane/api/serializers/issue.py b/apiserver/plane/api/serializers/issue.py index 569eef88c..227d3b96d 100644 --- a/apiserver/plane/api/serializers/issue.py +++ b/apiserver/plane/api/serializers/issue.py @@ -443,4 +443,4 @@ class IssueSerializer(BaseSerializer): "updated_by", "created_at", "updated_at", - ] + ] \ No newline at end of file diff --git a/apiserver/plane/api/views/base.py b/apiserver/plane/api/views/base.py index 5d8e75fb3..a4b9ac584 100644 --- a/apiserver/plane/api/views/base.py +++ b/apiserver/plane/api/views/base.py @@ -1,6 +1,7 @@ # Django imports from django.urls import resolve from django.conf import settings + # Third part imports from rest_framework import status from rest_framework.viewsets import ModelViewSet @@ -39,32 +40,23 @@ class BaseViewSet(ModelViewSet, BasePaginator): return self.model.objects.all() except Exception as e: print(e) - raise APIException( - "Please check the view", status.HTTP_400_BAD_REQUEST - ) + raise APIException("Please check the view", status.HTTP_400_BAD_REQUEST) def dispatch(self, request, *args, **kwargs): response = super().dispatch(request, *args, **kwargs) if settings.DEBUG: from django.db import connection - print(f'# of Queries: {len(connection.queries)}') + + print( + f"{request.method} - {request.get_full_path()} of Queries: {len(connection.queries)}" + ) return response @property def workspace_slug(self): return self.kwargs.get("slug", None) - @property - def workspace(self): - if self.workspace_slug: - try: - return Workspace.objects.get(slug=self.workspace_slug) - except Workspace.DoesNotExist: - raise NotFound(detail="Workspace does not exist") - else: - return None - @property def project_id(self): project_id = self.kwargs.get("project_id", None) @@ -74,16 +66,6 @@ class BaseViewSet(ModelViewSet, BasePaginator): if resolve(self.request.path_info).url_name == "project": return self.kwargs.get("pk", None) - @property - def project(self): - if self.project_id: - try: - return Project.objects.get(pk=self.project_id) - except Project.DoesNotExist: - raise NotFound(detail="Project does not exist") - else: - return None - class BaseAPIView(APIView, BasePaginator): @@ -110,33 +92,16 @@ class BaseAPIView(APIView, BasePaginator): if settings.DEBUG: from django.db import connection - print(f'# of Queries: {len(connection.queries)}') + + print( + f"{request.method} - {request.get_full_path()} of Queries: {len(connection.queries)}" + ) return response @property def workspace_slug(self): return self.kwargs.get("slug", None) - @property - def workspace(self): - if self.workspace_slug: - try: - return Workspace.objects.get(slug=self.workspace_slug) - except Workspace.DoesNotExist: - raise NotFound(detail="Workspace does not exist") - else: - return None - @property def project_id(self): return self.kwargs.get("project_id", None) - - @property - def project(self): - if self.project_id: - try: - return Project.objects.get(pk=self.project_id) - except Project.DoesNotExist: - raise NotFound(detail="Project does not exist") - else: - return None diff --git a/apiserver/plane/api/views/cycle.py b/apiserver/plane/api/views/cycle.py index 62c0376b3..28125512f 100644 --- a/apiserver/plane/api/views/cycle.py +++ b/apiserver/plane/api/views/cycle.py @@ -32,6 +32,7 @@ class CycleViewSet(BaseViewSet): .filter(project__project_projectmember__member=self.request.user) .select_related("project") .select_related("workspace") + .select_related("owned_by") .distinct() ) @@ -62,8 +63,8 @@ class CycleIssueViewSet(BaseViewSet): .select_related("project") .select_related("workspace") .select_related("cycle") - .select_related("issue") - .select_related("issue__state") + .select_related("issue", "issue__state", "issue__project") + .prefetch_related("issue__assignees", "issue__labels") .distinct() ) diff --git a/apiserver/plane/api/views/issue.py b/apiserver/plane/api/views/issue.py index 73ee40a83..dd012c129 100644 --- a/apiserver/plane/api/views/issue.py +++ b/apiserver/plane/api/views/issue.py @@ -125,7 +125,9 @@ class IssueViewSet(BaseViewSet): .prefetch_related( Prefetch( "issue_module", - queryset=ModuleIssue.objects.select_related("module", "issue"), + queryset=ModuleIssue.objects.select_related( + "module", "issue" + ).prefetch_related("module__members"), ), ) ) @@ -161,10 +163,18 @@ class IssueViewSet(BaseViewSet): return Response(issue_dict, status=status.HTTP_200_OK) - return self.paginate( - request=request, - queryset=issue_queryset, - on_results=lambda issues: IssueSerializer(issues, many=True).data, + return Response( + { + "next_cursor": str(0), + "prev_cursor": str(0), + "next_page_results": False, + "prev_page_results": False, + "count": issue_queryset.count(), + "total_pages": 1, + "extra_stats": {}, + "results": IssueSerializer(issue_queryset, many=True).data, + }, + status=status.HTTP_200_OK, ) except Exception as e: @@ -206,8 +216,42 @@ class IssueViewSet(BaseViewSet): class UserWorkSpaceIssues(BaseAPIView): def get(self, request, slug): try: - issues = Issue.objects.filter( - assignees__in=[request.user], workspace__slug=slug + issues = ( + Issue.objects.filter(assignees__in=[request.user], workspace__slug=slug) + .select_related("project") + .select_related("workspace") + .select_related("state") + .select_related("parent") + .prefetch_related("assignees") + .prefetch_related("labels") + .prefetch_related( + Prefetch( + "blocked_issues", + queryset=IssueBlocker.objects.select_related( + "blocked_by", "block" + ), + ) + ) + .prefetch_related( + Prefetch( + "blocker_issues", + queryset=IssueBlocker.objects.select_related( + "block", "blocked_by" + ), + ) + ) + .prefetch_related( + Prefetch( + "issue_cycle", + queryset=CycleIssue.objects.select_related("cycle", "issue"), + ), + ) + .prefetch_related( + Prefetch( + "issue_module", + queryset=ModuleIssue.objects.select_related("module", "issue"), + ), + ) ) serializer = IssueSerializer(issues, many=True) return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/apiserver/plane/api/views/module.py b/apiserver/plane/api/views/module.py index a1aff67a0..4c121f23f 100644 --- a/apiserver/plane/api/views/module.py +++ b/apiserver/plane/api/views/module.py @@ -15,7 +15,13 @@ from plane.api.serializers import ( ModuleIssueSerializer, ) from plane.api.permissions import ProjectEntityPermission -from plane.db.models import Module, ModuleIssue, Project, Issue, ModuleLink +from plane.db.models import ( + Module, + ModuleIssue, + Project, + Issue, + ModuleLink, +) class ModuleViewSet(BaseViewSet): @@ -45,13 +51,15 @@ class ModuleViewSet(BaseViewSet): .prefetch_related( Prefetch( "issue_module", - queryset=ModuleIssue.objects.select_related("module", "issue"), + queryset=ModuleIssue.objects.select_related( + "module", "issue", "issue__state", "issue__project" + ).prefetch_related("issue__assignees", "issue__labels"), ) ) .prefetch_related( Prefetch( "link_module", - queryset=ModuleLink.objects.select_related("module"), + queryset=ModuleLink.objects.select_related("module", "created_by"), ) ) ) @@ -117,7 +125,9 @@ class ModuleIssueViewSet(BaseViewSet): .select_related("project") .select_related("workspace") .select_related("module") - .select_related("issue") + .select_related("issue", "issue__state", "issue__project") + .prefetch_related("issue__assignees", "issue__labels") + .prefetch_related("module__members") .distinct() ) @@ -164,4 +174,4 @@ class ModuleIssueViewSet(BaseViewSet): return Response( {"error": "Something went wrong please try again later"}, status=status.HTTP_400_BAD_REQUEST, - ) \ No newline at end of file + ) diff --git a/apiserver/plane/api/views/project.py b/apiserver/plane/api/views/project.py index a3113a10a..2ec6faf1e 100644 --- a/apiserver/plane/api/views/project.py +++ b/apiserver/plane/api/views/project.py @@ -67,7 +67,9 @@ class ProjectViewSet(BaseViewSet): .get_queryset() .filter(workspace__slug=self.kwargs.get("slug")) .filter(Q(project_projectmember__member=self.request.user) | Q(network=2)) - .select_related("workspace", "workspace__owner") + .select_related( + "workspace", "workspace__owner", "default_assignee", "project_lead" + ) .distinct() ) @@ -294,7 +296,7 @@ class UserProjectInvitationsViewset(BaseViewSet): super() .get_queryset() .filter(email=self.request.user.email) - .select_related("workspace") + .select_related("workspace", "workspace__owner", "project") ) def create(self, request): @@ -349,6 +351,7 @@ class ProjectMemberViewSet(BaseViewSet): .filter(project_id=self.kwargs.get("project_id")) .select_related("project") .select_related("member") + .select_related("workspace", "workspace__owner") ) @@ -481,6 +484,7 @@ class ProjectMemberInvitationsViewset(BaseViewSet): .filter(workspace__slug=self.kwargs.get("slug")) .filter(project_id=self.kwargs.get("project_id")) .select_related("project") + .select_related("workspace", "workspace__owner") ) @@ -496,7 +500,12 @@ class ProjectMemberInviteDetailViewSet(BaseViewSet): ] def get_queryset(self): - return self.filter_queryset(super().get_queryset().select_related("project")) + return self.filter_queryset( + super() + .get_queryset() + .select_related("project") + .select_related("workspace", "workspace__owner") + ) class ProjectIdentifierEndpoint(BaseAPIView): diff --git a/apiserver/plane/api/views/workspace.py b/apiserver/plane/api/views/workspace.py index 53f0159c4..e03a80e82 100644 --- a/apiserver/plane/api/views/workspace.py +++ b/apiserver/plane/api/views/workspace.py @@ -176,7 +176,7 @@ class InviteWorkspaceEndpoint(BaseAPIView): workspace_members = WorkspaceMember.objects.filter( workspace_id=workspace.id, member__email__in=[email.get("email") for email in emails], - ) + ).select_related("member", "worspace", "workspace__owner") if len(workspace_members): return Response( @@ -339,7 +339,7 @@ class WorkspaceInvitationsViewset(BaseViewSet): super() .get_queryset() .filter(workspace__slug=self.kwargs.get("slug")) - .select_related("workspace") + .select_related("workspace", "workspace__owner") ) @@ -353,7 +353,7 @@ class UserWorkspaceInvitationsEndpoint(BaseViewSet): super() .get_queryset() .filter(email=self.request.user.email) - .select_related("workspace") + .select_related("workspace", "workspace__owner") ) def create(self, request): @@ -524,7 +524,7 @@ class UserLastProjectWithWorkspaceEndpoint(BaseAPIView): project_member = ProjectMember.objects.filter( workspace_id=last_workspace_id, member=request.user - ).select_related("workspace", "project", "member") + ).select_related("workspace", "project", "member", "workspace__owner") project_member_serializer = ProjectMemberSerializer( project_member, many=True diff --git a/apiserver/plane/settings/staging.py b/apiserver/plane/settings/staging.py index 2804d4e29..429530d94 100644 --- a/apiserver/plane/settings/staging.py +++ b/apiserver/plane/settings/staging.py @@ -14,7 +14,7 @@ from sentry_sdk.integrations.redis import RedisIntegration from .common import * # noqa # Database -DEBUG = False +DEBUG = True DATABASES = { "default": { "ENGINE": "django.db.backends.postgresql_psycopg2", diff --git a/apiserver/requirements/base.txt b/apiserver/requirements/base.txt index 407c8b8c8..7dff0a765 100644 --- a/apiserver/requirements/base.txt +++ b/apiserver/requirements/base.txt @@ -6,7 +6,7 @@ django-taggit==2.1.0 psycopg2==2.9.3 django-oauth-toolkit==2.0.0 mistune==2.0.3 -djangorestframework==3.13.1 +djangorestframework==3.14.0 redis==4.2.2 django-nested-admin==3.4.0 django-cors-headers==3.11.0 @@ -16,7 +16,7 @@ faker==13.4.0 django-filter==21.1 jsonmodels==2.5.0 djangorestframework-simplejwt==5.1.0 -sentry-sdk==1.5.12 +sentry-sdk==1.13.0 django-s3-storage==0.13.6 django-crum==0.7.9 django-guardian==2.4.0