Reset source/target storage if related cloud storage has been deleted

changelog.d/20231117_123912_maria_fix_cs_deletion_handling.md

@@ -0,0 +1,4 @@
+### Fixed
+
+- Reset source/target storage if related cloud storage has been deleted
+  (<https://github.com/opencv/cvat/pull/6801>)

cvat/apps/engine/location.py

@@ -3,9 +3,9 @@
 # SPDX-License-Identifier: MIT
 
 from enum import Enum
-from typing import Any, Dict
+from typing import Any, Dict, Union
 
-from cvat.apps.engine.models import Location, Job
+from cvat.apps.engine.models import Location, Project, Task, Job
 
 class StorageType(str, Enum):
     TARGET = 'target_storage'
@@ -14,7 +14,11 @@ class StorageType(str, Enum):
     def __str__(self):
         return self.value
 
-def get_location_configuration(obj, field_name: str, use_settings: bool = False) -> Dict[str, Any]:
+def get_location_configuration(
+    obj: Union[Project, Task, Job, Dict],
+    field_name: str,
+    use_settings: bool = False,
+) -> Dict[str, Any]:
     location_conf = {
         "is_default": use_settings
     }
@@ -25,20 +29,12 @@ def get_location_configuration(obj, field_name: str, use_settings: bool = False)
             location_conf['location'] = Location.LOCAL
         else:
             location_conf['location'] = storage.location
-            sid = storage.cloud_storage_id
-            if sid:
-                location_conf['storage_id'] = sid
+            if cloud_storage_id := storage.cloud_storage_id:
+                location_conf['storage_id'] = cloud_storage_id
     else:
         # obj is query_params
-        # FIXME when ui part will be done
         location_conf['location'] = obj.get('location', Location.LOCAL)
-        # try:
-        #     location_conf['location'] = obj['location']
-        # except KeyError:
-        #     raise ValidationError("Custom settings were selected but no location was specified")
-
-        sid = obj.get('cloud_storage_id')
-        if sid:
-            location_conf['storage_id'] = int(sid)
+        if cloud_storage_id := obj.get('cloud_storage_id'):
+            location_conf['storage_id'] = int(cloud_storage_id)
 
     return location_conf

cvat/apps/engine/migrations/0076_remove_storages_that_refer_to_deleted_cloud_storages.py

@@ -0,0 +1,26 @@
+# Generated by Django 4.2.6 on 2023-11-17 10:10
+
+from django.db import migrations, models
+from cvat.apps.engine.models import Location
+
+
+def manually_remove_outdated_relations(apps, schema_editor):
+    Storage = apps.get_model("engine", "Storage")
+    CloudStorage = apps.get_model("engine", "CloudStorage")
+    Storage.objects.filter(location=Location.LOCAL, cloud_storage_id__isnull=False).update(cloud_storage_id=None)
+    Storage.objects.filter(
+        ~models.Exists(
+            CloudStorage.objects.filter(pk=models.OuterRef("cloud_storage_id"))
+        ),
+        location=Location.CLOUD_STORAGE,
+    ).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("engine", "0075_annotationguide_is_public"),
+    ]
+
+    operations = [
+        migrations.RunPython(manually_remove_outdated_relations),
+    ]

cvat/apps/engine/migrations/0077_auto_20231121_1952.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.6 on 2023-11-21 19:52
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("engine", "0076_remove_storages_that_refer_to_deleted_cloud_storages"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="storage",
+            name="cloud_storage_id",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="+",
+                to="engine.cloudstorage",
+            ),
+        ),
+        migrations.RenameField(
+            model_name="storage",
+            old_name="cloud_storage_id",
+            new_name="cloud_storage",
+        ),
+    ]

cvat/apps/engine/models.py

@@ -1114,7 +1114,12 @@ def has_at_least_one_manifest(self) -> bool:
 
 class Storage(models.Model):
     location = models.CharField(max_length=16, choices=Location.choices(), default=Location.LOCAL)
-    cloud_storage_id = models.IntegerField(null=True, blank=True, default=None)
+    cloud_storage = models.ForeignKey(
+        CloudStorage,
+        on_delete=models.CASCADE,
+        null=True,
+        related_name='+',
+    )
 
     class Meta:
         default_permissions = ()

cvat/apps/engine/serializers.py

@@ -1030,6 +1030,8 @@ def _create_files(self, instance, files):
                 )
 
 class StorageSerializer(serializers.ModelSerializer):
+    cloud_storage_id = serializers.IntegerField(required=False, allow_null=True)
+
     class Meta:
         model = models.Storage
         fields = ('id', 'location', 'cloud_storage_id')
@@ -1980,63 +1982,81 @@ class Meta:
         read_only_fields = ('path',)
 
 
-def _update_related_storages(instance, validated_data):
-    for storage in ('source_storage', 'target_storage'):
-        new_conf = validated_data.pop(storage, None)
+def _update_related_storages(
+    instance: Union[models.Project, models.Task],
+    validated_data: Dict[str, Any],
+) -> None:
+    for storage_type in ('source_storage', 'target_storage'):
+        new_conf = validated_data.pop(storage_type, None)
 
         if not new_conf:
             continue
 
-        cloud_storage_id = new_conf.get('cloud_storage_id')
-        if cloud_storage_id:
-            _validate_existence_of_cloud_storage(cloud_storage_id)
+        new_cloud_storage_id = new_conf.get('cloud_storage_id')
+        new_location = new_conf.get('location')
 
         # storage_instance maybe None
-        storage_instance = getattr(instance, storage)
+        storage_instance = getattr(instance, storage_type)
+
+        if new_cloud_storage_id:
+            if new_location and new_location != models.Location.CLOUD_STORAGE:
+                raise serializers.ValidationError(
+                    f"It is not allowed to specify '{new_location}' location together with cloud storage id"
+                )
+            elif (
+                not new_location
+                and getattr(storage_instance, "location", None) != models.Location.CLOUD_STORAGE
+            ):
+                raise serializers.ValidationError(
+                    f"The configuration of {storage_type} is not full"
+                )
+
+            if not models.CloudStorage.objects.filter(id=new_cloud_storage_id).exists():
+                raise serializers.ValidationError(
+                    f"The specified cloud storage {new_cloud_storage_id} does not exist."
+                )
+        else:
+            if new_location == models.Location.CLOUD_STORAGE:
+                raise serializers.ValidationError(
+                    "Cloud storage was selected as location but its id was not specified"
+                )
+            elif (
+                not new_location
+                and getattr(storage_instance, "location", None) == models.Location.CLOUD_STORAGE
+                and "cloud_storage_id" in new_conf
+            ):
+                raise serializers.ValidationError(
+                    "It is not allowed to reset a cloud storage id without explicitly resetting a location"
+                )
+
         if not storage_instance:
             storage_instance = models.Storage(**new_conf)
             storage_instance.save()
-            setattr(instance, storage, storage_instance)
+            setattr(instance, storage_type, storage_instance)
             continue
 
-        new_location = new_conf.get('location')
         storage_instance.location = new_location or storage_instance.location
-        storage_instance.cloud_storage_id = new_conf.get('cloud_storage_id', \
-            storage_instance.cloud_storage_id if not new_location else None)
-
-        cloud_storage_id = storage_instance.cloud_storage_id
-        if cloud_storage_id:
-            try:
-                _ = models.CloudStorage.objects.get(id=cloud_storage_id)
-            except models.CloudStorage.DoesNotExist:
-                raise serializers.ValidationError(f'The specified cloud storage {cloud_storage_id} does not exist.')
-
+        storage_instance.cloud_storage_id = new_cloud_storage_id
         storage_instance.save()
 
-def _configure_related_storages(validated_data):
-
+def _configure_related_storages(validated_data: Dict[str, Any]) -> Dict[str, Optional[models.Storage]]:
     storages = {
         'source_storage': None,
         'target_storage': None,
     }
 
     for i in storages:
-        storage_conf = validated_data.get(i)
-        if storage_conf:
-            cloud_storage_id = storage_conf.get('cloud_storage_id')
-            if cloud_storage_id:
-                _validate_existence_of_cloud_storage(cloud_storage_id)
+        if storage_conf := validated_data.get(i):
+            if (
+                (cloud_storage_id := storage_conf.get('cloud_storage_id')) and
+                not models.CloudStorage.objects.filter(id=cloud_storage_id).exists()
+            ):
+                raise serializers.ValidationError(f'The specified cloud storage {cloud_storage_id} does not exist.')
             storage_instance = models.Storage(**storage_conf)
             storage_instance.save()
             storages[i] = storage_instance
     return storages
 
-def _validate_existence_of_cloud_storage(cloud_storage_id):
-    try:
-        _ = models.CloudStorage.objects.get(id=cloud_storage_id)
-    except models.CloudStorage.DoesNotExist:
-        raise serializers.ValidationError(f'The specified cloud storage {cloud_storage_id} does not exist.')
-
 class AssetReadSerializer(WriteOnceMixin, serializers.ModelSerializer):
     filename = serializers.CharField(required=True, max_length=1024)
     owner = BasicUserSerializer(required=False)

cvat/schema.yml

@@ -9385,8 +9385,6 @@ components:
           $ref: '#/components/schemas/LocationEnum'
         cloud_storage_id:
           type: integer
-          maximum: 2147483647
-          minimum: -2147483648
           nullable: true
     StorageMethod:
       enum:
@@ -9403,8 +9401,6 @@ components:
           $ref: '#/components/schemas/LocationEnum'
         cloud_storage_id:
           type: integer
-          maximum: 2147483647
-          minimum: -2147483648
           nullable: true
     StorageType:
       enum: