Skip to content

Commit

Permalink
feat: let users unlock stuck documents (backport #24782) (#25225)
Browse files Browse the repository at this point in the history
* fix: better file locking

(cherry picked from commit 1f9efb7)

# Conflicts:
#	frappe/model/document.py

* fix: Auto delete very old document locks

locks older than 12 hours are most likely from dead processes. They can be (mostly) safely ignored.

(cherry picked from commit d616341)

# Conflicts:
#	frappe/model/document.py

* fix!: Accept flat arguments for server_action

(cherry picked from commit eb1b1b4)

* feat: support primary_action for `frappe.throw`

(cherry picked from commit 40f1ae1)

* feat: let users unlock stuck documents

(cherry picked from commit d89e0e7)

* refactor: simplify code

Co-authored-by: Akhil Narang <me@akhilnarang.dev>
(cherry picked from commit 2c19e84)

* chore: conflicts

* Revert "fix!: Accept flat arguments for server_action"

This reverts commit e7f2319.

* fix: make unlocking Backward compatibile

* fix: wrong variable assignment

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
Co-authored-by: Akhil Narang <me@akhilnarang.dev>
(cherry picked from commit b0db641)

# Conflicts:
#	frappe/model/document.py
#	frappe/tests/test_document_locks.py
  • Loading branch information
mergify[bot] committed Apr 18, 2024
1 parent aefc6f9 commit e58ea03
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 4 deletions.
10 changes: 9 additions & 1 deletion frappe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,18 @@ def throw(
is_minimizable: bool = False,
wide: bool = False,
as_list: bool = False,
primary_action=None,
) -> None:
"""Throw execption and show message (`msgprint`).
:param msg: Message.
:param exc: Exception class. Default `frappe.ValidationError`"""
:param exc: Exception class. Default `frappe.ValidationError`
:param title: [optional] Message title. Default: "Message".
:param is_minimizable: [optional] Allow users to minimize the modal
:param wide: [optional] Show wide modal
:param as_list: [optional] If `msg` is a list, render as un-ordered list.
:param primary_action: [optional] Bind a primary server/client side action.
"""
msgprint(
msg,
raise_exception=exc,
Expand All @@ -546,6 +553,7 @@ def throw(
is_minimizable=is_minimizable,
wide=wide,
as_list=as_list,
primary_action=primary_action,
)


Expand Down
99 changes: 96 additions & 3 deletions frappe/model/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
from frappe.utils.global_search import update_global_search


DOCUMENT_LOCK_EXPIRTY = 12 * 60 * 60 # All locks expire in 12 hours automatically
DOCUMENT_LOCK_SOFT_EXPIRY = 60 * 60 # Let users force-unlock after 60 minutes


def get_doc(*args, **kwargs):
"""returns a frappe.model.Document object.
Expand Down Expand Up @@ -1411,8 +1415,8 @@ def log_error(self, title=None, message=None):
)

def get_signature(self):
"""Returns signature (hash) for private URL."""
return hashlib.sha224(get_datetime_str(self.creation).encode()).hexdigest()
"""Return signature (hash) for private URL."""
return hashlib.sha224(f"{self.doctype}:{self.name}".encode(), usedforsecurity=False).hexdigest()

def get_document_share_key(self, expires_on=None, no_expiry=False):
if no_expiry:
Expand Down Expand Up @@ -1470,9 +1474,25 @@ def queue_action(self, action, **kwargs):
try:
self.lock()
except frappe.DocumentLockedError:
# Allow unlocking if created more than 60 minutes ago
primary_action = None
if file_lock.lock_age(self.get_signature()) > DOCUMENT_LOCK_SOFT_EXPIRY:
primary_action = {
"label": "Force Unlock",
"server_action": "frappe.model.document.unlock_document",
"hide_on_success": True,
"args": {
"doctype": self.doctype,
"name": self.name,
},
}

frappe.throw(
_("This document is currently queued for execution. Please try again"),
_(
"This document is currently locked and queued for execution. Please try again after some time."
),
title=_("Document Queued"),
primary_action=primary_action,
)

enqueue_after_commit = kwargs.pop("enqueue_after_commit", None)
Expand All @@ -1496,6 +1516,9 @@ def lock(self, timeout=None):
signature = self.get_signature()
if file_lock.lock_exists(signature):
lock_exists = True
if file_lock.lock_age(signature) > DOCUMENT_LOCK_EXPIRTY:
file_lock.delete_lock(signature)
lock_exists = False
if timeout:
for _i in range(timeout):
time.sleep(1)
Expand Down Expand Up @@ -1601,3 +1624,73 @@ def execute_action(__doctype, __name, __action, **kwargs):

doc.add_comment("Comment", _("Action Failed") + "<br><br>" + msg)
doc.notify_update()
<<<<<<< HEAD
=======


def bulk_insert(
doctype: str,
documents: Iterable["Document"],
ignore_duplicates: bool = False,
chunk_size=10_000,
):
"""Insert simple Documents objects to database in bulk.
Warning/Info:
- All documents are inserted without triggering ANY hooks.
- This function assumes you've done the due dilligence and inserts in similar fashion as db_insert
- Documents can be any iterable / generator containing Document objects
"""

doctype_meta = frappe.get_meta(doctype)
documents = list(documents)

valid_column_map = {
doctype: doctype_meta.get_valid_columns(),
}
values_map = {
doctype: _document_values_generator(documents, valid_column_map[doctype]),
}

for child_table in doctype_meta.get_table_fields():
valid_column_map[child_table.options] = frappe.get_meta(child_table.options).get_valid_columns()
values_map[child_table.options] = _document_values_generator(
(
ch_doc
for ch_doc in (
child_docs for doc in documents for child_docs in doc.get(child_table.fieldname)
)
),
valid_column_map[child_table.options],
)

for dt, docs in values_map.items():
frappe.db.bulk_insert(
dt, valid_column_map[dt], docs, ignore_duplicates=ignore_duplicates, chunk_size=chunk_size
)


def _document_values_generator(
documents: Iterable["Document"],
columns: list[str],
) -> Generator[tuple[Any], None, None]:
for doc in documents:
doc.creation = doc.modified = now()
doc.owner = doc.modified_by = frappe.session.user
doc_values = doc.get_valid_dict(
convert_dates_to_str=True,
ignore_nulls=True,
ignore_virtual=True,
)
yield tuple(doc_values.get(col) for col in columns)


@frappe.whitelist()
def unlock_document(doctype: str | None = None, name: str | None = None, args=None):
if not doctype and not name and args:
# Backward compatibility
doctype = str(args["doctype"])
name = str(args["name"])
frappe.get_doc(doctype, name).unlock()
frappe.msgprint(frappe._("Document Unlocked"), alert=True)
>>>>>>> b0db64106a (feat: let users unlock stuck documents (backport #24782) (#25225))
33 changes: 33 additions & 0 deletions frappe/tests/test_document_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License: MIT. See LICENSE
import frappe
from frappe.tests.utils import FrappeTestCase
from frappe.utils.data import add_to_date, today


class TestDocumentLocks(FrappeTestCase):
Expand All @@ -16,3 +17,35 @@ def test_locking(self):
todo_1.lock()
self.assertRaises(frappe.DocumentLockedError, todo.lock)
todo_1.unlock()
<<<<<<< HEAD
=======

def test_operations_on_locked_documents(self):
todo = frappe.get_doc(dict(doctype="ToDo", description="testing operations")).insert()
todo.lock()

with self.assertRaises(frappe.DocumentLockedError):
todo.description = "Random"
todo.save()

# Checking for persistant locks across all instances.
doc = frappe.get_doc("ToDo", todo.name)
self.assertEqual(doc.is_locked, True)

with self.assertRaises(frappe.DocumentLockedError):
doc.description = "Random"
doc.save()

doc.unlock()
self.assertEqual(doc.is_locked, False)
self.assertEqual(todo.is_locked, False)

def test_locks_auto_expiry(self):
todo = frappe.get_doc(dict(doctype="ToDo", description=frappe.generate_hash())).insert()
todo.lock()

self.assertRaises(frappe.DocumentLockedError, todo.lock)

with self.freeze_time(add_to_date(today(), days=3)):
todo.lock()
>>>>>>> b0db64106a (feat: let users unlock stuck documents (backport #24782) (#25225))
6 changes: 6 additions & 0 deletions frappe/utils/file_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"""

import os
from pathlib import Path
from time import time

import frappe
Expand Down Expand Up @@ -41,6 +42,11 @@ def lock_exists(name):
return os.path.exists(get_lock_path(name))


def lock_age(name) -> float:
"""Return time in seconds since lock was created."""
return time() - Path(get_lock_path(name)).stat().st_mtime


def check_lock(path, timeout=600):
if not os.path.exists(path):
return False
Expand Down

0 comments on commit e58ea03

Please sign in to comment.