From a9572d2b4ea7aa28af0f2ff9d39394a87d9084b7 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Wed, 7 Feb 2024 17:05:18 +0200 Subject: [PATCH 1/4] Factor out repetitive created_date and updated_date fields --- cvat/apps/engine/models.py | 43 +++++++++++++++---------------------- cvat/apps/events/signals.py | 2 +- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 53d2ed55a55..8029a032c50 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -24,7 +24,6 @@ from drf_spectacular.utils import extend_schema_field from cvat.apps.engine.utils import parse_specific_attributes -from cvat.apps.organizations.models import Organization from cvat.apps.events.utils import cache_deleted class SafeCharField(models.CharField): @@ -316,18 +315,23 @@ class Image(models.Model): class Meta: default_permissions = () -class Project(models.Model): +class TimestampedModel(models.Model): + created_date = models.DateTimeField(auto_now_add=True) + updated_date = models.DateTimeField(auto_now=True) + + class Meta: + abstract = True + +class Project(TimestampedModel): name = SafeCharField(max_length=256) owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL, related_name="+") assignee = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL, related_name="+") bug_tracker = models.CharField(max_length=2000, blank=True, default="") - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) status = models.CharField(max_length=32, choices=StatusChoice.choices(), default=StatusChoice.ANNOTATION) - organization = models.ForeignKey(Organization, null=True, default=None, + organization = models.ForeignKey('organizations.Organization', null=True, default=None, blank=True, on_delete=models.SET_NULL, related_name="projects") source_storage = models.ForeignKey('Storage', null=True, default=None, blank=True, on_delete=models.SET_NULL, related_name='+') @@ -383,7 +387,7 @@ def with_job_summary(self): ) ) -class Task(models.Model): +class Task(TimestampedModel): objects = TaskQuerySet.as_manager() project = models.ForeignKey(Project, on_delete=models.CASCADE, @@ -396,8 +400,6 @@ class Task(models.Model): assignee = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL, related_name="assignees") bug_tracker = models.CharField(max_length=2000, blank=True, default="") - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) overlap = models.PositiveIntegerField(null=True) # Zero means that there are no limits (default) # Note that the files can be split into jobs in a custom way in this case @@ -407,7 +409,7 @@ class Task(models.Model): data = models.ForeignKey(Data, on_delete=models.CASCADE, null=True, related_name="tasks") dimension = models.CharField(max_length=2, choices=DimensionType.choices(), default=DimensionType.DIM_2D) subset = models.CharField(max_length=64, blank=True, default="") - organization = models.ForeignKey(Organization, null=True, default=None, + organization = models.ForeignKey('organizations.Organization', null=True, default=None, blank=True, on_delete=models.SET_NULL, related_name="tasks") source_storage = models.ForeignKey('Storage', null=True, default=None, blank=True, on_delete=models.SET_NULL, related_name='+') @@ -656,15 +658,12 @@ def _validate_constraints(self, obj: Dict[str, Any]): -class Job(models.Model): +class Job(TimestampedModel): objects = JobQuerySet.as_manager() segment = models.ForeignKey(Segment, on_delete=models.CASCADE) assignee = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) - # TODO: it has to be deleted in Job, Task, Project and replaced by (stage, state) # The stage field cannot be changed by an assignee, but state field can be. For # now status is read only and it will be updated by (stage, state). Thus we don't @@ -962,7 +961,7 @@ class Profile(models.Model): user = models.OneToOneField(User, on_delete=models.CASCADE) rating = models.FloatField(default=0.0) -class Issue(models.Model): +class Issue(TimestampedModel): frame = models.PositiveIntegerField() position = FloatArrayField() job = models.ForeignKey(Job, related_name='issues', on_delete=models.CASCADE) @@ -970,8 +969,6 @@ class Issue(models.Model): on_delete=models.SET_NULL) assignee = models.ForeignKey(User, null=True, blank=True, related_name='+', on_delete=models.SET_NULL) - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) resolved = models.BooleanField(default=False) def get_project_id(self): @@ -991,12 +988,10 @@ def get_job_id(self): return self.job_id -class Comment(models.Model): +class Comment(TimestampedModel): issue = models.ForeignKey(Issue, related_name='comments', on_delete=models.CASCADE) owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) message = models.TextField(default='') - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) def get_project_id(self): return self.issue.get_project_id() @@ -1072,7 +1067,7 @@ def __str__(self): def list(cls): return [i.value for i in cls] -class CloudStorage(models.Model): +class CloudStorage(TimestampedModel): # restrictions: # AWS bucket name, Azure container name - 63, Google bucket name - 63 without dots and 222 with dots # https://cloud.google.com/storage/docs/naming-buckets#requirements @@ -1090,13 +1085,11 @@ class CloudStorage(models.Model): display_name = models.CharField(max_length=63) owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL, related_name="cloud_storages") - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) credentials = models.CharField(max_length=1024, null=True, blank=True) credentials_type = models.CharField(max_length=29, choices=CredentialsTypeChoice.choices())#auth_type specific_attributes = models.CharField(max_length=1024, blank=True) description = models.TextField(blank=True) - organization = models.ForeignKey(Organization, null=True, default=None, + organization = models.ForeignKey('organizations.Organization', null=True, default=None, blank=True, on_delete=models.SET_NULL, related_name="cloudstorages") class Meta: @@ -1130,12 +1123,10 @@ class Storage(models.Model): class Meta: default_permissions = () -class AnnotationGuide(models.Model): +class AnnotationGuide(TimestampedModel): task = models.OneToOneField(Task, null=True, blank=True, on_delete=models.CASCADE, related_name="annotation_guide") project = models.OneToOneField(Project, null=True, blank=True, on_delete=models.CASCADE, related_name="annotation_guide") markdown = models.TextField(blank=True, default='') - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) is_public = models.BooleanField(default=False) @property diff --git a/cvat/apps/events/signals.py b/cvat/apps/events/signals.py index 54d14b6be75..31cdd27f9c5 100644 --- a/cvat/apps/events/signals.py +++ b/cvat/apps/events/signals.py @@ -7,7 +7,6 @@ from django.core.exceptions import ObjectDoesNotExist from cvat.apps.engine.models import ( - Organization, Project, Task, Job, @@ -17,6 +16,7 @@ Comment, Label, ) +from cvat.apps.organizations.models import Organization from .handlers import handle_update, handle_create, handle_delete from .event import EventScopeChoice, event_scope From 7a219b49d90b7378875de01daf8929cb66c92c9c Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Wed, 7 Feb 2024 18:22:54 +0200 Subject: [PATCH 2/4] Don't save all fields of a model when bumping the update date Instead, use a specialized method that only changes the date. This provides the following advantages: * Prevents the date updates from overwriting updates to other fields that may happen at the same time. * Possibly makes the updates more efficient due to fewer columns being updated (I have no data on this, though). * Makes it easier to recognize the purpose of the query when analyzing logs/live statistics. * Makes the purpose of the code more obvious. The name of the method references the standard POSIX touch utility. --- cvat/apps/dataset_manager/task.py | 7 ++----- cvat/apps/engine/models.py | 3 +++ cvat/apps/engine/signals.py | 7 +++---- cvat/apps/engine/views.py | 17 ++++++++--------- cvat/apps/organizations/models.py | 6 +++--- cvat/apps/webhooks/models.py | 12 +++--------- cvat/schema.yml | 16 ++++++++-------- 7 files changed, 30 insertions(+), 38 deletions(-) diff --git a/cvat/apps/dataset_manager/task.py b/cvat/apps/dataset_manager/task.py index cbd21256a61..5dbb1ba8b8c 100644 --- a/cvat/apps/dataset_manager/task.py +++ b/cvat/apps/dataset_manager/task.py @@ -12,7 +12,6 @@ from django.db import transaction from django.db.models.query import Prefetch -from django.utils import timezone from rest_framework.exceptions import ValidationError from cvat.apps.engine import models, serializers @@ -389,9 +388,7 @@ def _save_tags_to_db(self, tags): self.ir_data.tags = tags def _set_updated_date(self): - db_task = self.db_job.segment.task - db_task.updated_date = timezone.now() - db_task.save() + self.db_job.segment.task.touch() def _save_to_db(self, data): self.reset() @@ -404,7 +401,7 @@ def _save_to_db(self, data): def _create(self, data): if self._save_to_db(data): self._set_updated_date() - self.db_job.save() + self.db_job.touch() def create(self, data): self._create(data) diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 8029a032c50..08147dfc751 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -322,6 +322,9 @@ class TimestampedModel(models.Model): class Meta: abstract = True + def touch(self) -> None: + self.save(update_fields=["updated_date"]) + class Project(TimestampedModel): name = SafeCharField(max_length=256) owner = models.ForeignKey(User, null=True, blank=True, diff --git a/cvat/apps/engine/signals.py b/cvat/apps/engine/signals.py index 570325c5a58..297baec9488 100644 --- a/cvat/apps/engine/signals.py +++ b/cvat/apps/engine/signals.py @@ -34,7 +34,7 @@ def __save_job_handler(instance, created, **kwargs): if status != db_task.status: db_task.status = status - db_task.save() + db_task.save(update_fields=["status", "updated_date"]) @receiver(post_save, sender=User, dispatch_uid=__name__ + ".save_user_handler") @@ -66,9 +66,8 @@ def __delete_task_handler(instance, **kwargs): instance.data.delete() try: - if instance.project: # update project - db_project = instance.project - db_project.save() + if db_project := instance.project: # update project + db_project.touch() except Project.DoesNotExist: pass # probably the project has been deleted diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 5d33c0430cc..63c12c98eac 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -894,9 +894,9 @@ def perform_update(self, serializer): updated_instance = serializer.instance if instance.project: - instance.project.save() + instance.project.touch() if updated_instance.project: - updated_instance.project.save() + updated_instance.project.touch() @transaction.atomic def perform_create(self, serializer, **kwargs): @@ -905,9 +905,8 @@ def perform_create(self, serializer, **kwargs): organization=self.request.iam_context['organization'] ) - if serializer.instance.project: - db_project = serializer.instance.project - db_project.save() + if db_project := serializer.instance.project: + db_project.touch() assert serializer.instance.organization == db_project.organization # Required for the extra summary information added in the queryset @@ -1944,9 +1943,9 @@ def metadata(self, request, pk): db_data.deleted_frames, )) db_data = serializer.save() - db_job.segment.task.save() + db_job.segment.task.touch() if db_job.segment.task.project: - db_job.segment.task.project.save() + db_job.segment.task.project.touch() if hasattr(db_data, 'video'): media = [db_data.video] @@ -2285,10 +2284,10 @@ def perform_destroy(self, instance: models.Label): code=status.HTTP_400_BAD_REQUEST) if project := instance.project: - project.save(update_fields=['updated_date']) + project.touch() ProjectWriteSerializer(project).update_child_objects_on_labels_update(project) elif task := instance.task: - task.save(update_fields=['updated_date']) + task.touch() TaskWriteSerializer(task).update_child_objects_on_labels_update(task) return super().perform_destroy(instance) diff --git a/cvat/apps/organizations/models.py b/cvat/apps/organizations/models.py index 49975bd1466..3da77bafbeb 100644 --- a/cvat/apps/organizations/models.py +++ b/cvat/apps/organizations/models.py @@ -15,12 +15,12 @@ from django.core.exceptions import ImproperlyConfigured from django.utils import timezone -class Organization(models.Model): +from cvat.apps.engine.models import TimestampedModel + +class Organization(TimestampedModel): slug = models.SlugField(max_length=16, blank=False, unique=True) name = models.CharField(max_length=64, blank=True) description = models.TextField(blank=True) - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) contact = models.JSONField(blank=True, default=dict) owner = models.ForeignKey(get_user_model(), null=True, diff --git a/cvat/apps/webhooks/models.py b/cvat/apps/webhooks/models.py index 69b0311dcd4..104faccd60a 100644 --- a/cvat/apps/webhooks/models.py +++ b/cvat/apps/webhooks/models.py @@ -7,7 +7,7 @@ from django.contrib.auth.models import User from django.db import models -from cvat.apps.engine.models import Project +from cvat.apps.engine.models import Project, TimestampedModel from cvat.apps.organizations.models import Organization @@ -34,7 +34,7 @@ def __str__(self): return self.value -class Webhook(models.Model): +class Webhook(TimestampedModel): target_url = models.URLField(max_length=8192) description = models.CharField(max_length=128, default="", blank=True) @@ -50,9 +50,6 @@ class Webhook(models.Model): is_active = models.BooleanField(default=True) enable_ssl = models.BooleanField(default=True) - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) - owner = models.ForeignKey( User, null=True, blank=True, on_delete=models.SET_NULL, related_name="+" ) @@ -82,7 +79,7 @@ class Meta: ] -class WebhookDelivery(models.Model): +class WebhookDelivery(TimestampedModel): webhook = models.ForeignKey( Webhook, on_delete=models.CASCADE, related_name="deliveries" ) @@ -91,9 +88,6 @@ class WebhookDelivery(models.Model): status_code = models.PositiveIntegerField(null=True, default=None) redelivery = models.BooleanField(default=False) - created_date = models.DateTimeField(auto_now_add=True) - updated_date = models.DateTimeField(auto_now=True) - changed_fields = models.CharField(max_length=4096, default="") request = models.JSONField(default=dict) diff --git a/cvat/schema.yml b/cvat/schema.yml index f8ee3e42c3a..9a40fb547a0 100644 --- a/cvat/schema.yml +++ b/cvat/schema.yml @@ -6521,14 +6521,6 @@ components: type: string maxLength: 1024 default: [] - provider_type: - $ref: '#/components/schemas/ProviderTypeEnum' - resource: - type: string - maxLength: 222 - display_name: - type: string - maxLength: 63 created_date: type: string format: date-time @@ -6537,6 +6529,14 @@ components: type: string format: date-time readOnly: true + provider_type: + $ref: '#/components/schemas/ProviderTypeEnum' + resource: + type: string + maxLength: 222 + display_name: + type: string + maxLength: 63 credentials_type: $ref: '#/components/schemas/CredentialsTypeEnum' specific_attributes: From 03a48723a5a82972e4265fbc6b522d829d47eee3 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 15 Feb 2024 14:29:59 +0200 Subject: [PATCH 3/4] JobAnnotation._set_updated_date: touch the job and not just the task This seems to be in line with the intention of the method. This also fixes the bug where the job wasn't touched if annotations were cleared but new ones weren't created. --- cvat/apps/dataset_manager/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat/apps/dataset_manager/task.py b/cvat/apps/dataset_manager/task.py index 5dbb1ba8b8c..9b195c26bd0 100644 --- a/cvat/apps/dataset_manager/task.py +++ b/cvat/apps/dataset_manager/task.py @@ -389,6 +389,7 @@ def _save_tags_to_db(self, tags): def _set_updated_date(self): self.db_job.segment.task.touch() + self.db_job.touch() def _save_to_db(self, data): self.reset() @@ -401,7 +402,6 @@ def _save_to_db(self, data): def _create(self, data): if self._save_to_db(data): self._set_updated_date() - self.db_job.touch() def create(self, data): self._create(data) From 389d7c136c250f65ebc89d86d661834afaced06e Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 15 Feb 2024 14:34:15 +0200 Subject: [PATCH 4/4] Avoid touching the same project twice in `TaskViewSet.perform_update` --- cvat/apps/engine/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 63c12c98eac..306ed74efca 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -895,7 +895,7 @@ def perform_update(self, serializer): if instance.project: instance.project.touch() - if updated_instance.project: + if updated_instance.project and updated_instance.project != instance.project: updated_instance.project.touch() @transaction.atomic