Add mutex to tree move function, fix #1518

CHANGELOG.md

@@ -7,6 +7,7 @@ UNRELEASED
 * [ [#1578](https://github.com/digitalfabrik/integreat-cms/issues/1578) ] Add option to hide files in global media library
 * [ [#1752](https://github.com/digitalfabrik/integreat-cms/issues/1752) ] Show chapter for internal link suggestions
 * [ [#1965](https://github.com/digitalfabrik/integreat-cms/issues/1965) ] Add ical rrule for recurring events
+* [ [#1518](https://github.com/digitalfabrik/integreat-cms/issues/1518) ] Prevent database corruption when moving 2 pages at the same time
 
 
 2023.2.0

integreat_cms/cms/models/abstract_tree_node.py

@@ -8,6 +8,9 @@
 from treebeard.exceptions import InvalidPosition
 from treebeard.ns_tree import NS_Node
 
+from db_mutex import DBMutexError, DBMutexTimeoutError
+from db_mutex.db_mutex import db_mutex
+
 from .abstract_base_model import AbstractBaseModel
 from ..constants import position
 
@@ -259,30 +262,42 @@ def move(self, target, pos=None):
         :type pos: str
 
         :raises ~treebeard.exceptions.InvalidPosition: If the node is moved to another region
+        :raises ~db_mutex.exceptions.DBMutexError: If the DB mutex could not be retrieved
+        :raises ~db_mutex.exceptions.DBMutexTimeoutError: If waiting for the DB mutex timed out
         """
         logger.debug("Moving %r to position %r of %r", self, pos, target)
-        # Do not allow to move a node outside its region
-        if self.region != target.region:
-            # Allow moving as siblings of root nodes (because it's a separate tree)
-            if not (
-                target.is_root()
-                and pos
-                in [
-                    position.LEFT,
-                    position.RIGHT,
-                    position.FIRST_SIBLING,
-                    position.LAST_SIBLING,
-                ]
-            ):
-                raise InvalidPosition(
-                    _('The node "{}" in region "{}" cannot be moved to "{}".').format(
-                        self, self.region, target.region
-                    )
-                )
-        # Moving a node can modify all other nodes via raw sql queries (which are not recognized by cachalot),
-        # so we have to invalidate the whole model manually.
-        invalidate_model(self.__class__)
-        super().move(target=target, pos=pos)
+        try:
+            with db_mutex(self.__class__.__name__):
+                # Do not allow to move a node outside its region
+                if self.region != target.region:
+                    # Allow moving as siblings of root nodes (because it's a separate tree)
+                    if not (
+                        target.is_root()
+                        and pos
+                        in [
+                            position.LEFT,
+                            position.RIGHT,
+                            position.FIRST_SIBLING,
+                            position.LAST_SIBLING,
+                        ]
+                    ):
+                        raise InvalidPosition(
+                            _(
+                                'The node "{}" in region "{}" cannot be moved to "{}".'
+                            ).format(self, self.region, target.region)
+                        )
+                # Moving a node can modify all other nodes via raw sql queries (which are not recognized by cachalot),
+                # so we have to invalidate the whole model manually.
+                invalidate_model(self.__class__)
+                super().move(target, pos)
+        except DBMutexError as e:
+            raise DBMutexError(
+                _('Could not change position in tree of "{}".').format(self)
+            ) from e
+        except DBMutexTimeoutError as e:
+            raise DBMutexTimeoutError(
+                _('Could not change position in tree of "{}".').format(self)
+            ) from e
         invalidate_model(self.__class__)
 
         # Reload 'self' because lft/rgt may have changed

integreat_cms/cms/views/pages/page_actions.py

@@ -16,6 +16,7 @@
 from django.views.decorators.http import require_POST
 from django.db import transaction
 
+from db_mutex import DBMutexError, DBMutexTimeoutError
 from treebeard.exceptions import InvalidPosition, InvalidMoveToDescendant
 
 from ....api.decorators import json_response
@@ -491,7 +492,13 @@ def move_page(request, region_slug, language_slug, page_id, target_id, position)
                 page=page.best_translation.title
             ),
         )
-    except (ValueError, InvalidPosition, InvalidMoveToDescendant) as e:
+    except (
+        ValueError,
+        InvalidPosition,
+        InvalidMoveToDescendant,
+        DBMutexTimeoutError,
+        DBMutexError,
+    ) as e:
         messages.error(request, e)
         logger.exception(e)
 

integreat_cms/core/settings.py

@@ -247,6 +247,7 @@
     "django.contrib.staticfiles",
     # Installed third-party-apps
     "corsheaders",
+    "db_mutex",
     "linkcheck",
     "polymorphic",
     "rules.apps.AutodiscoverRulesConfig",
@@ -1012,3 +1013,11 @@
 
 #: The URL path where XLIFF files are served for download
 XLIFF_URL = "/xliff/"
+
+
+############
+# DB Mutex #
+############
+
+# Our database operations should never exceed 60 seconds
+DB_MUTEX_TTL_SECONDS = 60

integreat_cms/locale/de/LC_MESSAGES/django.po

@@ -2340,6 +2340,10 @@ msgid "The node \"{}\" in region \"{}\" cannot be moved to \"{}\"."
 msgstr ""
 "Der Knoten \"{}\" in Region \"{}\" kann nicht nach \"{}\" verschoben werden."
 
+#: cms/models/abstract_tree_node.py
+msgid "Could not change position in tree of \"{}\"."
+msgstr "Position von \"{}\" im Baum konnte nicht geändert werden."
+
 #: cms/models/chat/chat_message.py
 msgid "sender"
 msgstr "Absender:in"

setup.cfg

@@ -49,6 +49,7 @@ install_requires =
 	Django>=3.2,<4.0
 	django-cacheops
 	django-cors-headers
+	django-db-mutex
 	django-debug-toolbar
 	django-linkcheck
 	django-polymorphic

sphinx/conf.py

@@ -94,6 +94,7 @@
         "https://sphinx-rtd-tutorial.readthedocs.io/en/latest/",
         None,
     ),
+    "db_mutex": ("https://django-db-mutex.readthedocs.io/en/latest/", None),
     "django": (
         f"https://docs.djangoproject.com/en/{django_version}/",
         f"https://docs.djangoproject.com/en/{django_version}/_objects/",