fixup! 🐛(backend) race condition create doc

Author: AntoLC

src/backend/core/api/viewsets.py

@@ -573,6 +573,8 @@ def retrieve(self, request, *args, **kwargs):
     @transaction.atomic
     def perform_create(self, serializer):
         """Set the current user as creator and owner of the newly created object."""
+
+        # locks the table to ensure safe concurrent access
         with connection.cursor() as cursor:
             cursor.execute(
                 f'LOCK TABLE "{models.Document._meta.db_table}" '  # noqa: SLF001
@@ -637,10 +639,19 @@ def trashbin(self, request, *args, **kwargs):
         permission_classes=[],
         url_path="create-for-owner",
     )
+    @transaction.atomic
     def create_for_owner(self, request):
         """
         Create a document on behalf of a specified owner (pre-existing user or invited).
         """
+
+        # locks the table to ensure safe concurrent access
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f'LOCK TABLE "{models.Document._meta.db_table}" '  # noqa: SLF001
+                "IN SHARE ROW EXCLUSIVE MODE;"
+            )
+
         # Deserialize and validate the data
         serializer = serializers.ServerCreateDocumentSerializer(data=request.data)
         if not serializer.is_valid():
@@ -747,7 +758,12 @@ def children(self, request, *args, **kwargs):
             serializer.is_valid(raise_exception=True)
 
             with transaction.atomic():
-                child_document = document.add_child(
+                # "select_for_update" locks the table to ensure safe concurrent access
+                locked_parent = models.Document.objects.select_for_update().get(
+                    pk=document.pk
+                )
+
+                child_document = locked_parent.add_child(
                     creator=request.user,
                     **serializer.validated_data,
                 )

src/backend/core/tests/documents/test_api_documents_children_create.py

@@ -2,6 +2,7 @@
 Tests for Documents API endpoint in impress's core app: create
 """
 
+from concurrent.futures import ThreadPoolExecutor
 from uuid import uuid4
 
 import pytest
@@ -249,3 +250,41 @@ def test_api_documents_children_create_force_id_existing():
     assert response.json() == {
         "id": ["A document with this ID already exists. You cannot override it."]
     }
+
+
+@pytest.mark.django_db(transaction=True)
+def test_api_documents_create_document_children_race_condition():
+    """
+    It should be possible to create several documents at the same time
+    without causing any race conditions or data integrity issues.
+    """
+
+    user = factories.UserFactory()
+
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory()
+
+    factories.UserDocumentAccessFactory(user=user, document=document, role="owner")
+
+    def create_document():
+        return client.post(
+            f"/api/v1.0/documents/{document.id}/children/",
+            {
+                "title": "my child",
+            },
+        )
+
+    with ThreadPoolExecutor(max_workers=2) as executor:
+        future1 = executor.submit(create_document)
+        future2 = executor.submit(create_document)
+
+        response1 = future1.result()
+        response2 = future2.result()
+
+        assert response1.status_code == 201
+        assert response2.status_code == 201
+
+        document.refresh_from_db()
+        assert document.numchild == 2

src/backend/core/tests/documents/test_api_documents_create.py

@@ -52,6 +52,7 @@ def test_api_documents_create_authenticated_success():
     assert document.accesses.filter(role="owner", user=user).exists()
 
 
+@pytest.mark.django_db(transaction=True)
 def test_api_documents_create_document_race_condition():
     """
     It should be possible to create several documents at the same time

src/backend/core/tests/documents/test_api_documents_create_for_owner.py

@@ -4,6 +4,7 @@
 
 # pylint: disable=W0621
 
+from concurrent.futures import ThreadPoolExecutor
 from unittest.mock import patch
 
 from django.core import mail
@@ -425,6 +426,36 @@ def test_api_documents_create_for_owner_new_user_no_sub_no_fallback_allow_duplic
     assert document.creator == user
 
 
+@pytest.mark.django_db(transaction=True)
+def test_api_documents_create_document_race_condition():
+    """
+    It should be possible to create several documents at the same time
+    without causing any race conditions or data integrity issues.
+    """
+
+    def create_document(title):
+        user = factories.UserFactory()
+        client = APIClient()
+        client.force_login(user)
+        return client.post(
+            "/api/v1.0/documents/",
+            {
+                "title": title,
+            },
+            format="json",
+        )
+
+    with ThreadPoolExecutor(max_workers=2) as executor:
+        future1 = executor.submit(create_document, "my document 1")
+        future2 = executor.submit(create_document, "my document 2")
+
+        response1 = future1.result()
+        response2 = future2.result()
+
+        assert response1.status_code == 201
+        assert response2.status_code == 201
+
+
 @patch.object(ServerCreateDocumentSerializer, "_send_email_notification")
 @override_settings(SERVER_TO_SERVER_API_TOKENS=["DummyToken"], LANGUAGE_CODE="de-de")
 def test_api_documents_create_for_owner_with_default_language(