From dd8a6b8a8299afaac699d04f2ed886ff85c685fe Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Thu, 2 Jul 2020 08:56:08 +0000 Subject: [PATCH] [ADD] test_new_api: tests on queries made by create with computed fields closes odoo/odoo#54209 Related: odoo/upgrade#1464 Signed-off-by: Raphael Collet (rco) --- .../test_new_api/models/test_new_api.py | 23 +++- .../test_new_api/security/ir.model.access.csv | 3 +- .../test_new_api/tests/test_new_fields.py | 107 +++++++++++++++++- odoo/models.py | 2 +- odoo/tests/common.py | 9 +- 5 files changed, 136 insertions(+), 8 deletions(-) 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 ad0085f82b3..0ceb37a0988 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -379,9 +379,10 @@ class Related(models.Model): message_name = fields.Text(related="message.body", related_sudo=False, string='Message Body') message_currency = fields.Many2one(related="message.author", string='Message Author') -class ComputeProtected(models.Model): - _name = 'test_new_api.compute.protected' - _description = 'Test New API Compute Protected' + +class ComputeReadonly(models.Model): + _name = 'test_new_api.compute.readonly' + _description = 'Model with a computed readonly field' foo = fields.Char(default='') bar = fields.Char(compute='_compute_bar', store=True) @@ -391,9 +392,10 @@ class ComputeProtected(models.Model): for record in self: record.bar = record.foo + class ComputeInverse(models.Model): _name = 'test_new_api.compute.inverse' - _description = 'Test New API Compute Inversse' + _description = 'Model with a computed inversed field' foo = fields.Char() bar = fields.Char(compute='_compute_bar', inverse='_inverse_bar', store=True) @@ -520,6 +522,19 @@ class ComputeCascade(models.Model): record.baz = "<%s>" % (record.bar or "") +class ComputeReadWrite(models.Model): + _name = 'test_new_api.compute.readwrite' + _description = 'Model with a computed non-readonly field' + + foo = fields.Char() + bar = fields.Char(compute='_compute_bar', store=True, readonly=False) + + @api.depends('foo') + def _compute_bar(self): + for record in self: + record.bar = record.foo + + class ComputeOnchange(models.Model): _name = 'test_new_api.compute.onchange' _description = "Compute method as an onchange" 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 6063ecef49f..7133940fc92 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -19,10 +19,11 @@ access_test_new_api_related,access_test_new_api_related,model_test_new_api_relat access_test_new_api_company,access_test_new_api_company,model_test_new_api_company,,1,1,1,1 access_test_new_api_company_attr,access_test_new_api_company_attr,model_test_new_api_company_attr,,1,1,1,1 access_test_new_api_compute_inverse,access_test_new_api_compute_inverse,model_test_new_api_compute_inverse,,1,1,1,1 -access_test_new_api_compute_protected,access_test_new_api_compute_protected,model_test_new_api_compute_protected,,1,1,1,1 +access_test_new_api_compute_readonly,access_test_new_api_compute_readonly,model_test_new_api_compute_readonly,,1,1,1,1 access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inverse,model_test_new_api_multi_compute_inverse,,1,1,1,1 access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,,1,1,1,1 access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,,1,1,1,1 +access_test_new_api_compute_readwrite,access_test_new_api_compute_readwrite,model_test_new_api_compute_readwrite,,1,1,1,1 access_test_new_api_compute_onchange,access_test_new_api_compute_onchange,model_test_new_api_compute_onchange,,1,1,1,1 access_test_new_api_compute_onchange_line,access_test_new_api_compute_onchange_line,model_test_new_api_compute_onchange_line,,1,1,1,1 access_test_new_api_binary_svg,access_test_new_api_binary_svg,model_test_new_api_binary_svg,,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 a42ce7a9d85..b071e043414 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -322,7 +322,7 @@ class TestFields(TransactionCaseWithUserDemo): def test_11_stored_protected(self): """ test protection against recomputation """ - model = self.env['test_new_api.compute.protected'] + model = self.env['test_new_api.compute.readonly'] field = model._fields['bar'] record = model.create({'foo': 'unprotected #1'}) @@ -2826,3 +2826,108 @@ class TestSelectionOndeleteAdvanced(common.TransactionCase): with self.assertRaises(ValueError): self.registry.setup_models(self.env.cr) + + +def insert(model, *fnames): + """ Return the expected query string to INSERT the given columns. """ + columns = ['create_uid', 'create_date', 'write_uid', 'write_date'] + sorted(fnames) + return 'INSERT INTO "{}" ("id", {}) VALUES (nextval(%s), {}) RETURNING id'.format( + model._table, + ", ".join('"{}"'.format(column) for column in columns), + ", ".join('%s' for column in columns), + ) + + +def update(model, *fnames): + """ Return the expected query string to UPDATE the given columns. """ + columns = sorted(fnames) + ['write_uid', 'write_date'] + return 'UPDATE "{}" SET {} WHERE id IN %s'.format( + model._table, + ", ".join('"{}" = %s'.format(column) for column in columns), + ) + + +class TestComputeQueries(common.TransactionCase): + """ Test the queries made by create() with computed fields. """ + + def test_compute_readonly(self): + model = self.env['test_new_api.compute.readonly'] + model.create({}) + + # no value, no default + with self.assertQueries([insert(model, 'foo'), update(model, 'bar')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.bar, 'Foo') + + # some value, no default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'bar')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.bar, 'Foo') + + model = model.with_context(default_bar='Def') + + # no value, some default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'bar')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.bar, 'Foo') + + # some value, some default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'bar')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.bar, 'Foo') + + def test_compute_readwrite(self): + model = self.env['test_new_api.compute.readwrite'] + model.create({}) + + # no value, no default + with self.assertQueries([insert(model, 'foo'), update(model, 'bar')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.bar, 'Foo') + + # some value, no default + with self.assertQueries([insert(model, 'foo', 'bar')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.bar, 'Bar') + + model = model.with_context(default_bar='Def') + + # no value, some default + with self.assertQueries([insert(model, 'foo', 'bar')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.bar, 'Def') + + # some value, some default + with self.assertQueries([insert(model, 'foo', 'bar')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.bar, 'Bar') + + def test_compute_inverse(self): + model = self.env['test_new_api.compute.inverse'] + model.create({}) + + # no value, no default + with self.assertQueries([insert(model, 'foo'), update(model, 'bar')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.foo, 'Foo') + self.assertEqual(record.bar, 'Foo') + + # some value, no default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'foo')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.foo, 'Bar') + self.assertEqual(record.bar, 'Bar') + + model = model.with_context(default_bar='Def') + + # no value, some default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'foo')]): + record = model.create({'foo': 'Foo'}) + self.assertEqual(record.foo, 'Def') + self.assertEqual(record.bar, 'Def') + + # some value, some default + with self.assertQueries([insert(model, 'foo', 'bar'), update(model, 'foo')]): + record = model.create({'foo': 'Foo', 'bar': 'Bar'}) + self.assertEqual(record.foo, 'Bar') + self.assertEqual(record.bar, 'Bar') diff --git a/odoo/models.py b/odoo/models.py index 4988bf4eada..99a45c0de00 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3633,7 +3633,7 @@ Record ids: %(records)s # determine SQL values columns = [] # list of (column_name, format, value) - for name, val in vals.items(): + for name, val in sorted(vals.items()): if self._log_access and name in LOG_ACCESS_COLUMNS and not val: continue field = self._fields[name] diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 84ab36e5008..dc557682709 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -381,7 +381,7 @@ class BaseCase(TreeCase, MetaCase('DummyCase', (object,), {})): return self._assertRaises(exception, **kwargs) @contextmanager - def assertQueries(self, expected): + def assertQueries(self, expected, flush=True): """ Check the queries made by the current cursor. ``expected`` is a list of strings representing the expected queries being made. Query strings are matched against each other, ignoring case and whitespaces. @@ -396,9 +396,16 @@ class BaseCase(TreeCase, MetaCase('DummyCase', (object,), {})): def get_unaccent_wrapper(cr): return lambda x: x + if flush: + self.env.user.flush() + self.env.cr.precommit() + with patch('odoo.sql_db.Cursor.execute', execute): with patch('odoo.osv.expression.get_unaccent_wrapper', get_unaccent_wrapper): yield actual_queries + if flush: + self.env.user.flush() + self.env.cr.precommit() self.assertEqual( len(actual_queries), len(expected),