Fix /api/tasks leads to 500 server error

CHANGELOG.md

@@ -70,6 +70,7 @@ Tracks can be exported/imported to/from Datumaro and Sly Pointcloud formats (<ht
 - Clean up disk space after a project is removed (<https://github.com/opencv/cvat/pull/5632>)
 - \[Server API\] Various errors in the generated schema (<https://github.com/opencv/cvat/pull/5575>)
 - SiamMask and TransT serverless functions (<https://github.com/opencv/cvat/pull/5658>)
+- Сreating a project or task with the same labels (<https://github.com/opencv/cvat/pull/5700>)
 - \[Server API\] Ability to rename label to an existing name (<https://github.com/opencv/cvat/pull/5662>)
 
 ### Security

cvat/apps/engine/migrations/0064_delete_wrong_labels.py

@@ -0,0 +1,39 @@
+import os
+
+from django.db import migrations
+from cvat.apps.engine.log import get_migration_logger
+
+def delete_wrong_labels(apps, schema_editor):
+    migration_name = os.path.splitext(os.path.basename(__file__))[0]
+    with get_migration_logger(migration_name) as log:
+        log.info('\nDeleting skeleton Labels without skeletons...')
+
+        Label = apps.get_model('engine', 'Label')
+        for label in Label.objects.all():
+            if label.type == "skeleton" and not hasattr(label, "skeleton"):
+                label.delete()
+
+        log.info('\nDeleting duplicate Labels...')
+        for name, parent, project, task in Label.objects.values_list("name", "parent", "project", "task").distinct():
+            duplicate_labels = Label.objects.filter(name=name, parent=parent, project=project).values_list('id', "parent")
+            if task is not None:
+                duplicate_labels = Label.objects.filter(name=name, parent=parent, task=task).values_list('id', "parent")
+
+            if len(duplicate_labels) > 1:
+                label = duplicate_labels[0]
+                if label[1] is not None:
+                    Label.objects.get(pk=label[1]).delete()
+                else:
+                    Label.objects.get(pk=label[0]).delete()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('engine', '0063_delete_jobcommit'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=delete_wrong_labels
+        ),
+    ]

cvat/apps/engine/migrations/0065_auto_20230221_0931.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.18 on 2023-02-21 09:31
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('engine', '0064_delete_wrong_labels'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='label',
+            unique_together=set(),
+        ),
+        migrations.AddConstraint(
+            model_name='label',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True), ('task__isnull', True)), fields=('project', 'name'), name='project_name_unique'),
+        ),
+        migrations.AddConstraint(
+            model_name='label',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True), ('project__isnull', True)), fields=('task', 'name'), name='task_name_unique'),
+        ),
+        migrations.AddConstraint(
+            model_name='label',
+            constraint=models.UniqueConstraint(condition=models.Q(('task__isnull', True)), fields=('project', 'name', 'parent'), name='project_name_parent_unique'),
+        ),
+        migrations.AddConstraint(
+            model_name='label',
+            constraint=models.UniqueConstraint(condition=models.Q(('project__isnull', True)), fields=('task', 'name', 'parent'), name='task_name_parent_unique'),
+        ),
+    ]

cvat/apps/engine/models.py

@@ -12,13 +12,16 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.files.storage import FileSystemStorage
-from django.db import models
+from django.db import IntegrityError, models
 from django.db.models.fields import FloatField
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import extend_schema_field
+from rest_framework import exceptions
+
 from cvat.apps.engine.utils import parse_specific_attributes
 from cvat.apps.organizations.models import Organization
 
+
 class SafeCharField(models.CharField):
     def get_prep_value(self, value):
         value = super().get_prep_value(value)
@@ -502,10 +505,6 @@ def get_labels(self):
     class Meta:
         default_permissions = ()
 
-
-class InvalidLabel(ValueError):
-    pass
-
 class Label(models.Model):
     task = models.ForeignKey(Task, null=True, blank=True, on_delete=models.CASCADE)
     project = models.ForeignKey(Project, null=True, blank=True, on_delete=models.CASCADE)
@@ -520,28 +519,48 @@ def __str__(self):
     def has_parent_label(self):
         return bool(self.parent)
 
-    def _check_save_constraints(self) -> None:
-        # NOTE: constraints don't work for some reason
-        # https://github.com/opencv/cvat/pull/5700#discussion_r1112276036
-        # This method is not 100% reliable because of possible race conditions
-        # but it should work in relevant cases.
-
-        parent_entity = self.project or self.task
+    def save(self, force_insert=False, force_update=False, using=None,
+             update_fields=None):
+        try:
+            super().save(force_insert, force_update, using, update_fields)
+        except ValueError as exc:
+            raise exceptions.ValidationError(str(exc)) from exc
+        except IntegrityError:
+            raise exceptions.ValidationError("All label names must be unique")
 
-        # Check for possible labels name duplicates in case of saving the new label
-        existing_labels: models.QuerySet = parent_entity.get_labels()
-        if self.id:
-            existing_labels = existing_labels.exclude(id=self.id)
-        if existing_labels.filter(name=self.name).count():
-            raise InvalidLabel(f"Label '{self.name}' already exists")
-
-    def save(self, *args, **kwargs) -> None:
-        self._check_save_constraints()
-        return super().save(*args, **kwargs)
+    @classmethod
+    def create(cls, **kwargs):
+        try:
+            return cls.objects.create(**kwargs)
+        except ValueError as exc:
+            raise exceptions.ValidationError(str(exc)) from exc
+        except IntegrityError:
+            raise exceptions.ValidationError("All label names must be unique")
 
     class Meta:
         default_permissions = ()
-        unique_together = ('task', 'name', 'parent')
+        constraints = [
+            models.UniqueConstraint(
+                name='project_name_unique',
+                fields=('project', 'name'),
+                condition=models.Q(task__isnull=True, parent__isnull=True)
+            ),
+            models.UniqueConstraint(
+                name='task_name_unique',
+                fields=('task', 'name'),
+                condition=models.Q(project__isnull=True, parent__isnull=True)
+            ),
+            models.UniqueConstraint(
+                name='project_name_parent_unique',
+                fields=('project', 'name', 'parent'),
+                condition=models.Q(task__isnull=True)
+            ),
+            models.UniqueConstraint(
+                name='task_name_parent_unique',
+                fields=('task', 'name', 'parent'),
+                condition=models.Q(project__isnull=True)
+            )
+        ]
 
 class Skeleton(models.Model):
     root = models.OneToOneField(Label, on_delete=models.CASCADE)

cvat/apps/engine/serializers.py

@@ -287,7 +287,7 @@ def update_label(
 
             logger.info("Label id {} ({}) was updated".format(db_label.id, db_label.name))
         else:
-            db_label = models.Label.objects.create(
+            db_label = models.Label.create(
                 name=validated_data.get('name'),
                 type=validated_data.get('type'),
                 parent=parent_label,
@@ -309,10 +309,7 @@ def update_label(
         else:
             db_label.color = validated_data.get('color', db_label.color)
 
-        try:
-            db_label.save()
-        except models.InvalidLabel as exc:
-            raise exceptions.ValidationError(str(exc)) from exc
+        db_label.save()
 
         for attr in attributes:
             (db_attr, created) = models.AttributeSpec.objects.get_or_create(
@@ -358,7 +355,7 @@ def create_labels(cls,
 
             sublabels = label.pop('sublabels', [])
             svg = label.pop('svg', '')
-            db_label = models.Label.objects.create(**label, **parent_info, parent=parent_label)
+            db_label = models.Label.create(**label, **parent_info, parent=parent_label)
             logger.info(
                 f'label:create Label id:{db_label.id} for spec:{label} '
                 f'with sublabels:{sublabels}, parent_label:{parent_label}'
@@ -990,20 +987,9 @@ def validate(self, attrs):
             for label, sublabels in new_sublabel_names.items():
                 if sublabels != target_project_sublabel_names.get(label):
                     raise serializers.ValidationError('All task or project label names must be mapped to the target project')
-        else:
-            if 'label_set' in attrs.keys():
-                # FIXME: doesn't work for renaming just a single label
-                label_names = [
-                    label['name']
-                    for label in attrs.get('label_set')
-                    if not label.get('deleted')
-                ]
-                if len(label_names) != len(set(label_names)):
-                    raise serializers.ValidationError('All label names must be unique for the task')
 
         return attrs
 
-
 class ProjectReadSerializer(serializers.ModelSerializer):
     owner = BasicUserSerializer(required=False, read_only=True)
     assignee = BasicUserSerializer(allow_null=True, required=False, read_only=True)
@@ -1090,18 +1076,6 @@ def update(self, instance, validated_data):
         instance.save()
         return instance
 
-    def validate_labels(self, value):
-        if value:
-            # FIXME: doesn't work for renaming just a single label
-            label_names = [
-                label['name']
-                for label in value
-                if not label.get('deleted')
-            ]
-            if len(label_names) != len(set(label_names)):
-                raise serializers.ValidationError('All label names must be unique for the project')
-        return value
-
 class AboutSerializer(serializers.Serializer):
     name = serializers.CharField(max_length=128)
     description = serializers.CharField(max_length=2048)

tests/python/rest_api/test_projects.py

@@ -21,7 +21,14 @@
 from deepdiff import DeepDiff
 from PIL import Image
 
-from shared.utils.config import BASE_URL, USER_PASS, get_method, make_api_client, patch_method
+from shared.utils.config import (
+    BASE_URL,
+    USER_PASS,
+    get_method,
+    make_api_client,
+    patch_method,
+    post_method,
+)
 
 from .utils import CollectionSimpleFilterTestBase, export_dataset
 
@@ -390,6 +397,30 @@ def _create_org(cls, api_client: ApiClient, members: Optional[Dict[str, str]] =
 
         return org
 
+    def test_cannot_create_project_with_same_labels(self, admin_user):
+        project_spec = {
+            "name": "test cannot create project with same labels",
+            "labels": [{"name": "l1"}, {"name": "l1"}],
+        }
+        response = post_method(admin_user, "/projects", project_spec)
+        assert response.status_code == HTTPStatus.BAD_REQUEST
+
+        response = get_method(admin_user, "/projects")
+        assert response.status_code == HTTPStatus.OK
+
+    def test_cannot_create_project_with_same_skeleton_sublabels(self, admin_user):
+        project_spec = {
+            "name": "test cannot create project with same skeleton sublabels",
+            "labels": [
+                {"name": "s1", "type": "skeleton", "sublabels": [{"name": "1"}, {"name": "1"}]}
+            ],
+        }
+        response = post_method(admin_user, "/projects", project_spec)
+        assert response.status_code == HTTPStatus.BAD_REQUEST
+
+        response = get_method(admin_user, "/projects")
+        assert response.status_code == HTTPStatus.OK
+
 
 def _check_cvat_for_video_project_annotations_meta(content, values_to_be_checked):
     document = ET.fromstring(content)

tests/python/rest_api/test_tasks.py

@@ -27,7 +27,14 @@
 
 import shared.utils.s3 as s3
 from shared.fixtures.init import get_server_image_tag
-from shared.utils.config import BASE_URL, USER_PASS, get_method, make_api_client, patch_method
+from shared.utils.config import (
+    BASE_URL,
+    USER_PASS,
+    get_method,
+    make_api_client,
+    patch_method,
+    post_method,
+)
 from shared.utils.helpers import generate_image_files
 
 from .utils import CollectionSimpleFilterTestBase, export_dataset
@@ -929,6 +936,30 @@ def test_can_specify_file_job_mapping(self):
 
                 start_frame = stop_frame + 1
 
+    def test_cannot_create_task_with_same_labels(self):
+        task_spec = {
+            "name": "test cannot create task with same labels",
+            "labels": [{"name": "l1"}, {"name": "l1"}],
+        }
+        response = post_method(self._USERNAME, "/tasks", task_spec)
+        assert response.status_code == HTTPStatus.BAD_REQUEST
+
+        response = get_method(self._USERNAME, "/tasks")
+        assert response.status_code == HTTPStatus.OK
+
+    def test_cannot_create_task_with_same_skeleton_sublabels(self):
+        task_spec = {
+            "name": "test cannot create task with same skeleton sublabels",
+            "labels": [
+                {"name": "s1", "type": "skeleton", "sublabels": [{"name": "1"}, {"name": "1"}]}
+            ],
+        }
+        response = post_method(self._USERNAME, "/tasks", task_spec)
+        assert response.status_code == HTTPStatus.BAD_REQUEST
+
+        response = get_method(self._USERNAME, "/tasks")
+        assert response.status_code == HTTPStatus.OK
+
 
 @pytest.mark.usefixtures("restore_db_per_function")
 class TestPatchTaskLabel: