Skip to content

Commit

Permalink
Add mutex to page tree move function, fix #1518
Browse files Browse the repository at this point in the history
* Use django-db-mutex module to implement a mutex that allows only
  one page tree move operation at a time.
  • Loading branch information
svenseeberg committed Feb 18, 2023
1 parent 534f226 commit e9a80c0
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ UNRELEASED
* [ [#1989](https://github.com/digitalfabrik/integreat-cms/issues/1989) ] Improve load time of locations endpoint
* [ [#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
* [ [#1518](https://github.com/digitalfabrik/integreat-cms/issues/1518) ] Prevent database corruption when moving 2 pages at the same time


2023.2.0
Expand Down
1 change: 1 addition & 0 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ twine = "*"

[packages]
integreat-cms = {editable = true, path = "."}
django-db-mutex = "*"

[requires]
python_version = "3.9"
124 changes: 65 additions & 59 deletions Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 20 additions & 1 deletion integreat_cms/cms/models/abstract_tree_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -259,6 +262,8 @@ def move(self, target, pos=None):
:type pos: str
:raises ~treebeard.exceptions.InvalidPosition: If the node is moved to another region
:raises db_mutex.DBMutexError: If the DB mutex could not be retrieved
:raises db_mutex.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
Expand All @@ -282,7 +287,21 @@ def move(self, target, pos=None):
# 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(f"tree-page-{self.region}"):
super().move(target, pos)
except DBMutexError as e:
logger.error("Could not retrieve database mutex to save %s", self)
raise DBMutexError(
# pylint: disable=consider-using-f-string
_('Could not change position in tree of "{}".'.format(self))
) from e
except DBMutexTimeoutError as e:
logger.error("Retrieving database mutex timed out while saving %s", self)
raise DBMutexTimeoutError(
# pylint: disable=consider-using-f-string
_('Could not change position in tree of "{}".'.format(self))
) from e
invalidate_model(self.__class__)

# Reload 'self' because lft/rgt may have changed
Expand Down
9 changes: 9 additions & 0 deletions integreat_cms/core/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@
"django.contrib.staticfiles",
# Installed third-party-apps
"corsheaders",
"db_mutex",
"linkcheck",
"polymorphic",
"rules.apps.AutodiscoverRulesConfig",
Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions integreat_cms/locale/de/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions sphinx/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@
("py:class", "builtins.AssertionError"),
("py:class", "builtins.int"),
("py:class", "NoneType"),
("py:class", "db_mutex"),
("py:attr", "db_mutex.DBMutexTimeoutError"),
("py:attr", "db_mutex.DBMutexError"),
("py:class", "django.contrib.admin.helpers.ActionForm"),
("py:class", "django.contrib.admin.checks.ModelAdminChecks"),
("py:attr", "django.contrib.auth.models.Permission.user_set"),
Expand Down

0 comments on commit e9a80c0

Please sign in to comment.