Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor webhooks #5916

Merged
merged 38 commits into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
632bd92
use built-in django signals for sending webhooks
sizov-kirill Mar 23, 2023
af4c460
fix tests
sizov-kirill Mar 23, 2023
2bbcf8f
send webhooks after deleting project or organization
sizov-kirill Mar 23, 2023
b5fd22e
rest api tests: rebuild images
sizov-kirill Mar 23, 2023
d195da0
add migration file
sizov-kirill Mar 23, 2023
ea5da16
fix black warnings
sizov-kirill Mar 23, 2023
eb1a2c3
fix isort warnings
sizov-kirill Mar 23, 2023
1cf847a
fix isort warnings
sizov-kirill Mar 23, 2023
8d7c472
Merge branch 'develop' into sk/refactor-webhooks
sizov-kirill Mar 23, 2023
e5e23d3
fix migration
sizov-kirill Mar 23, 2023
7ee387f
update schema
sizov-kirill Mar 23, 2023
721a33a
update schema
sizov-kirill Mar 23, 2023
0956912
update assets
sizov-kirill Mar 24, 2023
eb5709b
fix deleting webhooks
sizov-kirill Mar 24, 2023
79fdd7e
use docker-compose v2
sizov-kirill Mar 24, 2023
787104b
update tests
sizov-kirill Mar 24, 2023
1ab6b7c
fix black warnings
sizov-kirill Mar 24, 2023
62110fb
revert changes for webhooks ping view
sizov-kirill Mar 24, 2023
f2b68a7
send information between pre and post signals
sizov-kirill Mar 24, 2023
31099c5
Merge branch 'develop' into sk/refactor-webhooks
sizov-kirill Mar 24, 2023
ad4c66e
extend logs for e2e tests
sizov-kirill Mar 25, 2023
69fc37f
Merge branch 'develop' into sk/refactor-webhooks
nmanovic Mar 27, 2023
10a1239
fix bug with project deleting
sizov-kirill Mar 27, 2023
2ac2386
Merge branch 'sk/refactor-webhooks' of https://github.com/opencv/cvat…
sizov-kirill Mar 27, 2023
d111e36
Merge branch 'develop' into sk/refactor-webhooks
yasakova-anastasia Mar 28, 2023
8032617
fix comments
sizov-kirill Apr 6, 2023
8d6c08f
fixes
sizov-kirill Apr 6, 2023
d4112d7
merge develop with resolving conflicts
sizov-kirill Apr 6, 2023
2f079fe
fix imports
sizov-kirill Apr 6, 2023
8e56962
Update tests/python/shared/fixtures/init.py
Apr 6, 2023
602d0b8
remove invitation update event
sizov-kirill Apr 6, 2023
64ca96a
apply review comments
sizov-kirill Apr 6, 2023
db2aaeb
fix test
sizov-kirill Apr 6, 2023
aa03260
Merge branch 'sk/refactor-webhooks' of https://github.com/opencv/cvat…
sizov-kirill Apr 6, 2023
69fed67
fix black warning
sizov-kirill Apr 6, 2023
757b3a1
delete label event, raise error if _deleted_object not accessible
sizov-kirill Apr 6, 2023
ba72abe
update schema
sizov-kirill Apr 6, 2023
d8a0d10
fix tests
sizov-kirill Apr 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
24 changes: 0 additions & 24 deletions cvat/apps/engine/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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)
7 changes: 7 additions & 0 deletions cvat/apps/engine/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
21 changes: 10 additions & 11 deletions cvat/apps/engine/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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
Expand All @@ -81,6 +81,7 @@
from cvat.apps.engine.cache import MediaCache
from cvat.apps.events.handlers import handle_annotations_patch


_UPLOAD_PARSER_CLASSES = api_settings.DEFAULT_PARSER_CLASSES + [MultiPartParser]

@extend_schema(tags=['server'])
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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']
)
Expand Down Expand Up @@ -673,7 +673,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(
Expand Down Expand Up @@ -797,8 +797,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']
)
Expand Down Expand Up @@ -1711,7 +1710,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(
Expand Down Expand Up @@ -1748,7 +1747,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(
Expand Down Expand Up @@ -1781,7 +1780,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(
Expand Down Expand Up @@ -1817,7 +1816,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'])
Expand Down
68 changes: 50 additions & 18 deletions cvat/apps/events/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from crum import get_current_user, get_current_request

from cvat.apps.engine.models import (
Organization,
Project,
Task,
Job,
Expand All @@ -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):
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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):
Expand All @@ -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:
Expand Down Expand Up @@ -205,7 +229,7 @@ def _get_object_name(instance):

return None

def _get_serializer(instance):
def get_serializer(instance):
context = {
"request": get_current_request()
}
Expand All @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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():
Expand Down
21 changes: 10 additions & 11 deletions cvat/apps/organizations/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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']
Expand All @@ -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:
Expand Down
15 changes: 6 additions & 9 deletions cvat/apps/webhooks/event_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Loading