From 62355b6ad3e2af16efabe1c3ca2fb053a6084bc1 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Wed, 8 Jul 2020 13:26:27 +0000 Subject: [PATCH] [FIX] test_new_api: Fix bad shared cache usage for stored computed fields Purpose ======= Since the 13.0, a regression has been introduced in hr_timesheet. Considering: - The account.analytic.line model, representing a timesheet entry - The project.task model, representing a task, on which we have the fields: - timesheet_ids: One2many - planned_hours: Computed (sudo), stored, representing the sum of the timesheet amounts - An ir.rule restricting the timesheets CRUD to his own only. A user can only see its own timesheets on a task, but the field "Planned Hours", which is stored-compute_sudo, should take all the timesheet lines into account However, when adding a new line and then recomputing the value, no existing line from another user is binded on self, then the value is erased and saved on the database. Specification ============= This commit introduces a test that illustrates that bad usage of a shared cache. A correct way to fix this could be to avoid using the cache if an ir.rule is pointing to the related model. That would force the recomputation in a real sudoed environment. closes odoo/odoo#55408 Taskid: 2285924 X-original-commit: 50e229fe66b2a82717d4260287e18a875bbd26f5 Signed-off-by: Yannick Tivisse (yti) --- odoo/addons/test_new_api/__manifest__.py | 1 + .../test_new_api/models/test_new_api.py | 26 ++++++++++++++ .../test_new_api/security/ir.model.access.csv | 2 ++ .../security/test_new_api_security.xml | 11 ++++++ .../test_new_api/tests/test_new_fields.py | 36 +++++++++++++++++++ .../test_new_api/views/test_new_api_views.xml | 22 ++++++++++++ 6 files changed, 98 insertions(+) create mode 100644 odoo/addons/test_new_api/security/test_new_api_security.xml diff --git a/odoo/addons/test_new_api/__manifest__.py b/odoo/addons/test_new_api/__manifest__.py index bd7e8f7fb69..8b60f6dabc3 100644 --- a/odoo/addons/test_new_api/__manifest__.py +++ b/odoo/addons/test_new_api/__manifest__.py @@ -11,6 +11,7 @@ 'auto_install': False, 'data': [ 'security/ir.model.access.csv', + 'security/test_new_api_security.xml', 'views/test_new_api_views.xml', 'views/test_new_api_templates.xml', 'data/test_new_api_data.xml', 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 0ceb37a0988..602173c4145 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -1052,3 +1052,29 @@ class SelectionNonStored(models.Model): ('foo', "Foo"), ('bar', "Bar"), ], store=False) + + +# Special classes to ensure the correct usage of a shared cache amongst users. +# See the method test_shared_cache_computed_field +class SharedCacheComputeParent(models.Model): + _name = 'test_new_api.model_shared_cache_compute_parent' + _description = 'model_shared_cache_compute_parent' + + name = fields.Char(string="Task Name") + line_ids = fields.One2many( + 'test_new_api.model_shared_cache_compute_line', 'parent_id', string="Timesheets") + total_amount = fields.Integer(compute='_compute_total_amount', store=True, compute_sudo=True) + + @api.depends('line_ids.amount') + def _compute_total_amount(self): + for parent in self: + parent.total_amount = sum(parent.line_ids.mapped('amount')) + + +class ShareCacheComputeLine(models.Model): + _name = 'test_new_api.model_shared_cache_compute_line' + _description = 'model_shared_cache_compute_line' + + parent_id = fields.Many2one('test_new_api.model_shared_cache_compute_parent') + amount = fields.Integer() + user_id = fields.Many2one('res.users', default= lambda self: self.env.user) # Note: There is an ir.rule about this. 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 7133940fc92..be706801d98 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -60,3 +60,5 @@ access_test_new_api_model_selection_required,access_test_new_api_model_selection access_test_new_api_model_selection_non_stored,access_test_new_api_model_selection_non_stored,model_test_new_api_model_selection_non_stored,,1,1,1,1 access_test_new_api_move,access_test_new_api_move,model_test_new_api_move,,1,1,1,1 access_test_new_api_move_line,access_test_new_api_move_line,model_test_new_api_move_line,,1,1,1,1 +access_test_new_api_model_shared_cache_compute_parent,access_test_new_api.model_shared_cache_compute_parent,model_test_new_api_model_shared_cache_compute_parent,,1,1,1,1 +access_test_new_api_model_shared_cache_compute_line,access_test_new_api.model_shared_cache_compute_line,model_test_new_api_model_shared_cache_compute_line,,1,1,1,1 diff --git a/odoo/addons/test_new_api/security/test_new_api_security.xml b/odoo/addons/test_new_api/security/test_new_api_security.xml new file mode 100644 index 00000000000..56f591b9374 --- /dev/null +++ b/odoo/addons/test_new_api/security/test_new_api_security.xml @@ -0,0 +1,11 @@ + + + + + model_shared_cache_compute_line: See own lines + + [('user_id', '=', user.id)] + + + + 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 c3a7005cab0..5591ce7e4c5 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -2947,3 +2947,39 @@ class TestComputeQueries(common.TransactionCase): record = model.create({'foo': 'Foo', 'bar': 'Bar'}) self.assertEqual(record.foo, 'Bar') self.assertEqual(record.bar, 'Bar') + +class test_shared_cache(TransactionCaseWithUserDemo): + def test_shared_cache_computed_field(self): + # Test case: Check that the shared cache is not used if a compute_sudo stored field + # is computed IF there is an ir.rule defined on this specific model. + + # Real life example: + # A user can only see its own timesheets on a task, but the field "Planned Hours", + # which is stored-compute_sudo, should take all the timesheet lines into account + # However, when adding a new line and then recomputing the value, no existing line + # from another user is binded on self, then the value is erased and saved on the + # database. + + task = self.env['test_new_api.model_shared_cache_compute_parent'].create({ + 'name': 'Shared Task'}) + self.env['test_new_api.model_shared_cache_compute_line'].create({ + 'user_id': self.env.ref('base.user_admin').id, + 'parent_id': task.id, + 'amount': 1, + }) + self.assertEqual(task.total_amount, 1) + + self.env['base'].flush() + task.invalidate_cache() # Start fresh, as it would be the case on 2 different sessions. + + task = task.with_user(self.user_demo) + with common.Form(task) as task_form: + # Use demo has no access to the already existing line + self.assertEqual(len(task_form.line_ids), 0) + # But see the real total_amount + self.assertEqual(task_form.total_amount, 1) + # Now let's add a new line (and retrigger the compute method) + with task_form.line_ids.new() as line: + line.amount = 2 + # The new value for total_amount, should be 3, not 2. + self.assertEqual(task_form.total_amount, 2) diff --git a/odoo/addons/test_new_api/views/test_new_api_views.xml b/odoo/addons/test_new_api/views/test_new_api_views.xml index c0ed0a7baa7..cd995456def 100644 --- a/odoo/addons/test_new_api/views/test_new_api_views.xml +++ b/odoo/addons/test_new_api/views/test_new_api_views.xml @@ -319,5 +319,27 @@ + + + model_shared_cache_compute_parent_view_form + test_new_api.model_shared_cache_compute_parent + +
+ + + + + + + + + + + + + +
+ +