[FIX] base_automation: recursive computed field cause too many calls to flush

The use-case that motivated this fix is the deletion of a project task
with many subtasks.  The field 'project_id' on tasks is recursively
computed, and some automated action must be executed when its value
corresponds to a given project.

The issue occurs when the domain of automated actions is evaluated by
method search(), because the latter flushes the fields to search on,
which are also the ones being recomputed.  Combined with the fact that
recursive fields are not computed in batch, this leads to a huge amount
of recursive calls between the automated action and flush().

The execution of task.unlink() looks like this:
- mark 'project_id' to compute on subtasks
- delete task
- flush()
  - recompute 'project_id' on subtask1
    - call compute on subtask1
    - in action, search([('id', 'in', subtask1.ids), ('project_id', '=', pid)])
      - flush(['id', 'project_id'])
        - recompute 'project_id' on subtask2
          - call compute on subtask2
          - in action, search([('id', 'in', subtask2.ids), ('project_id', '=', pid)])
            - flush(['id', 'project_id'])
              - recompute 'project_id' on subtask3
                - call compute on subtask3
                - in action, search([('id', 'in', subtask3.ids), ('project_id', '=', pid)])
                  - flush(['id', 'project_id'])
                    - recompute 'project_id' on subtask4
                      ...

closes odoo/odoo#80141

X-original-commit: e2788b580ef15ef3083ac919737a46d850143832
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

addons/base_automation/models/base_automation.py

@@ -218,8 +218,8 @@ class BaseAutomation(models.Model):
         """ Filter the records that satisfy the postcondition of action ``self``. """
         self_sudo = self.sudo()
         if self_sudo.filter_domain and records:
-            domain = [('id', 'in', records.ids)] + safe_eval.safe_eval(self_sudo.filter_domain, self._get_eval_context())
-            return records.sudo().search(domain).with_env(records.env), domain
+            domain = safe_eval.safe_eval(self_sudo.filter_domain, self._get_eval_context())
+            return records.sudo().filtered_domain(domain).with_env(records.env), domain
         else:
             return records, None
 

addons/test_base_automation/models/test_base_automation.py

@@ -65,3 +65,27 @@ class ModelWithoutAccess(models.Model):
 
     name = fields.Char()
     another_field = fields.Char()
+
+
+class Project(models.Model):
+    _name = _description = 'test_base_automation.project'
+
+    name = fields.Char()
+    task_ids = fields.One2many('test_base_automation.task', 'project_id')
+
+
+class Task(models.Model):
+    _name = _description = 'test_base_automation.task'
+
+    name = fields.Char()
+    parent_id = fields.Many2one('test_base_automation.task')
+    project_id = fields.Many2one(
+        'test_base_automation.project',
+        compute='_compute_project_id', recursive=True, store=True, readonly=False,
+    )
+
+    @api.depends('parent_id.project_id')
+    def _compute_project_id(self):
+        for task in self:
+            if not task.project_id:
+                task.project_id = task.parent_id.project_id

addons/test_base_automation/security/ir.model.access.csv

@@ -3,3 +3,5 @@ access_base_automation_lead_test,access_base_automation_lead_test,model_base_aut
 access_base_automation_line_test,access_base_automation_line_test,model_base_automation_line_test,base.group_system,1,1,1,1
 access_base_automation_link_test,access_base_automation_link_test,model_base_automation_link_test,,1,1,1,1
 access_base_automation_linked_test,access_base_automation_linked_test,model_base_automation_linked_test,,1,1,1,1
+access_test_base_automation_project,access_test_base_automation_project,model_test_base_automation_project,,1,1,1,1
+access_test_base_automation_task,access_test_base_automation_task,model_test_base_automation_task,,1,1,1,1

addons/test_base_automation/tests/test_flow.py

@@ -2,6 +2,7 @@
 # # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from unittest.mock import patch
+import sys
 
 from odoo.addons.base.tests.common import TransactionCaseWithUserDemo
 from odoo.tests import common, tagged
@@ -355,7 +356,8 @@ record['name'] = record.name + 'X'""",
         rec4 = Model.with_user(self.user_demo).create({'name': 'again another record'})
         rec4.write({'name': 'another value'})
 
-@common.tagged('post_install','-at_install')
+
+@common.tagged('post_install', '-at_install')
 class TestCompute(common.TransactionCase):
     def test_inversion(self):
         """ If a stored field B depends on A, an update to the trigger for A
@@ -404,3 +406,58 @@ class TestCompute(common.TransactionCase):
         })
         r.parent_id = company2
         self.assertEqual(r.display_name, 'Awiclo, Bob')
+
+    def test_recursion(self):
+        project = self.env['test_base_automation.project'].create({})
+
+        # this action is executed every time a task is assigned to project
+        self.env['base.automation'].create({
+            'name': 'dummy',
+            'model_id': self.env['ir.model']._get_id('test_base_automation.task'),
+            'state': 'code',
+            'trigger': 'on_create_or_write',
+            'filter_domain': repr([('project_id', '=', project.id)]),
+        })
+
+        # create one task in project with 10 subtasks; all the subtasks are
+        # automatically assigned to project, too
+        task = self.env['test_base_automation.task'].create({'project_id': project.id})
+        subtasks = task.create([{'parent_id': task.id} for _ in range(10)])
+        subtasks.flush()
+
+        # This test checks what happens when a stored recursive computed field
+        # is marked to compute on many records, and automated actions are
+        # triggered depending on that field.  In this case, we trigger the
+        # recomputation of 'project_id' on 'subtasks' by deleting their parent
+        # task.
+        #
+        # An issue occurs when the domain of automated actions is evaluated by
+        # method search(), because the latter flushes the fields to search on,
+        # which are also the ones being recomputed.  Combined with the fact
+        # that recursive fields are not computed in batch, this leads to a huge
+        # amount of recursive calls between the automated action and flush().
+        #
+        # The execution of task.unlink() looks like this:
+        # - mark 'project_id' to compute on subtasks
+        # - delete task
+        # - flush()
+        #   - recompute 'project_id' on subtask1
+        #     - call compute on subtask1
+        #     - in action, search([('id', 'in', subtask1.ids), ('project_id', '=', pid)])
+        #       - flush(['id', 'project_id'])
+        #         - recompute 'project_id' on subtask2
+        #           - call compute on subtask2
+        #           - in action search([('id', 'in', subtask2.ids), ('project_id', '=', pid)])
+        #             - flush(['id', 'project_id'])
+        #               - recompute 'project_id' on subtask3
+        #                 - call compute on subtask3
+        #                 - in action, search([('id', 'in', subtask3.ids), ('project_id', '=', pid)])
+        #                   - flush(['id', 'project_id'])
+        #                     - recompute 'project_id' on subtask4
+        #                       ...
+        limit = sys.getrecursionlimit()
+        try:
+            sys.setrecursionlimit(100)
+            task.unlink()
+        finally:
+            sys.setrecursionlimit(limit)