Refactor webhooks (cvat-ai#5916)

This PR fixes the following problems: - cvat-ai#5631 - This will allow webhooks to be sent not only when an explicit request has been sent to the server. Co-authored-by: Nikita Manovich <[email protected]> Co-authored-by: yasakova-anastasia <[email protected]> Co-authored-by: Roman Donchenko <[email protected]>
jasgnft · Apr 7, 2023 · 7cff867 · 7cff867
1 parent 37427bb
commit 7cff867
Show file tree

Hide file tree

Showing 14 changed files with 235 additions and 326 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -325,6 +325,8 @@ jobs:
         if: failure()
         run: |
             docker logs cvat_server > ${{ github.workspace }}/tests/cvat_${{ matrix.specs }}.log
+            docker logs cvat_worker_export > ${{ github.workspace }}/tests/cvat_worker_export_${{ matrix.specs }}.log
+            docker logs cvat_worker_import > ${{ github.workspace }}/tests/cvat_worker_import_${{ matrix.specs }}.log
 
       - name: Uploading "cvat" container logs as an artifact
         if: failure()

diff --git a/cvat/apps/engine/mixins.py b/cvat/apps/engine/mixins.py
@@ -17,7 +17,6 @@
 from cvat.apps.engine.models import Location
 from cvat.apps.engine.location import StorageType, get_location_configuration
 from cvat.apps.engine.serializers import DataSerializer
-from cvat.apps.webhooks.signals import signal_update, signal_create, signal_delete
 
 class TusFile:
     _tus_cache_timeout = 3600
@@ -334,11 +333,6 @@ def deserialize(self, request, import_func):
         return self.upload_data(request)
 
 
-class CreateModelMixin(mixins.CreateModelMixin):
-    def perform_create(self, serializer, **kwargs):
-        serializer.save(**kwargs)
-        signal_create.send(self, instance=serializer.instance)
-
 class PartialUpdateModelMixin:
     """
     Update fields of a model instance.
@@ -351,26 +345,8 @@ def _update(self, request, *args, **kwargs):
         return mixins.UpdateModelMixin.update(self, request, *args, **kwargs)
 
     def perform_update(self, serializer):
-        instance = serializer.instance
-        data = serializer.to_representation(instance)
-        old_values = {
-            attr: data[attr] if attr in data else getattr(instance, attr, None)
-            for attr in self.request.data.keys()
-        }
-
         mixins.UpdateModelMixin.perform_update(self, serializer=serializer)
 
-        if getattr(serializer.instance, '_prefetched_objects_cache', None):
-            serializer.instance._prefetched_objects_cache = {}
-
-        signal_update.send(self, instance=serializer.instance, old_values=old_values)
-
     def partial_update(self, request, *args, **kwargs):
         with mock.patch.object(self, 'update', new=self._update, create=True):
             return mixins.UpdateModelMixin.partial_update(self, request=request, *args, **kwargs)
-
-
-class DestroyModelMixin(mixins.DestroyModelMixin):
-    def perform_destroy(self, instance):
-        signal_delete.send(self, instance=instance)
-        super().perform_destroy(instance)
diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py
@@ -542,6 +542,13 @@ def create(cls, **kwargs):
         except IntegrityError:
             raise InvalidLabel("All label names must be unique")
 
+    def get_organization_id(self):
+        if self.project is not None:
+            return self.project.organization.id
+        if self.task is not None:
+            return self.task.organization.id
+        return None
+
     class Meta:
         default_permissions = ()
         constraints = [

diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py
@@ -69,7 +69,7 @@
     av_scan_paths, process_failed_job, configure_dependent_job, parse_exception_message, get_rq_job_meta
 )
 from cvat.apps.engine import backup
-from cvat.apps.engine.mixins import PartialUpdateModelMixin, UploadMixin, AnnotationMixin, SerializeMixin, DestroyModelMixin, CreateModelMixin
+from cvat.apps.engine.mixins import PartialUpdateModelMixin, UploadMixin, AnnotationMixin, SerializeMixin
 from cvat.apps.engine.location import get_location_configuration, StorageType
 
 from . import models, task
@@ -81,6 +81,7 @@
 from cvat.apps.events.handlers import handle_annotations_patch
 from cvat.apps.engine.view_utils import tus_chunk_action
 
+
 _UPLOAD_PARSER_CLASSES = api_settings.DEFAULT_PARSER_CLASSES + [MultiPartParser]
 
 @extend_schema(tags=['server'])
@@ -220,7 +221,7 @@ def plugins(request):
         })
 )
 class ProjectViewSet(viewsets.GenericViewSet, mixins.ListModelMixin,
-    mixins.RetrieveModelMixin, CreateModelMixin, DestroyModelMixin,
+    mixins.RetrieveModelMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin,
     PartialUpdateModelMixin, UploadMixin, AnnotationMixin, SerializeMixin
 ):
     queryset = models.Project.objects.select_related(
@@ -255,8 +256,7 @@ def get_queryset(self):
         return queryset
 
     def perform_create(self, serializer, **kwargs):
-        super().perform_create(
-            serializer,
+        serializer.save(
             owner=self.request.user,
             organization=self.request.iam_context['organization']
         )
@@ -651,7 +651,7 @@ def __call__(self, request, start, stop, db_data):
         })
 )
 class TaskViewSet(viewsets.GenericViewSet, mixins.ListModelMixin,
-    mixins.RetrieveModelMixin, CreateModelMixin, DestroyModelMixin,
+    mixins.RetrieveModelMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin,
     PartialUpdateModelMixin, UploadMixin, AnnotationMixin, SerializeMixin
 ):
     queryset = Task.objects.select_related(
@@ -764,8 +764,7 @@ def perform_update(self, serializer):
             updated_instance.project.save()
 
     def perform_create(self, serializer, **kwargs):
-        super().perform_create(
-            serializer,
+        serializer.save(
             owner=self.request.user,
             organization=self.request.iam_context['organization']
         )
@@ -1644,7 +1643,7 @@ def preview(self, request, pk):
         })
 )
 class IssueViewSet(viewsets.GenericViewSet, mixins.ListModelMixin,
-    mixins.RetrieveModelMixin, CreateModelMixin, DestroyModelMixin,
+    mixins.RetrieveModelMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin,
     PartialUpdateModelMixin
 ):
     queryset = Issue.objects.prefetch_related(
@@ -1681,7 +1680,7 @@ def get_serializer_class(self):
             return IssueWriteSerializer
 
     def perform_create(self, serializer, **kwargs):
-        super().perform_create(serializer, owner=self.request.user)
+        serializer.save(owner=self.request.user)
 
 @extend_schema(tags=['comments'])
 @extend_schema_view(
@@ -1714,7 +1713,7 @@ def perform_create(self, serializer, **kwargs):
         })
 )
 class CommentViewSet(viewsets.GenericViewSet, mixins.ListModelMixin,
-    mixins.RetrieveModelMixin, CreateModelMixin, DestroyModelMixin,
+    mixins.RetrieveModelMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin,
     PartialUpdateModelMixin
 ):
     queryset = Comment.objects.prefetch_related(
@@ -1750,7 +1749,7 @@ def get_serializer_class(self):
             return CommentWriteSerializer
 
     def perform_create(self, serializer, **kwargs):
-        super().perform_create(serializer, owner=self.request.user)
+        serializer.save(owner=self.request.user)
 
 
 @extend_schema(tags=['labels'])

diff --git a/cvat/apps/events/handlers.py b/cvat/apps/events/handlers.py
@@ -13,7 +13,6 @@
 from crum import get_current_user, get_current_request
 
 from cvat.apps.engine.models import (
-    Organization,
     Project,
     Task,
     Job,
@@ -34,13 +33,38 @@
     LabelSerializer,
 )
 from cvat.apps.engine.models import ShapeType
-from cvat.apps.organizations.serializers import OrganizationReadSerializer
-from cvat.apps.webhooks.signals import project_id, organization_id
+from cvat.apps.organizations.models import Membership, Organization, Invitation
+from cvat.apps.organizations.serializers import OrganizationReadSerializer, MembershipReadSerializer, InvitationReadSerializer
 from cvat.apps.engine.log import vlogger
 
 from .event import event_scope, create_event
 from .cache import get_cache
 
+def project_id(instance):
+    if isinstance(instance, Project):
+        return instance.id
+
+    try:
+        pid = getattr(instance, "project_id", None)
+        if pid is None:
+            return instance.get_project_id()
+        return pid
+    except Exception:
+        return None
+
+
+def organization_id(instance):
+    if isinstance(instance, Organization):
+        return instance.id
+
+    try:
+        oid = getattr(instance, "organization_id", None)
+        if oid is None:
+            return instance.get_organization_id()
+        return oid
+    except Exception:
+        return None
+
 
 def task_id(instance):
     if isinstance(instance, Task):
@@ -66,7 +90,7 @@ def job_id(instance):
     except Exception:
         return None
 
-def _get_current_user(instance=None):
+def get_user(instance=None):
     # Try to get current user from request
     user = get_current_user()
     if user is not None:
@@ -85,7 +109,7 @@ def _get_current_user(instance=None):
 
     return None
 
-def _get_current_request(instance=None):
+def get_request(instance=None):
     request = get_current_request()
     if request is not None:
         return request
@@ -108,19 +132,19 @@ def _get_value(obj, key):
     return None
 
 def request_id(instance=None):
-    request = _get_current_request(instance)
+    request = get_request(instance)
     return _get_value(request, "uuid")
 
 def user_id(instance=None):
-    current_user = _get_current_user(instance)
+    current_user = get_user(instance)
     return _get_value(current_user, "id")
 
 def user_name(instance=None):
-    current_user = _get_current_user(instance)
+    current_user = get_user(instance)
     return _get_value(current_user, "username")
 
 def user_email(instance=None):
-    current_user = _get_current_user(instance)
+    current_user = get_user(instance)
     return _get_value(current_user, "email")
 
 def organization_slug(instance):
@@ -135,13 +159,13 @@ def organization_slug(instance):
     except Exception:
         return None
 
-def _get_instance_diff(old_data, data):
-    ingone_related_fields = (
+def get_instance_diff(old_data, data):
+    ignore_related_fields = (
         "labels",
     )
     diff = {}
     for prop, value in data.items():
-        if prop in ingone_related_fields:
+        if prop in ignore_related_fields:
             continue
         old_value = old_data.get(prop)
         if old_value != value:
@@ -205,7 +229,7 @@ def _get_object_name(instance):
 
     return None
 
-def _get_serializer(instance):
+def get_serializer(instance):
     context = {
         "request": get_current_request()
     }
@@ -229,8 +253,16 @@ def _get_serializer(instance):
         serializer = CommentReadSerializer(instance=instance, context=context)
     if isinstance(instance, Label):
         serializer = LabelSerializer(instance=instance, context=context)
+    if isinstance(instance, Membership):
+        serializer = MembershipReadSerializer(instance=instance, context=context)
+    if isinstance(instance, Invitation):
+        serializer = InvitationReadSerializer(instance=instance, context=context)
+
+    return serializer
 
-    if serializer :
+def get_serializer_without_url(instance):
+    serializer = get_serializer(instance)
+    if serializer:
         serializer.fields.pop("url", None)
     return serializer
 
@@ -254,7 +286,7 @@ def handle_create(scope, instance, **kwargs):
     uname = user_name(instance)
     uemail = user_email(instance)
 
-    serializer = _get_serializer(instance=instance)
+    serializer = get_serializer_without_url(instance=instance)
     try:
         payload = serializer.data
     except Exception:
@@ -290,9 +322,9 @@ def handle_update(scope, instance, old_instance, **kwargs):
     uname = user_name(instance)
     uemail = user_email(instance)
 
-    old_serializer = _get_serializer(instance=old_instance)
-    serializer = _get_serializer(instance=instance)
-    diff = _get_instance_diff(old_data=old_serializer.data, data=serializer.data)
+    old_serializer = get_serializer_without_url(instance=old_instance)
+    serializer = get_serializer_without_url(instance=instance)
+    diff = get_instance_diff(old_data=old_serializer.data, data=serializer.data)
 
     timestamp = str(datetime.now(timezone.utc).timestamp())
     for prop, change in diff.items():

diff --git a/cvat/apps/organizations/views.py b/cvat/apps/organizations/views.py
@@ -8,7 +8,7 @@
 from django.utils.crypto import get_random_string
 
 from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_view
-from cvat.apps.engine.mixins import PartialUpdateModelMixin, DestroyModelMixin, CreateModelMixin
+from cvat.apps.engine.mixins import PartialUpdateModelMixin
 
 from cvat.apps.iam.permissions import (
     InvitationPermission, MembershipPermission, OrganizationPermission)
@@ -112,7 +112,7 @@ class Meta:
             '204': OpenApiResponse(description='The membership has been deleted'),
         })
 )
-class MembershipViewSet(mixins.RetrieveModelMixin, DestroyModelMixin,
+class MembershipViewSet(mixins.RetrieveModelMixin, mixins.DestroyModelMixin,
     mixins.ListModelMixin, PartialUpdateModelMixin, viewsets.GenericViewSet):
     queryset = Membership.objects.all()
     ordering = '-id'
@@ -170,8 +170,8 @@ class InvitationViewSet(viewsets.GenericViewSet,
                    mixins.RetrieveModelMixin,
                    mixins.ListModelMixin,
                    PartialUpdateModelMixin,
-                   CreateModelMixin,
-                   DestroyModelMixin,
+                   mixins.CreateModelMixin,
+                   mixins.DestroyModelMixin,
     ):
     queryset = Invitation.objects.all()
     http_method_names = ['get', 'post', 'patch', 'delete', 'head', 'options']
@@ -196,13 +196,12 @@ def get_queryset(self):
         permission = InvitationPermission.create_scope_list(self.request)
         return permission.filter(queryset)
 
-    def perform_create(self, serializer, **kwargs):
-        extra_kwargs = {
-            'owner': self.request.user,
-            'key': get_random_string(length=64),
-            'organization': self.request.iam_context['organization']
-        }
-        super().perform_create(serializer, **extra_kwargs)
+    def perform_create(self, serializer):
+        serializer.save(
+            owner=self.request.user,
+            key=get_random_string(length=64),
+            organization=self.request.iam_context['organization']
+        )
 
     def perform_update(self, serializer):
         if 'accepted' in self.request.query_params:

diff --git a/cvat/apps/webhooks/event_type.py b/cvat/apps/webhooks/event_type.py
@@ -13,12 +13,12 @@ class Events:
     RESOURCES = {
         "project": ["create", "update", "delete"],
         "task": ["create", "update", "delete"],
+        "job": ["create", "update", "delete"],
         "issue": ["create", "update", "delete"],
         "comment": ["create", "update", "delete"],
-        "invitation": ["create", "delete"],  # TO-DO: implement invitation_updated,
-        "membership": ["update", "delete"],
-        "job": ["update"],
-        "organization": ["update"],
+        "organization": ["update", "delete"],
+        "invitation": ["create", "delete"],
+        "membership": ["create", "update", "delete"],
     }
 
     @classmethod
@@ -47,12 +47,9 @@ class AllEvents:
 
 class ProjectEvents:
     webhook_type = WebhookTypeChoice.PROJECT
-    events = [event_name("update", "project")] \
-        + Events.select(["job", "task", "issue", "comment"])
+    events = [*Events.select(["task", "job", "label", "issue", "comment"]), event_name("update", "project"), event_name("delete", "project")]
 
 
 class OrganizationEvents:
     webhook_type = WebhookTypeChoice.ORGANIZATION
-    events = [event_name("update", "organization")] \
-        + Events.select(["membership", "invitation", "project"]) \
-        + ProjectEvents.events
+    events = AllEvents.events