diff --git a/addons/sale_coupon/models/sale_order.py b/addons/sale_coupon/models/sale_order.py index 8606cff345d..0f32932a1eb 100644 --- a/addons/sale_coupon/models/sale_order.py +++ b/addons/sale_coupon/models/sale_order.py @@ -471,8 +471,8 @@ class SaleOrderLine(models.Model): # Another possibility is to add on product.product a one2many to sale.order.line 'order_line_ids', # and then add the depends @api.depends('discount_line_product_id.order_line_ids'), # but I am not sure this will as efficient as the below. - def modified(self, fnames, create=False): - super(SaleOrderLine, self).modified(fnames, create) + def modified(self, fnames, *args, **kwargs): + super(SaleOrderLine, self).modified(fnames, *args, **kwargs) if 'product_id' in fnames: Program = self.env['coupon.program'].sudo() field_order_count = Program._fields['order_count'] 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 a465cc15fe5..08995bf9cce 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -493,8 +493,17 @@ class ComputeRecursive(models.Model): name = fields.Char(required=True) parent = fields.Many2one('test_new_api.recursive', ondelete='cascade') + full_name = fields.Char(compute='_compute_full_name') display_name = fields.Char(compute='_compute_display_name', store=True) + @api.depends('name', 'parent.full_name') + def _compute_full_name(self): + for rec in self: + if rec.parent: + rec.full_name = rec.parent.full_name + " / " + rec.name + else: + rec.full_name = rec.name + @api.depends('name', 'parent.display_name') def _compute_display_name(self): for rec in self: @@ -792,7 +801,7 @@ class Attachment(models.Model): rec.name = self.env[rec.res_model].browse(rec.res_id).display_name # DLE P55: `test_cache_invalidation` - def modified(self, fnames, create=False): + def modified(self, fnames, *args, **kwargs): if not self: return comodel = self.env[self.res_model] @@ -801,7 +810,7 @@ class Attachment(models.Model): record = comodel.browse(self.res_id) self.env.cache.invalidate([(field, record._ids)]) record.modified(['attachment_ids']) - return super(Attachment, self).modified(fnames, create) + return super(Attachment, self).modified(fnames, *args, **kwargs) class AttachmentHost(models.Model): @@ -1209,3 +1218,32 @@ class ConstrainedUnlinks(models.Model): for rec in self: if rec.foo and rec.foo == 'prosciutto': raise ValueError("You didn't say if you wanted it crudo or cotto...") + + +class TriggerLeft(models.Model): + _name = 'test_new_api.trigger.left' + _description = 'model with a related many2one' + + middle_ids = fields.One2many('test_new_api.trigger.middle', 'left_id') + right_id = fields.Many2one(related='middle_ids.right_id', store=True) + + +class TriggerMiddle(models.Model): + _name = 'test_new_api.trigger.middle' + _description = 'model linking test_new_api.trigger.left and test_new_api.trigger.right' + + left_id = fields.Many2one('test_new_api.trigger.left', required=True) + right_id = fields.Many2one('test_new_api.trigger.right', required=True) + + +class TriggerRight(models.Model): + _name = 'test_new_api.trigger.right' + _description = 'model with a dependency on the inverse of the related many2one' + + left_ids = fields.One2many('test_new_api.trigger.left', 'right_id') + left_size = fields.Integer(compute='_compute_left_size', store=True) + + @api.depends('left_ids') + def _compute_left_size(self): + for record in self: + record.left_size = len(record.left_ids) 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 7b313690904..946c10e27c4 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -69,3 +69,6 @@ access_test_new_api_compute_member,access_test_new_api_compute_member,model_test access_test_new_api_compute_editable,access_test_new_api_compute_editable,model_test_new_api_compute_editable,,1,1,1,1 access_test_new_api_compute_editable_line,access_test_new_api_compute_editable_line,model_test_new_api_compute_editable_line,,1,1,1,1 access_test_new_api_model_constrained_unlinks,access_test_new_api_model_constrained_unlinks,model_test_new_api_model_constrained_unlinks,,1,1,1,1 +access_test_new_api_trigger_left,access_test_new_api_trigger_left,model_test_new_api_trigger_left,,1,1,1,1 +access_test_new_api_trigger_middle,access_test_new_api_trigger_middle,model_test_new_api_trigger_middle,,1,1,1,1 +access_test_new_api_trigger_right,access_test_new_api_trigger_right,model_test_new_api_trigger_right,,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 d980c7a020a..e0335c2f201 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -450,20 +450,42 @@ class TestFields(TransactionCaseWithUserDemo): b = self.env['test_new_api.recursive'].create({'name': 'B', 'parent': a.id}) c = self.env['test_new_api.recursive'].create({'name': 'C', 'parent': b.id}) d = self.env['test_new_api.recursive'].create({'name': 'D', 'parent': c.id}) + self.assertEqual(a.full_name, 'A') + self.assertEqual(b.full_name, 'A / B') + self.assertEqual(c.full_name, 'A / B / C') + self.assertEqual(d.full_name, 'A / B / C / D') self.assertEqual(a.display_name, 'A') self.assertEqual(b.display_name, 'A / B') self.assertEqual(c.display_name, 'A / B / C') self.assertEqual(d.display_name, 'A / B / C / D') + a.name = 'A1' + self.assertEqual(a.full_name, 'A1') + self.assertEqual(b.full_name, 'A1 / B') + self.assertEqual(c.full_name, 'A1 / B / C') + self.assertEqual(d.full_name, 'A1 / B / C / D') + self.assertEqual(a.display_name, 'A1') + self.assertEqual(b.display_name, 'A1 / B') + self.assertEqual(c.display_name, 'A1 / B / C') + self.assertEqual(d.display_name, 'A1 / B / C / D') + b.parent = False - self.assertEqual(a.display_name, 'A') + self.assertEqual(a.full_name, 'A1') + self.assertEqual(b.full_name, 'B') + self.assertEqual(c.full_name, 'B / C') + self.assertEqual(d.full_name, 'B / C / D') + self.assertEqual(a.display_name, 'A1') self.assertEqual(b.display_name, 'B') self.assertEqual(c.display_name, 'B / C') self.assertEqual(d.display_name, 'B / C / D') # rename several records to trigger several recomputations at once (d + c + b).write({'name': 'X'}) - self.assertEqual(a.display_name, 'A') + self.assertEqual(a.full_name, 'A1') + self.assertEqual(b.full_name, 'X') + self.assertEqual(c.full_name, 'X / X') + self.assertEqual(d.full_name, 'X / X / X') + self.assertEqual(a.display_name, 'A1') self.assertEqual(b.display_name, 'X') self.assertEqual(c.display_name, 'X / X') self.assertEqual(d.display_name, 'X / X / X') @@ -1131,6 +1153,32 @@ class TestFields(TransactionCaseWithUserDemo): self.assertEqual(bar.foo, oof) self.assertIn(bar, bar.search([('foo', 'in', oof.ids)])) + def test_25_one2many_inverse_related(self): + left = self.env['test_new_api.trigger.left'].create({}) + right = self.env['test_new_api.trigger.right'].create({}) + self.assertFalse(left.right_id) + self.assertFalse(right.left_ids) + self.assertFalse(right.left_size) + + # create middle: this should trigger left.right_id by traversing + # middle.left_id, and right.left_size by traversing left.right_id + # after its computation! + middle = self.env['test_new_api.trigger.middle'].create({ + 'left_id': left.id, + 'right_id': right.id, + }) + self.assertEqual(left.right_id, right) + self.assertEqual(right.left_ids, left) + self.assertEqual(right.left_size, 1) + + # delete middle: this should trigger left.right_id by traversing + # middle.left_id, and right.left_size by traversing left.right_id + # before its computation! + middle.unlink() + self.assertFalse(left.right_id) + self.assertFalse(right.left_ids) + self.assertFalse(right.left_size) + def test_26_inherited(self): """ test inherited fields. """ # a bunch of fields are inherited from res_partner diff --git a/odoo/fields.py b/odoo/fields.py index 99bb26f5581..a515fd7ea5c 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1105,10 +1105,10 @@ class Field(MetaField('DummyField', (object,), {})): # new records: no business logic new_records = records.browse(new_ids) with records.env.protecting(records.pool.field_computed.get(self, [self]), records): - new_records.modified([self.name]) - self.write(new_records, value) if self.relational: - new_records.modified([self.name]) + new_records.modified([self.name], before=True) + self.write(new_records, value) + new_records.modified([self.name]) if self.inherited: # special case: also assign parent records if they are new diff --git a/odoo/models.py b/odoo/models.py index 7cd8f52172a..2fdc0db37eb 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3437,7 +3437,7 @@ Fields: # mark fields that depend on 'self' to recompute them after 'self' has # been deleted (like updating a sum of lines after deleting one line) self.flush() - self.modified(self._fields) + self.modified(self._fields, before=True) with self.env.norecompute(): self.check_access_rule('unlink') @@ -3618,8 +3618,25 @@ Fields: # protect fields being written against recomputation with env.protecting(protected, self): - # determine records depending on values - self.modified(relational_names) + # Determine records depending on values. When modifying a relational + # field, you have to recompute what depends on the field's values + # before and after modification. This is because the modification + # has an impact on the "data path" between a computed field and its + # dependency. Note that this double call to modified() is only + # necessary for relational fields. + # + # It is best explained with a simple example: consider two sales + # orders SO1 and SO2. The computed total amount on sales orders + # indirectly depends on the many2one field 'order_id' linking lines + # to their sales order. Now consider the following code: + # + # line = so1.line_ids[0] # pick a line from SO1 + # line.order_id = so2 # move the line to SO2 + # + # In this situation, the total amount must be recomputed on *both* + # sales order: the line's order before the modification, and the + # line's order after the modification. + self.modified(relational_names, before=True) real_recs = self.filtered('id') @@ -5707,14 +5724,14 @@ Fields: [(invf, None) for f in fields for invf in self._field_inverses[f]] self.env.cache.invalidate(spec) - def modified(self, fnames, create=False): - """ Notify that fields have been modified on ``self``. This invalidates - the cache, and prepares the recomputation of stored function fields - (new-style fields only). + def modified(self, fnames, create=False, before=False): + """ Notify that fields will be or have been modified on ``self``. This + invalidates the cache where necessary, and prepares the recomputation of + dependent stored fields. - :param fnames: iterable of field names that have been modified on - records ``self`` - :param create: whether modified is called in the context of record creation + :param fnames: iterable of field names modified on records ``self`` + :param create: whether called in the context of record creation + :param before: whether called before modifying records ``self`` """ if not self or not fnames: return @@ -5747,34 +5764,59 @@ Fields: node = self.pool.field_triggers.get(self._fields[fname]) if node: trigger_tree_merge(tree, node) + if tree: - self.sudo().with_context(active_test=False)._modified_triggers(tree, create) + # 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 in tocompute: + records -= self.env.protected(field) + if not records: + continue + # Dont force the recomputation of compute fields which are + # not stored as this is not really necessary. + recursive = not create and field.recursive + if field.compute and field.store: + if recursive: + marked_records = self.env.not_to_compute(field, records) + self.env.add_to_compute(field, records) + else: + if recursive: + marked_records = records & self.env.cache.get_records(records, field) + self.env.cache.invalidate([(field, records._ids)]) + # recursively trigger recomputation of field's dependents + if recursive: + marked_records.modified([field.name]) def _modified_triggers(self, tree, create=False): - """ Process a tree of field triggers on ``self``. """ + """ Return an iterator traversing a tree of field triggers on ``self``, + traversing backwards field dependencies along the way, and yielding + pairs ``(field, records)`` to recompute. + """ if not self: return + + # first yield what to compute + for field in tree.get(None, ()): + yield field, self + + # then traverse dependencies backwards, and proceed recursively for key, val in tree.items(): if key is None: - # val is a list of fields to mark as todo - for field in val: - records = self - self.env.protected(field) - if not records: - continue - # Dont force the recomputation of compute fields which are - # not stored as this is not really necessary. - recursive = not create and field.recursive - if field.compute and field.store: - if recursive: - added = self.env.not_to_compute(field, records) - self.env.add_to_compute(field, records) - else: - if recursive: - added = self & self.env.cache.get_records(self, field) - self.env.cache.invalidate([(field, records._ids)]) - # recursively trigger recomputation of field's dependents - if recursive: - added.modified([field.name]) + continue elif create and key.type in ('many2one', 'many2one_reference'): # upon creation, no other record has a reference to self continue @@ -5814,7 +5856,7 @@ Fields: if new_records: cache_records = self.env.cache.get_records(model, key) records |= cache_records.filtered(lambda r: set(r[key.name]._ids) & set(self._ids)) - records._modified_triggers(val) + yield from records._modified_triggers(val) @api.model def recompute(self, fnames=None, records=None):