diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py index 42bfb114eec..f3217e8320e 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -654,6 +654,45 @@ class ComputeRecursiveTree(models.Model): rec.display_name = '%s(%s)' % (rec.name, ', '.join(children_names)) +class ComputeRecursiveOrder(models.Model): + _name = _description = 'test_new_api.recursive.order' + + value = fields.Integer() + + +class ComputeRecursiveLine(models.Model): + _name = _description = 'test_new_api.recursive.line' + + order_id = fields.Many2one('test_new_api.recursive.order') + task_ids = fields.One2many('test_new_api.recursive.task', 'line_id') + task_number = fields.Integer(compute='_compute_task_number', store=True) + + # line.task_number indirectly depends on recursive field task.line_id, and + # is triggered by the recursion in modified() on field task.line_id + @api.depends('task_ids') + def _compute_task_number(self): + for record in self: + record.task_number = len(record.task_ids) + + +class ComputeRecursiveTask(models.Model): + _name = _description = 'test_new_api.recursive.task' + + value = fields.Integer() + line_id = fields.Many2one('test_new_api.recursive.line', + compute='_compute_line_id', recursive=True, store=True) + + # the recursive nature of task.line_id is a bit artificial, but it makes + # line.task_number be triggered by a recursive call in modified() + @api.depends('value', 'line_id.order_id.value') + def _compute_line_id(self): + # this assignment forces the new value of record.line_id to be dirty in cache + self.line_id = False + for record in self: + domain = [('order_id.value', '=', record.value)] + record.line_id = record.line_id.search(domain, order='id desc', limit=1) + + class ComputeCascade(models.Model): _name = 'test_new_api.cascade' _description = 'Test New API Cascade' diff --git a/odoo/addons/test_new_api/security/ir.model.access.csv b/odoo/addons/test_new_api/security/ir.model.access.csv index 8207897d07e..012148d255e 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -23,6 +23,9 @@ access_test_new_api_compute_readonly,access_test_new_api_compute_readonly,model_ access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inverse,model_test_new_api_multi_compute_inverse,base.group_system,1,1,1,1 access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,base.group_system,1,1,1,1 access_test_new_api_recursive_tree,access_test_new_api_recursive_tree,model_test_new_api_recursive_tree,base.group_system,1,1,1,1 +access_test_new_api_recursive_order,access_test_new_api_recursive_order,model_test_new_api_recursive_order,base.group_system,1,1,1,1 +access_test_new_api_recursive_line,access_test_new_api_recursive_line,model_test_new_api_recursive_line,base.group_system,1,1,1,1 +access_test_new_api_recursive_task,access_test_new_api_recursive_task,model_test_new_api_recursive_task,base.group_system,1,1,1,1 access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,base.group_system,1,1,1,1 access_test_new_api_compute_readwrite,access_test_new_api_compute_readwrite,model_test_new_api_compute_readwrite,base.group_system,1,1,1,1 access_test_new_api_compute_onchange,access_test_new_api_compute_onchange,model_test_new_api_compute_onchange,base.group_system,1,1,1,1 diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index d2f5370f1e0..8a71df105c0 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -505,6 +505,25 @@ class TestFields(TransactionCaseWithUserDemo): baz = foo.create({'name': 'baz', 'parent_id': bar.id}) self.assertEqual(foo.display_name, 'foo(bar(baz()))') + def test_12_recursive_unlink(self): + order = self.env['test_new_api.recursive.order'].create({'value': 42}) + line = self.env['test_new_api.recursive.line'].create({'order_id': order.id}) + task = self.env['test_new_api.recursive.task'].create({'value': 42}) + self.assertEqual(task.line_id, line) + self.assertEqual(line.task_ids, task) + self.assertTrue(line.task_number) + + # Before deleting order, the following are marked to recompute: + # - task.line_id (recursive, depends on task.line_id.order_id.value) + # - line.task_number (implicitely depends on line.task_ids.line_id) + # + # If task.line_id is ever recomputed in order to mark line.task_number, + # its recomputed value will be lost in the cache invalidation, and + # there will be nothing left to write in the database afterwards! This + # makes the call to unlink() crash in that case. + # + order.unlink() + def test_12_cascade(self): """ test computed field depending on computed field """ message = self.env.ref('test_new_api.message_0_0') diff --git a/odoo/models.py b/odoo/models.py index a8e3c362e82..700c4055d2c 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -6677,6 +6677,63 @@ class BaseModel(metaclass=MetaModel): # - mark I to recompute on inverse(W, inverse(X, records)), # - mark J to recompute on inverse(Y, records). + if before: + # When called before modification, we should determine what + # currently depends on self, and it should not be recomputed before + # the modification. So we only collect what should be marked for + # recomputation. + marked = self.env.all.tocompute # {field: ids} + tomark = defaultdict(OrderedSet) # {field: ids} + else: + # When called after modification, one should traverse backwards + # dependencies by taking into account all fields already known to + # be recomputed. In that case, we mark fieds to compute as soon as + # possible. + marked = {} + tomark = self.env.all.tocompute + + # determine what to trigger (with iterators) + todo = [self._modified([self._fields[fname] for fname in fnames], create)] + + # process what to trigger by lazily chaining todo + for field, records, create in itertools.chain.from_iterable(todo): + records -= self.env.protected(field) + if not records: + continue + + if field.recursive: + # discard already processed records, in order to avoid cycles + if field.compute and field.store: + ids = (marked.get(field) or set()) | (tomark.get(field) or set()) + records = records.browse(id_ for id_ in records._ids if id_ not in ids) + else: + records = records & self.env.cache.get_records(records, field) + if not records: + continue + # recursively trigger recomputation of field's dependents + todo.append(records._modified([field], create)) + + # mark for recomputation (now or later, depending on 'before') + if field.compute and field.store: + tomark[field].update(records._ids) + else: + # Don't force the recomputation of compute fields which are + # not stored as this is not really necessary. + self.env.cache.invalidate([(field, records._ids)]) + + if before: + # effectively mark for recomputation now + for field, ids in tomark.items(): + records = self.env[field.model_name].browse(ids) + self.env.add_to_compute(field, records) + + def _modified(self, fields, create): + """ Return an iterator traversing a tree of field triggers on ``self``, + traversing backwards field dependencies along the way, and yielding + tuple ``(field, records, created)`` to recompute. + """ + cache = self.env.cache + # The fields' trigger trees are merged in order to evaluate all triggers # at once. For non-stored computed fields, `_modified_triggers` might # traverse the tree (at the cost of extra queries) only to know which @@ -6684,47 +6741,14 @@ class BaseModel(metaclass=MetaModel): # fields have no data in cache, so they can be ignored from the start. # This allows us to discard subtrees from the merged tree when they # only contain such fields. - cache = self.env.cache - tree = self.pool.get_trigger_tree( - [self._fields[fname] for fname in fnames], - select=lambda field: (field.compute and field.store) or cache.contains_field(field), - ) + def select(field): + return (field.compute and field.store) or cache.contains_field(field) + + tree = self.pool.get_trigger_tree(fields, select=select) if not tree: - return + return () - # determine what to compute (through an iterator) - tocompute = self.sudo().with_context(active_test=False)._modified_triggers(tree, create) - - # When called after modification, one should traverse backwards - # dependencies by taking into account all fields already known to be - # recomputed. In that case, we mark fieds to compute as soon as - # possible. - # - # When called before modification, one should mark fields to compute - # after having inversed all dependencies. This is because we - # determine what currently depends on self, and it should not be - # recomputed before the modification! - if before: - tocompute = list(tocompute) - - # process what to compute - for field, records, create in tocompute: - records -= self.env.protected(field) - if not records: - continue - if field.compute and field.store: - if field.recursive: - recursively_marked = self.env.not_to_compute(field, records) - self.env.add_to_compute(field, records) - else: - # Don't force the recomputation of compute fields which are - # not stored as this is not really necessary. - if field.recursive: - recursively_marked = records & self.env.cache.get_records(records, field) - self.env.cache.invalidate([(field, records._ids)]) - # recursively trigger recomputation of field's dependents - if field.recursive: - recursively_marked.modified([field.name], create) + return self.sudo().with_context(active_test=False)._modified_triggers(tree, create) def _modified_triggers(self, tree, create=False): """ Return an iterator traversing a tree of field triggers on ``self``,