Reapply "Make move between collections a background job"

This reverts commit 4c6b1b3.
genialis · May 13, 2024 · 2ef54ca · 2ef54ca
1 parent 2471188
commit 2ef54ca
Show file tree

Hide file tree

Showing 7 changed files with 228 additions and 148 deletions.
diff --git a/docs/CHANGELOG.rst b/docs/CHANGELOG.rst
@@ -6,6 +6,15 @@ All notable changes to this project are documented in this file.
 This project adheres to `Semantic Versioning <http://semver.org/>`_.
 
 
+==========
+Unreleased
+==========
+
+Changed
+-------
+- **BACKWARD INCOMPATIBLE:** Make move between collections a background job
+
+
 ===================
 39.0.0 - 2024-05-09
 ===================

diff --git a/resolwe/flow/models/data.py b/resolwe/flow/models/data.py
@@ -217,15 +217,20 @@ def delete_background(self, contributor):
             BackgroundTaskType.DELETE, "Delete data", task_data, contributor
         )
 
-    @transaction.atomic
-    def move_to_collection(self, destination_collection):
-        """Move data objects to destination collection.
+    def move_to_collection(self, destination_collection, contributor):
+        """Move data objects to destination collection in the background.
 
         Note that this method will also copy tags and permissions
         of the destination collection to the data objects.
         """
-        for data in self:
-            data.move_to_collection(destination_collection)
+        task_data = {
+            "target_id": destination_collection.pk,
+            "data_ids": list(self.values_list("pk", flat=True)),
+            "entity_ids": [],
+        }
+        return start_background_task(
+            BackgroundTaskType.MOVE, "Delete data", task_data, contributor
+        )
 
     def annotate_sample_path(self, path, annotation_name, value_to_label=False):
         """Add annotation to the Entity QuerySet.

diff --git a/resolwe/flow/models/entity.py b/resolwe/flow/models/entity.py
@@ -45,11 +45,16 @@ def delete_background(self, contributor):
             BackgroundTaskType.DELETE, "Delete entities", task_data, contributor
         )
 
-    @transaction.atomic
-    def move_to_collection(self, destination_collection: Collection):
+    def move_to_collection(self, destination_collection: Collection, contributor):
         """Move entities to destination collection."""
-        for entity in self:
-            entity.move_to_collection(destination_collection)
+        task_data = {
+            "target_id": destination_collection.pk,
+            "data_ids": [],
+            "entity_ids": list(self.values_list("pk", flat=True)),
+        }
+        return start_background_task(
+            BackgroundTaskType.MOVE, "Move entities", task_data, contributor
+        )
 
     def annotate_all(self, add_labels: bool = False):
         """Annotate with all metadata on the entities.
@@ -272,7 +277,8 @@ def move_to_collection(self, collection: Collection):
         self.tags = collection.tags
         self.permission_group = collection.permission_group
         self.save(update_fields=["collection", "tags", "permission_group"])
-        self.data.move_to_collection(collection)
+        for data in self.data.all():
+            data.move_to_collection(collection)
 
     def invalid_annotation_fields(self, annotation_fields=None):
         """Get the Queryset of invalid annotation fields.

diff --git a/resolwe/flow/tests/test_api.py b/resolwe/flow/tests/test_api.py
@@ -334,7 +334,7 @@ def test_duplicate_entity(self):
         self.assertEqual(collection_without_perm.data.count(), 1)
 
 
-class TestDataViewSetCase(TestCase):
+class TestDataViewSetCaseMixin:
     def setUp(self):
         super().setUp()
         self.client = APIClient()
@@ -396,6 +396,109 @@ def setUp(self):
         self.proc.set_permission(Permission.VIEW, self.contributor)
         self.descriptor_schema.set_permission(Permission.VIEW, self.contributor)
 
+
+class TestDataViewSetCaseTransaction(TestDataViewSetCaseMixin, TransactionTestCase):
+    """Test background jobs."""
+
+    def test_move_to_collection(self):
+        data_in_entity = Data.objects.create(
+            contributor=self.contributor, process=self.proc
+        )
+        data_orphan = Data.objects.create(
+            contributor=self.contributor, process=self.proc
+        )
+        data_orphan.entity.data.remove(data_orphan)
+
+        collection_1 = Collection.objects.create(contributor=self.contributor)
+        collection_1.data.add(data_orphan)
+        collection_1.set_permission(Permission.EDIT, self.contributor)
+
+        collection_2 = Collection.objects.create(contributor=self.contributor)
+        collection_2.set_permission(Permission.EDIT, self.contributor)
+
+        # Assert preventing moving data in entity.
+        request = factory.post(
+            reverse("resolwe-api:data-move-to-collection"),
+            {
+                "ids": [data_in_entity.id],
+                "destination_collection": collection_2.id,
+            },
+            format="json",
+        )
+        force_authenticate(request, self.contributor)
+        response = self.move_to_collection_viewset(request)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        # Wait for the task to finish.
+        task = BackgroundTask.objects.get(pk=response.data["id"])
+        task.wait(final_statuses=["ER"])
+        task.refresh_from_db()
+        self.assertEqual(
+            task.output[0],
+            "If Data is in entity, you can only move it to another collection by moving entire entity.",
+        )
+
+        # Assert moving data not in entity.
+        request = factory.post(
+            reverse("resolwe-api:data-move-to-collection"),
+            {
+                "ids": [data_orphan.id],
+                "destination_collection": collection_2.id,
+            },
+            format="json",
+        )
+        self.assertEqual(collection_1.data.count(), 1)
+        self.assertEqual(collection_2.data.count(), 0)
+
+        force_authenticate(request, self.contributor)
+        response = self.move_to_collection_viewset(request)
+        task = BackgroundTask.objects.get(pk=response.data["id"])
+        task.wait()
+        self.assertEqual(collection_1.data.count(), 0)
+        self.assertEqual(collection_2.data.count(), 1)
+
+        # Assert preventing moving data if destination collection
+        # lacks permissions.
+        collection_1.set_permission(Permission.VIEW, self.contributor)
+        request = factory.post(
+            reverse("resolwe-api:data-move-to-collection"),
+            {
+                "ids": [data_orphan.id],
+                "destination_collection": collection_1.id,
+            },
+            format="json",
+        )
+        force_authenticate(request, self.contributor)
+        response = self.move_to_collection_viewset(request)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(
+            response.data["detail"],
+            "You do not have permission to perform this action.",
+        )
+
+        # It shouldn't be possible to move the data if you don't
+        # have edit permission on both collections.
+        collection_1.set_permission(Permission.EDIT, self.contributor)
+        collection_2.set_permission(Permission.VIEW, self.contributor)
+        request = factory.post(
+            reverse("resolwe-api:data-move-to-collection"),
+            {
+                "ids": [data_orphan.id],
+                "destination_collection": collection_1.id,
+            },
+            format="json",
+        )
+        force_authenticate(request, self.contributor)
+        response = self.move_to_collection_viewset(request)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(
+            response.data["detail"],
+            "You do not have permission to perform this action.",
+        )
+        self.assertEqual(collection_1.data.count(), 0)
+        self.assertEqual(collection_2.data.count(), 1)
+
+
+class TestDataViewSetCase(TestDataViewSetCaseMixin, TestCase):
     def test_restart(self):
         data = Data.objects.create(contributor=self.contributor, process=self.proc)
         data.status = Data.STATUS_DONE
@@ -774,99 +877,6 @@ def test_change_collection(self):
         data.refresh_from_db()
         self.assertEqual(data.tags, collection.tags)
 
-    def test_move_to_collection(self):
-        data_in_entity = Data.objects.create(
-            contributor=self.contributor, process=self.proc
-        )
-        data_orphan = Data.objects.create(
-            contributor=self.contributor, process=self.proc
-        )
-        data_orphan.entity.data.remove(data_orphan)
-
-        collection_1 = Collection.objects.create(contributor=self.contributor)
-        collection_1.data.add(data_orphan)
-        collection_1.set_permission(Permission.EDIT, self.contributor)
-
-        collection_2 = Collection.objects.create(contributor=self.contributor)
-        collection_2.set_permission(Permission.EDIT, self.contributor)
-
-        # Assert preventing moving data in entity.
-        request = factory.post(
-            reverse("resolwe-api:data-move-to-collection"),
-            {
-                "ids": [data_in_entity.id],
-                "destination_collection": collection_2.id,
-            },
-            format="json",
-        )
-        force_authenticate(request, self.contributor)
-        response = self.move_to_collection_viewset(request)
-
-        self.assertEqual(
-            response.data["error"],
-            "If Data is in entity, you can only move it to another collection by moving entire entity.",
-        )
-
-        # Assert moving data not in entity.
-        request = factory.post(
-            reverse("resolwe-api:data-move-to-collection"),
-            {
-                "ids": [data_orphan.id],
-                "destination_collection": collection_2.id,
-            },
-            format="json",
-        )
-
-        self.assertEqual(collection_1.data.count(), 1)
-        self.assertEqual(collection_2.data.count(), 0)
-
-        force_authenticate(request, self.contributor)
-        response = self.move_to_collection_viewset(request)
-
-        self.assertEqual(collection_1.data.count(), 0)
-        self.assertEqual(collection_2.data.count(), 1)
-
-        # Assert preventing moving data if destination collection
-        # lacks permissions.
-        collection_1.set_permission(Permission.VIEW, self.contributor)
-        request = factory.post(
-            reverse("resolwe-api:data-move-to-collection"),
-            {
-                "ids": [data_orphan.id],
-                "destination_collection": collection_1.id,
-            },
-            format="json",
-        )
-        force_authenticate(request, self.contributor)
-        response = self.move_to_collection_viewset(request)
-
-        self.assertEqual(
-            response.data["detail"],
-            "You do not have permission to perform this action.",
-        )
-
-        # It shouldn't be possible to move the data if you don't
-        # have edit permission on both collections.
-        collection_1.set_permission(Permission.EDIT, self.contributor)
-        collection_2.set_permission(Permission.VIEW, self.contributor)
-        request = factory.post(
-            reverse("resolwe-api:data-move-to-collection"),
-            {
-                "ids": [data_orphan.id],
-                "destination_collection": collection_1.id,
-            },
-            format="json",
-        )
-        force_authenticate(request, self.contributor)
-        response = self.move_to_collection_viewset(request)
-
-        self.assertEqual(
-            response.data["detail"],
-            "You do not have permission to perform this action.",
-        )
-        self.assertEqual(collection_1.data.count(), 0)
-        self.assertEqual(collection_2.data.count(), 1)
-
     def test_process_is_active(self):
         # Do not allow creating data of inactive processes
         Process.objects.filter(slug="test-process").update(is_active=False)
@@ -1361,6 +1371,7 @@ def test_bulk_delete(self):
         )
 
         # Anonymous request, no permission.
+        get_anonymous_user(False)
         response = client.post(
             request_path, data={"ids": [collection1.pk]}, format="json"
         )
@@ -1572,6 +1583,49 @@ def _create_data(self):
         )
 
 
+class EntityViewSetTestTransaction(EntityViewSetTestCommonMixin, TransactionTestCase):
+    """Test background task."""
+
+    def test_move_to_collection(self):
+        source_collection = Collection.objects.create(contributor=self.contributor)
+        source_collection.set_permission(Permission.EDIT, self.contributor)
+        entity = Entity.objects.create(
+            contributor=self.contributor, collection=source_collection
+        )
+        data = self._create_data()
+        data.entity = entity
+        data.collection = source_collection
+        data.save()
+
+        destination_collection = Collection.objects.create(contributor=self.contributor)
+        destination_collection.set_permission(Permission.EDIT, self.contributor)
+
+        request = factory.post(
+            reverse("resolwe-api:entity-move-to-collection"),
+            {
+                "ids": [entity.id],
+                "source_collection": source_collection.id,
+                "destination_collection": destination_collection.id,
+            },
+            format="json",
+        )
+        self.assertEqual(source_collection.entity_set.count(), 1)
+        self.assertEqual(source_collection.data.count(), 1)
+        self.assertEqual(destination_collection.entity_set.count(), 0)
+        self.assertEqual(destination_collection.data.count(), 0)
+
+        force_authenticate(request, self.contributor)
+        response = self.move_to_collection_viewset(request)
+        task = BackgroundTask.objects.get(pk=response.data["id"])
+        task.wait()
+
+        self.assertEqual(source_collection.entity_set.count(), 0)
+        self.assertEqual(source_collection.data.count(), 0)
+        self.assertEqual(destination_collection.entity_set.count(), 1)
+        self.assertEqual(destination_collection.entity_set.first().id, entity.id)
+        self.assertEqual(destination_collection.data.first().id, data.id)
+
+
 class EntityViewSetTest(EntityViewSetTestCommonMixin, TestCase):
     def test_prefetch(self):
         self.entity.delete()
@@ -1663,44 +1717,6 @@ def test_change_collection_to_none(self):
 
         self.assertIn("can only be moved to another container.", resp.data["error"])
 
-    def test_move_to_collection(self):
-        source_collection = Collection.objects.create(contributor=self.contributor)
-        source_collection.set_permission(Permission.EDIT, self.contributor)
-        entity = Entity.objects.create(
-            contributor=self.contributor, collection=source_collection
-        )
-        data = self._create_data()
-        data.entity = entity
-        data.collection = source_collection
-        data.save()
-
-        destination_collection = Collection.objects.create(contributor=self.contributor)
-        destination_collection.set_permission(Permission.EDIT, self.contributor)
-
-        request = factory.post(
-            reverse("resolwe-api:entity-move-to-collection"),
-            {
-                "ids": [entity.id],
-                "source_collection": source_collection.id,
-                "destination_collection": destination_collection.id,
-            },
-            format="json",
-        )
-        force_authenticate(request, self.contributor)
-
-        self.assertEqual(source_collection.entity_set.count(), 1)
-        self.assertEqual(source_collection.data.count(), 1)
-        self.assertEqual(destination_collection.entity_set.count(), 0)
-        self.assertEqual(destination_collection.data.count(), 0)
-
-        self.move_to_collection_viewset(request)
-
-        self.assertEqual(source_collection.entity_set.count(), 0)
-        self.assertEqual(source_collection.data.count(), 0)
-        self.assertEqual(destination_collection.entity_set.count(), 1)
-        self.assertEqual(destination_collection.entity_set.first().id, entity.id)
-        self.assertEqual(destination_collection.data.first().id, data.id)
-
     def test_duplicate_not_auth(self):
         request = factory.post(reverse("resolwe-api:entity-duplicate"), format="json")
         response = self.duplicate_viewset(request)