[IMP] base: _has_cycle replaces _check_recursion in SQL

addons/account/models/account_account.py

@@ -940,7 +940,7 @@ def _sanitize_vals(self, vals):
 
     @api.constrains('parent_id')
     def _check_parent_not_circular(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_("You cannot create recursive groups."))
 
     @api.model_create_multi

addons/account/models/account_tax.py

@@ -480,7 +480,7 @@ def _validate_repartition_lines(self):
     @api.constrains('children_tax_ids', 'type_tax_use')
     def _check_children_scope(self):
         for tax in self:
-            if not tax._check_m2m_recursion('children_tax_ids'):
+            if tax._has_cycle('children_tax_ids'):
                 raise ValidationError(_("Recursion found for tax %r.", tax.name))
             if any(
                 child.type_tax_use not in ('none', tax.type_tax_use)

addons/hr/models/hr_department.py

@@ -70,7 +70,7 @@ def _compute_plan_count(self):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('You cannot create recursive departments.'))
 
     @api.model_create_multi

addons/membership/models/partner.py

@@ -81,13 +81,8 @@ def _compute_membership_state(self):
 
     @api.constrains('associate_member')
     def _check_recursion_associate_member(self):
-        for partner in self:
-            level = 100
-            while partner:
-                partner = partner.associate_member
-                if not level:
-                    raise ValidationError(_('You cannot create recursive associated members.'))
-                level -= 1
+        if self._has_cycle('associate_member'):
+            raise ValidationError(_('You cannot create recursive associated members.'))
 
     @api.model
     def _cron_update_membership(self):

addons/mrp/models/mrp_routing.py

@@ -102,7 +102,7 @@ def _compute_workorder_count(self):
 
     @api.constrains('blocked_by_operation_ids')
     def _check_no_cyclic_dependencies(self):
-        if not self._check_m2m_recursion('blocked_by_operation_ids'):
+        if self._has_cycle('blocked_by_operation_ids'):
             raise ValidationError(_("You cannot create cyclic dependency."))
 
     @api.model_create_multi

addons/mrp/models/mrp_workorder.py

@@ -262,7 +262,7 @@ def _set_dates(self):
 
     @api.constrains('blocked_by_workorder_ids')
     def _check_no_cyclic_dependencies(self):
-        if not self._check_m2m_recursion('blocked_by_workorder_ids'):
+        if self._has_cycle('blocked_by_workorder_ids'):
             raise ValidationError(_("You cannot create cyclic dependency."))
 
     @api.depends('production_id.name')

addons/point_of_sale/models/pos_category.py

@@ -15,7 +15,7 @@ class PosCategory(models.Model):
 
     @api.constrains('parent_id')
     def _check_category_recursion(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('Error! You cannot create recursive categories.'))
 
     def get_default_color(self):

addons/product/models/product_category.py

@@ -45,7 +45,7 @@ def _compute_product_count(self):
 
     @api.constrains('parent_id')
     def _check_category_recursion(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('You cannot create recursive categories.'))
 
     @api.model

addons/product/models/product_pricelist_item.py

@@ -189,7 +189,7 @@ def _compute_rule_tip(self):
     #=== CONSTRAINT METHODS ===#
 
     @api.constrains('base_pricelist_id', 'pricelist_id', 'base')
-    def _check_recursion(self):
+    def _check_pricelist_recursion(self):
         if any(item.base == 'pricelist' and item.pricelist_id and item.pricelist_id == item.base_pricelist_id for item in self):
             raise ValidationError(_('You cannot assign the Main Pricelist as Other Pricelist in PriceList Item'))
 

addons/project/models/project_task.py

@@ -433,7 +433,7 @@ def message_subscribe(self, partner_ids=None, subtype_ids=None):
 
     @api.constrains('depend_on_ids')
     def _check_no_cyclic_dependencies(self):
-        if not self._check_m2m_recursion('depend_on_ids'):
+        if self._has_cycle('depend_on_ids'):
             raise ValidationError(_("Two tasks cannot depend on each other."))
 
     @api.model
@@ -505,7 +505,7 @@ def _compute_dependent_tasks_count(self):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('Error! You cannot create a recursive hierarchy of tasks.'))
 
     def _get_attachments_search_domain(self):

addons/website_forum/models/forum_post.py

@@ -163,7 +163,7 @@ class Post(models.Model):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('You cannot create recursive forum posts.'))
 
     @api.depends('content')

addons/website_sale/models/product_public_category.py

@@ -75,7 +75,7 @@ def _compute_display_name(self):
 
     @api.constrains('parent_id')
     def check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValueError(_("Error! You cannot create recursive categories."))
 
     #=== BUSINESS METHODS ===#

odoo/addons/base/models/ir_actions.py

@@ -753,8 +753,8 @@ def _check_python_code(self):
                 raise ValidationError(msg)
 
     @api.constrains('child_ids')
-    def _check_recursion(self):
-        if not self._check_m2m_recursion('child_ids'):
+    def _check_child_recursion(self):
+        if self._has_cycle('child_ids'):
             raise ValidationError(_('Recursion found in child server actions'))
 
     def _get_readable_fields(self):

odoo/addons/base/models/ir_module.py

@@ -101,7 +101,7 @@ def _compute_xml_id(self):
 
     @api.constrains('parent_id')
     def _check_parent_not_circular(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_("Error ! You cannot create recursive categories."))
 
 

odoo/addons/base/models/ir_ui_menu.py

@@ -70,7 +70,7 @@ def _read_image(self, path):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('Error! You cannot create recursive menus.'))
 
     @api.model

odoo/addons/base/models/ir_ui_view.py

@@ -430,7 +430,7 @@ def _check_groups(self):
     def _check_000_inheritance(self):
         # NOTE: constraints methods are check alphabetically. Always ensure this method will be
         #       called before other constraint methods to avoid infinite loop in `_get_combined_arch`.
-        if not self._check_recursion(parent='inherit_id'):
+        if self._has_cycle('inherit_id'):
             raise ValidationError(_('You cannot create recursive inherited views.'))
 
     _sql_constraints = [

odoo/addons/base/models/res_partner.py

@@ -122,7 +122,7 @@ def _get_default_color(self):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('You can not create recursive tags.'))
 
     @api.depends('parent_id')
@@ -435,7 +435,7 @@ def _get_view(self, view_id=None, view_type='form', **options):
 
     @api.constrains('parent_id')
     def _check_parent_id(self):
-        if not self._check_recursion():
+        if self._has_cycle():
             raise ValidationError(_('You cannot create recursive Partner hierarchies.'))
 
     def copy_data(self, default=None):

odoo/addons/base/tests/test_base.py

@@ -143,17 +143,17 @@ def test_res_groups_fullname_search(self):
         groups = all_groups.search([('full_name', 'in', ['Administration / Access Rights','Contact Creation'])])
         self.assertTrue(groups, "did not match search for 'Administration / Access Rights' and 'Contact Creation'")
 
-    def test_res_group_recursion(self):
+    def test_res_group_has_cycle(self):
         # four groups with no cycle, check them all together
         a = self.env['res.groups'].create({'name': 'A'})
         b = self.env['res.groups'].create({'name': 'B'})
         c = self.env['res.groups'].create({'name': 'G', 'implied_ids': [Command.set((a + b).ids)]})
         d = self.env['res.groups'].create({'name': 'D', 'implied_ids': [Command.set(c.ids)]})
-        self.assertTrue((a + b + c + d)._check_m2m_recursion('implied_ids'))
+        self.assertFalse((a + b + c + d)._has_cycle('implied_ids'))
 
         # create a cycle and check
         a.implied_ids = d
-        self.assertFalse(a._check_m2m_recursion('implied_ids'))
+        self.assertTrue(a._has_cycle('implied_ids'))
 
     def test_res_group_copy(self):
         a = self.env['res.groups'].with_context(lang='en_US').create({'name': 'A'})

odoo/addons/base/tests/test_res_partner.py

@@ -707,8 +707,11 @@ def setUp(self):
         self.p3 = res_partner.create({'name': 'Elmtree Grand-Child 1.1', 'parent_id': self.p2.id})
 
     def test_100_res_partner_recursion(self):
-        self.assertTrue(self.p3._check_recursion())
-        self.assertTrue((self.p1 + self.p2 + self.p3)._check_recursion())
+        self.assertFalse(self.p3._has_cycle())
+        self.assertFalse((self.p1 + self.p2 + self.p3)._has_cycle())
+
+        # special case: empty recordsets don't lead to cycles
+        self.assertFalse(self.env['res.partner']._has_cycle())
 
     # split 101, 102, 103 tests to force SQL rollback between them
 
@@ -731,6 +734,11 @@ def test_104_res_partner_recursion_indirect_cycle(self):
             self.p2.write({'child_ids': [Command.update(self.p3.id, {'parent_id': p3b.id}),
                                          Command.update(p3b.id, {'parent_id': self.p3.id})]})
 
+    def test_105_res_partner_recursion(self):
+        with self.assertRaises(ValidationError):
+            # p3 -> p2 -> p1 -> p2
+            (self.p3 + self.p1).parent_id = self.p2
+
     def test_110_res_partner_recursion_multi_update(self):
         """ multi-write on several partners in same hierarchy must not trigger a false cycle detection """
         ps = self.p1 + self.p2 + self.p3

odoo/models.py

@@ -5590,79 +5590,76 @@ def exists(self):
         valid_ids = set([r[0] for r in self._cr.fetchall()] + new_ids)
         return self.browse(i for i in self._ids if i in valid_ids)
 
-    def _check_recursion(self, parent=None):
+    def _has_cycle(self, field_name=None) -> bool:
         """
-        Verifies that there is no loop in a hierarchical structure of records,
-        by following the parent relationship using the **parent** field until a
-        loop is detected or until a top-level record is found.
+        Return whether the records in ``self`` are in a loop by following the
+        given relationship of the field.
+        By default the **parent** field is used as the relationship.
 
-        :param parent: optional parent field name (default: ``self._parent_name``)
-        :return: **True** if no loop was found, **False** otherwise.
-        """
-        if not parent:
-            parent = self._parent_name
+        Note that since the method does not use EXCLUSIVE LOCK for the sake of
+        performance, loops may still be created by concurrent transactions.
 
-        # must ignore 'active' flag, ir.rules, etc. => direct SQL query
-        cr = self._cr
-        self.flush_model([parent])
-        for id in self.ids:
-            current_id = id
-            seen_ids = {current_id}
-            while current_id:
-                cr.execute(SQL(
-                    "SELECT %s FROM %s WHERE id = %s",
-                    SQL.identifier(parent), SQL.identifier(self._table), current_id,
-                ))
-                result = cr.fetchone()
-                current_id = result[0] if result else None
-                if current_id in seen_ids:
-                    return False
-                seen_ids.add(current_id)
-        return True
-
-    def _check_m2m_recursion(self, field_name):
+        :param field_name: optional field name (default: ``self._parent_name``)
+        :return: **True** if a loop was found, **False** otherwise.
         """
-        Verifies that there is no loop in a directed graph of records, by
-        following a many2many relationship with the given field name.
+        if not field_name:
+            field_name = self._parent_name
 
-        :param field_name: field to check
-        :return: **True** if no loop was found, **False** otherwise.
-        """
         field = self._fields.get(field_name)
-        if not (field and field.type == 'many2many' and
-                field.comodel_name == self._name and field.store):
-            # field must be a many2many on itself
-            raise ValueError('invalid field_name: %r' % (field_name,))
+        if not field:
+            raise ValueError(f'Invalid field_name: {field_name!r}')
 
-        self.flush_model([field_name])
+        if not (
+            field.type in ('many2many', 'many2one')
+            and field.comodel_name == self._name
+            and field.store
+        ):
+            raise ValueError(f'Field must be a many2one or many2many relation on itself: {field_name!r}')
 
+        if not self.ids:
+            return False
+
+        # must ignore 'active' flag, ir.rules, etc.
+        # direct recursive SQL query with cycle detection for performance
+        self.flush_model([field_name])
+        if field.type == 'many2many':
+            relation = field.relation
+            column1 = field.column1
+            column2 = field.column2
+        else:
+            relation = self._table
+            column1 = 'id'
+            column2 = field_name
         cr = self._cr
-        succs = defaultdict(set)        # transitive closure of successors
-        preds = defaultdict(set)        # transitive closure of predecessors
-        todo, done = set(self.ids), set()
-        while todo:
-            # retrieve the respective successors of the nodes in 'todo'
-            cr.execute(SQL(
-                """ SELECT %(col1)s, %(col2)s FROM %(rel)s
-                    WHERE %(col1)s IN %(ids)s AND %(col2)s IS NOT NULL """,
-                rel=SQL.identifier(field.relation),
-                col1=SQL.identifier(field.column1),
-                col2=SQL.identifier(field.column2),
-                ids=tuple(todo),
-            ))
-            done.update(todo)
-            todo.clear()
-            for id1, id2 in cr.fetchall():
-                # connect id1 and its predecessors to id2 and its successors
-                for x, y in itertools.product([id1] + list(preds[id1]),
-                                              [id2] + list(succs[id2])):
-                    if x == y:
-                        return False    # we found a cycle here!
-                    succs[x].add(y)
-                    preds[y].add(x)
-                if id2 not in done:
-                    todo.add(id2)
-        return True
+        cr.execute(SQL(
+            """
+            WITH RECURSIVE __reachability AS (
+                SELECT %(col1)s AS source, %(col2)s AS destination
+                FROM %(rel)s
+                WHERE %(col1)s IN %(ids)s AND %(col2)s IS NOT NULL
+            UNION
+                SELECT r.source, t.%(col2)s
+                FROM __reachability r
+                JOIN %(rel)s t ON r.destination = t.%(col1)s AND t.%(col2)s IS NOT NULL
+            )
+            SELECT 1 FROM __reachability
+            WHERE source = destination
+            LIMIT 1
+            """,
+            ids=tuple(self.ids),
+            rel=SQL.identifier(relation),
+            col1=SQL.identifier(column1),
+            col2=SQL.identifier(column2),
+        ))
+        return bool(cr.fetchone())
+
+    def _check_recursion(self, parent=None):
+        warnings.warn("Since 18.0, one must use not _has_cycle() instead", DeprecationWarning, 2)
+        return not self._has_cycle(parent)
+
+    def _check_m2m_recursion(self, field_name):
+        warnings.warn("Since 18.0, one must use not _has_cycle() instead", DeprecationWarning, 2)
+        return not self._has_cycle(field_name)
 
     def _get_external_ids(self):
         """Retrieve the External ID(s) of any database record.