diff --git a/auditlog/README.rst b/auditlog/README.rst index 1d797d593ea..4db07f2dbd5 100644 --- a/auditlog/README.rst +++ b/auditlog/README.rst @@ -6,7 +6,7 @@ Audit Log ========= -.. +.. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! !! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! @@ -124,6 +124,9 @@ Contributors - Bogdan Valentin Gabor - Dennis Sluijk d.sluijk@onestein.nl - Adam Heinz +- `OERP Canada `__: + + - Hembert Iregui Other credits ------------- diff --git a/auditlog/models/auditlog_guard.py b/auditlog/models/auditlog_guard.py new file mode 100644 index 00000000000..e5ff345bdba --- /dev/null +++ b/auditlog/models/auditlog_guard.py @@ -0,0 +1,26 @@ +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). +import threading + +_auditlog_local = threading.local() + + +def get_guard(): + if not hasattr(_auditlog_local, "guard"): + _auditlog_local.guard = set() + return _auditlog_local.guard + + +def add_guard(keys): + guard = get_guard() + guard.update(keys) + return guard + + +def has_conflict(keys): + guard = get_guard() + return bool(guard.intersection(keys)) + + +def remove_guard(keys): + guard = get_guard() + guard.difference_update(keys) diff --git a/auditlog/models/auditlog_rule.py b/auditlog/models/auditlog_rule.py index 8c90c452bd7..b26aadf2f28 100644 --- a/auditlog/models/auditlog_rule.py +++ b/auditlog/models/auditlog_rule.py @@ -8,6 +8,8 @@ from odoo.fields import Command from odoo.orm.identifiers import NewId +from .auditlog_guard import add_guard, has_conflict, remove_guard + FIELDS_BLACKLIST = [ "id", "create_uid", @@ -365,7 +367,7 @@ def _make_create(self): @api.model_create_multi def create_full(self, vals_list, **kwargs): - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] new_records = create_full.origin(self, vals_list, **kwargs) # Take a snapshot of record values from the cache instead of using @@ -400,7 +402,7 @@ def create_full(self, vals_list, **kwargs): @api.model_create_multi def create_fast(self, vals_list, **kwargs): - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] vals_list = rule_model._update_vals_list(vals_list) vals_list2 = copy.deepcopy(vals_list) @@ -443,9 +445,9 @@ def read(self, fields=None, load="_classic_read", **kwargs): # avoid logs on `read` produced by auditlog during internal # processing: read data of relevant records, 'ir.model', # 'ir.model.fields'... (no interest in logging such operations) - if self.env.context.get("auditlog_disabled"): + if self.env.context.get("auditlog_disabled_read"): return result - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] if self.env.user in users_to_exclude: return result @@ -469,41 +471,56 @@ def _make_write(self): users_to_exclude = self.mapped("users_to_exclude_ids") def write_full(self, vals, **kwargs): - self = self.with_context(auditlog_disabled=True) - rule_model = self.env["auditlog.rule"] - fields_list = rule_model.get_auditlog_fields(self) - records_write = ( - self.filtered(lambda r: not isinstance(r.id, NewId)) - .sudo() - .with_context(prefetch_fields=False) - ) - if not records_write: + self = self.with_context(auditlog_disabled_read=True) + guard_keys = {(self._name, tuple(self.ids))} + + if has_conflict(guard_keys): return write_full.origin(self, vals, **kwargs) - with ThrowAwayCache(self.env): - old_values = {d["id"]: d for d in records_write.read(fields_list)} + add_guard(guard_keys) + try: + rule_model = self.env["auditlog.rule"] + fields_list = rule_model.get_auditlog_fields(self) - result = write_full.origin(self, vals, **kwargs) - self.flush_recordset() - if self.env.user in users_to_exclude: - return result + records_write = ( + self.filtered(lambda r: not isinstance(r.id, NewId)) + .sudo() + .with_context(prefetch_fields=False) + ) - with ThrowAwayCache(self.env): - new_values = {d["id"]: d for d in records_write.read(fields_list)} + if not records_write: + return write_full.origin(self, vals, **kwargs) - rule_model.sudo().create_logs( - self.env.uid, - self._name, - records_write.ids, - "write", - old_values, - new_values, - {"log_type": log_type}, - ) - return result + with ThrowAwayCache(self.env): + old_values = {d["id"]: d for d in records_write.read(fields_list)} + + result = write_full.origin(self, vals, **kwargs) + + self.flush_recordset() + + if self.env.user not in users_to_exclude: + with ThrowAwayCache(self.env): + new_values = { + d["id"]: d for d in records_write.read(fields_list) + } + + rule_model.sudo().create_logs( + self.env.uid, + self._name, + records_write.ids, + "write", + old_values, + new_values, + {"log_type": log_type}, + ) + + return result + + finally: + remove_guard(guard_keys) def write_fast(self, vals, **kwargs): - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] # Log the user input only, no matter if the `vals` is updated # afterwards as it could not represent the real state @@ -535,7 +552,7 @@ def _make_unlink(self): users_to_exclude = self.mapped("users_to_exclude_ids") def unlink_full(self, **kwargs): - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] fields_list = rule_model.get_auditlog_fields(self) old_values = { @@ -558,7 +575,7 @@ def unlink_full(self, **kwargs): return unlink_full.origin(self, **kwargs) def unlink_fast(self, **kwargs): - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] if self.env.user in users_to_exclude: return unlink_fast.origin(self, **kwargs) @@ -583,7 +600,7 @@ def _make_export_data(self): def export_data(self, fields_to_export): res = export_data.origin(self, fields_to_export) - self = self.with_context(auditlog_disabled=True) + self = self.with_context(auditlog_disabled_read=True) rule_model = self.env["auditlog.rule"] if self.env.user in users_to_exclude: return res diff --git a/test_auditlog/tests/__init__.py b/test_auditlog/tests/__init__.py index 9678814e1ad..10c5f5c6aae 100644 --- a/test_auditlog/tests/__init__.py +++ b/test_auditlog/tests/__init__.py @@ -1,3 +1,4 @@ from . import test_account_bank_statement_line from . import test_account_move_reverse from . import test_product_tax_multicompany +from . import test_account_reentrancy diff --git a/test_auditlog/tests/test_account_reentrancy.py b/test_auditlog/tests/test_account_reentrancy.py new file mode 100644 index 00000000000..0623292c224 --- /dev/null +++ b/test_auditlog/tests/test_account_reentrancy.py @@ -0,0 +1,63 @@ +from odoo.tests.common import tagged + +from odoo.addons.auditlog.tests.common import AuditLogRuleCommon + + +@tagged("post_install", "-at_install") +class TestAccountAuditlog(AuditLogRuleCommon): + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls.account_model_id = cls.env.ref("account.model_account_account").id + + cls.rule = cls.create_rule( + { + "name": "Account Audit Rule", + "model_id": cls.account_model_id, + "log_create": True, + "log_write": True, + "log_read": True, + "log_unlink": True, + "log_type": "full", + } + ) + cls.rule.set_to_confirmed() + + def test_write_code_reentrancy(self): + """Regression test for recursive audit logging. + Ensures that writes triggered by computed/inverse fields (code/code_store) + account.account do not cause infinite recursion while preserving + the expected audit log entries. + """ + + account = self.env["account.account"].create( + { + "name": "Test Account", + "code": "1000", + } + ) + + logs = self.env["auditlog.log"].search( + [ + ("model_id", "=", self.account_model_id), + ("method", "=", "write"), + ("res_id", "=", account.id), + ] + ) + + self.assertTrue( + logs.line_ids.filtered(lambda log_line: log_line.field_name == "code_store") + ) + + account.write({"code": "2000"}) + + logs = self.env["auditlog.log"].search( + [ + ("model_id", "=", self.account_model_id), + ("method", "=", "write"), + ("res_id", "=", account.id), + ] + ) + + self.assertEqual(len(logs), 2)