diff --git a/frappe/database/database.py b/frappe/database/database.py index 3ce3ccfbbc..1812576d63 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -48,7 +48,9 @@ MULTI_WORD_PATTERN = re.compile(r'([`"])(tab([A-Z]\w+)( [A-Z]\w+)+)\1') SQL_ITERATOR_BATCH_SIZE = 100 -TRANSACTION_DISABLED_MSG = "Commit/rollback are disabled during certain events. This command will be ignored." +TRANSACTION_DISABLED_MSG = """Commit/rollback are disabled during certain events. This command will +be ignored. Commit/Rollback from here WILL CAUSE very hard to debug problems with atomicity and +concurrent data update bugs.""" class Database: diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index c1abd1798e..cb87d63217 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -15,7 +15,7 @@ from frappe.database.utils import FallBackDateTimeStr from frappe.query_builder import Field from frappe.query_builder.functions import Concat_ws from frappe.tests.test_query_builder import db_type_is, run_only_if -from frappe.tests.utils import FrappeTestCase, timeout +from frappe.tests.utils import FrappeTestCase, patch_hooks, timeout from frappe.utils import add_days, now, random_string, set_request from frappe.utils.testutils import clear_custom_fields @@ -459,6 +459,19 @@ class TestDB(FrappeTestCase): ) self.assertEqual(1, frappe.db.transaction_writes - writes) + def test_transactions_disabled_during_writes(self): + hook_name = f"{bad_hook.__module__}.{bad_hook.__name__}" + nested_hook_name = f"{bad_nested_hook.__module__}.{bad_nested_hook.__name__}" + + with patch_hooks( + {"doc_events": {"*": {"before_validate": hook_name, "on_update": nested_hook_name}}} + ): + note = frappe.new_doc("Note", title=frappe.generate_hash()) + note.insert() + self.assertGreater(frappe.db.transaction_writes, 0) # This would've reset for commit/rollback + + self.assertFalse(frappe.db._disable_transaction_control) + def test_pk_collision_ignoring(self): # note has `name` generated from title for _ in range(3): @@ -1007,6 +1020,17 @@ class TestConcurrency(FrappeTestCase): self.assertRaises(frappe.QueryTimeoutError, frappe.delete_doc, note.doctype, note.name) +def bad_hook(*args, **kwargs): + frappe.db.commit() + frappe.db.rollback() + + +def bad_nested_hook(doc, *args, **kwargs): + doc.run_method("before_validate") + frappe.db.commit() + frappe.db.rollback() + + class TestSqlIterator(FrappeTestCase): def test_db_sql_iterator(self): test_queries = [