From eaccecd6cc2e21753240b44a42a2040d82de0fe1 Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Thu, 30 Jan 2020 15:31:19 +0000 Subject: [PATCH 1/5] [IMP] base: correctly escape identifiers in reflection queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the db reflects the python models at startup, a query is generated to update various `ir` models (models, fields, etc.). This query did not properly escape identifiers, preventing the use of the 'order' field name on ir.model because it is a reserved keyword in SQL and wasn't escaped. This commit introduces proper escaping for these reflection queries. Co-Authored-By: Raphaël Collet --- odoo/addons/base/models/ir_model.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 8056c35cf03..7d7a93d7f87 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -47,8 +47,8 @@ def query_insert(cr, table, rows): rows = [rows] cols = list(rows[0]) query = INSERT_QUERY.format( - table=table, - cols=",".join(cols), + table='"{}"'.format(table), + cols=",".join(['"{}"'.format(col) for col in cols]), rows=",".join("%s" for row in rows), ) params = [tuple(row[col] for col in cols) for row in rows] @@ -61,9 +61,9 @@ def query_update(cr, table, values, selectors): """ setters = set(values) - set(selectors) query = UPDATE_QUERY.format( - table=table, - assignment=",".join("{0}=%({0})s".format(s) for s in setters), - condition=" AND ".join("{0}=%({0})s".format(s) for s in selectors), + table='"{}"'.format(table), + assignment=",".join('"{0}"=%({0})s'.format(s) for s in setters), + condition=" AND ".join('"{0}"=%({0})s'.format(s) for s in selectors), ) cr.execute(query, values) return [row[0] for row in cr.fetchall()] From c643be7679921026a87ff307d57b38c174ec439e Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Thu, 30 Jan 2020 15:32:13 +0000 Subject: [PATCH 2/5] [IMP] base: ensure correct inheritance chain in registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When removing a custom model, it remained listed in the base classes (normally `base` and also possibly a list of mixins; e.g. `mail.thread` or `mail.activity.mixin`) list of inheriting classes, possibly causing a crash when trying to reload the registry. This commit ensures that any custom model is removed from its parent class `_inherit_children` set; it will be re-added automatically during the call to `_build_model` if the custom model still exists. Co-Authored-By: Raphaël Collet --- odoo/addons/base/models/ir_model.py | 2 +- odoo/modules/registry.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 7d7a93d7f87..76477936439 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -308,7 +308,7 @@ class IrModel(models.Model): # clean up registry first custom_models = [name for name, model_class in self.pool.items() if model_class._custom] for name in custom_models: - del self.pool.models[name] + del self.pool[name] # add manual models cr = self.env.cr cr.execute('SELECT * FROM ir_model WHERE state=%s', ['manual']) diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py index dc03d0b24d3..ddb1631ec51 100644 --- a/odoo/modules/registry.py +++ b/odoo/modules/registry.py @@ -183,6 +183,13 @@ class Registry(Mapping): """ Add or replace a model in the registry.""" self.models[model_name] = model + def __delitem__(self, model_name): + """ Remove a (custom) model from the registry. """ + del self.models[model_name] + # the custom model can inherit from mixins ('mail.thread', ...) + for Model in self.models.values(): + Model._inherit_children.discard(model_name) + def descendants(self, model_names, *kinds): """ Return the models corresponding to ``model_names`` and all those that inherit/inherits from them. From 8dab6caf468bde2cab03b89c6c48dd8a9ff1cf52 Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Thu, 30 Jan 2020 15:32:54 +0000 Subject: [PATCH 3/5] [IMP] tests: add registry cleanups in Single and Savepoint tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, the registry might be updated during a test step (creating custom models/fields, for example). In those cases, there should be an explicit call to `reset_changes` on the registry to make sure that the next test class starts with a registry that is consistent with the database state. Co-Authored-By: Raphaël Collet --- odoo/tests/common.py | 1 + 1 file changed, 1 insertion(+) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 447171cfd55..6dbc576b7be 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -560,6 +560,7 @@ class SingleTransactionCase(BaseCase): def setUpClass(cls): super().setUpClass() cls.registry = odoo.registry(get_db_name()) + cls.addClassCleanup(cls.registry.reset_changes) cls.addClassCleanup(cls.registry.clear_caches) cls.cr = cls.registry.cursor() From 4d2aa27157cf1a4ebd6a360a2309dda8da593ce5 Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Thu, 30 Jan 2020 15:35:04 +0000 Subject: [PATCH 4/5] [IMP] base: support ordering on custom models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Up until now, it was impossible to specify the default ordering on models created manually. This could somewhat be bypassed by specifying the ordering of records on views themselves, but this has one main drawback: when using a relational field that targets a custom model as a group-by key, the ordering defaulted to the id of the custom record. For example, if I create a custom field on partners that points to a custom model 'x_grade' on which an 'x_sequence' field exists, I could order my grade in their own list view according to their sequence, but any read on partners grouped by this 'x_grade_id' field would order the returned groups by id while I would prefer to have them ordered according to the 'x_sequence' field. This commit introduces a new field 'default_order' on the ir.model model that can store this default ordering clause (as an SQL expression). Co-Authored-By: Raphaël Collet --- odoo/addons/base/models/ir_model.py | 32 ++++++- odoo/addons/base/tests/test_ir_model.py | 105 +++++++++++++++++++++- odoo/addons/base/views/ir_model_views.xml | 1 + 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 76477936439..1813e76b26c 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -4,6 +4,7 @@ import datetime import dateutil import itertools import logging +import re import time from ast import literal_eval from collections import defaultdict, Mapping @@ -19,7 +20,7 @@ from odoo.tools.safe_eval import safe_eval _logger = logging.getLogger(__name__) MODULE_UNINSTALL_FLAG = '_force_unlink' - +RE_ORDER_FIELDS = re.compile(r'"?(\w+)"?\s*(?:asc|desc)?', flags=re.I) # base environment for doing a safe_eval SAFE_EVAL_BASE = { @@ -99,6 +100,8 @@ class IrModel(models.Model): name = fields.Char(string='Model Description', translate=True, required=True) model = fields.Char(default='x_', required=True, index=True) + order = fields.Char(string='Order', default='id', required=True, + help='SQL expression for ordering records in the model; e.g. "x_sequence asc, id desc"') info = fields.Text(string='Information') field_id = fields.One2many('ir.model.fields', 'model_id', string='Fields', required=True, copy=True, default=_default_field_id) @@ -156,6 +159,24 @@ class IrModel(models.Model): if not models.check_object_name(model.model): raise ValidationError(_("The model name can only contain lowercase characters, digits, underscores and dots.")) + @api.constrains('order', 'field_id') + def _check_order(self): + for model in self: + try: + model._check_qorder(model.order) # regex check for the whole clause ('is it valid sql?') + except UserError as e: + raise ValidationError(str(e)) + stored_fields = model.field_id.filtered('store').mapped('name') + if self.env.get(model.model) is None: + # model hasn't been init'd yet, which means that some fields are not yet in its + # list of fields but will be right after its creation - these fields can be used + # for ordering, so let's add them to the list of stored fields manually + stored_fields += models.MAGIC_COLUMNS + order_fields = RE_ORDER_FIELDS.findall(model.order) + for field in order_fields: + if field not in stored_fields: + raise ValidationError(_("Unable to order by %s: fields used for ordering must be present on the model and stored.") % field) + _sql_constraints = [ ('obj_name_uniq', 'unique (model)', 'Each model must be unique!'), ] @@ -238,7 +259,12 @@ class IrModel(models.Model): # writes (4,id,False) even for non dirty items. if 'field_id' in vals: vals['field_id'] = [op for op in vals['field_id'] if op[0] != 4] - return super(IrModel, self).write(vals) + res = super(IrModel, self).write(vals) + # ordering has been changed, reload registry to reflect update + signaling + if 'order' in vals: + self.flush() # setup_models need to fetch the updated values from the db + self.pool.setup_models(self._cr) + return res @api.model def create(self, vals): @@ -264,6 +290,7 @@ class IrModel(models.Model): return { 'model': model._name, 'name': model._description, + 'order': model._order, 'info': next(cls.__doc__ for cls in type(model).mro() if cls.__doc__), 'state': 'manual' if model._custom else 'base', 'transient': model._transient, @@ -299,6 +326,7 @@ class IrModel(models.Model): _module = False _custom = True _transient = bool(model_data['transient']) + _order = model_data['order'] __doc__ = model_data['info'] return CustomModel diff --git a/odoo/addons/base/tests/test_ir_model.py b/odoo/addons/base/tests/test_ir_model.py index ea5b4a5fa2e..eb52e8c8ad6 100644 --- a/odoo/addons/base/tests/test_ir_model.py +++ b/odoo/addons/base/tests/test_ir_model.py @@ -3,7 +3,8 @@ from psycopg2 import IntegrityError -from odoo.tests.common import TransactionCase +from odoo.exceptions import ValidationError +from odoo.tests.common import TransactionCase, SavepointCase from odoo.tools import mute_logger @@ -168,3 +169,105 @@ class TestXMLID(TransactionCase): }] with self.assertRaisesRegex(IntegrityError, 'ir_model_data_name_nospaces'): model._load_records(data_list) + + +class TestIrModel(SavepointCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + + # The test mode is necessary in this case. After each test, we call + # registry.reset_changes(), which opens a new cursor to retrieve custom + # models and fields. A regular cursor would correspond to the state of + # the database before setUpClass(), which is not correct. Instead, a + # test cursor will correspond to the state of the database of cls.cr at + # that point, i.e., before the call to setUp(). + cls.registry.enter_test_mode(cls.cr) + cls.addClassCleanup(cls.registry.leave_test_mode) + + # model and records for bananas + cls.bananas_model = cls.env['ir.model'].create({ + 'name': 'Bananas', + 'model': 'x_bananas', + 'field_id': [ + (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}), + (0, 0, {'name': 'x_length', 'ttype': 'float', 'field_description': 'Length'}), + (0, 0, {'name': 'x_color', 'ttype': 'integer', 'field_description': 'Color'}), + ] + }) + # add non-stored field that is not valid in order + cls.env['ir.model.fields'].create({ + 'name': 'x_is_yellow', + 'field_description': 'Is the banana yellow?', + 'ttype': 'boolean', + 'model_id': cls.bananas_model.id, + 'store': False, + 'depends': 'x_color', + 'compute': "for banana in self:\n banana['x_is_yellow'] = banana.x_color == 9" + }) + cls.env['x_bananas'].create([{ + 'x_name': 'Banana #1', + 'x_length': 3.14159, + 'x_color': 9, + }, { + 'x_name': 'Banana #2', + 'x_length': 0, + 'x_color': 6, + }, { + 'x_name': 'Banana #3', + 'x_length': 10, + 'x_color': 6, + }]) + + def setUp(self): + # this cleanup is necessary after each test, and must be done last + self.addCleanup(self.registry.reset_changes) + super().setUp() + + def test_model_order_constraint(self): + """Check that the order constraint is properly enforced.""" + VALID_ORDERS = ['id', 'id desc', 'id asc, x_length', 'x_color, x_length, create_uid'] + for order in VALID_ORDERS: + self.bananas_model.order = order + + INVALID_ORDERS = ['', 'x_wat', 'id esc', 'create_uid,', 'id, x_is_yellow'] + for order in INVALID_ORDERS: + with self.assertRaises(ValidationError), self.cr.savepoint(): + self.bananas_model.order = order + + # check that the constraint is checked at model creation + fields_value = [ + (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}), + (0, 0, {'name': 'x_length', 'ttype': 'float', 'field_description': 'Length'}), + (0, 0, {'name': 'x_color', 'ttype': 'integer', 'field_description': 'Color'}), + ] + self.env['ir.model'].create({ + 'name': 'MegaBananas', + 'model': 'x_mega_bananas', + 'order': 'x_name asc, id desc', # valid order + 'field_id': fields_value, + }) + with self.assertRaises(ValidationError): + self.env['ir.model'].create({ + 'name': 'GigaBananas', + 'model': 'x_giga_bananas', + 'order': 'x_name asc, x_wat', # invalid order + 'field_id': fields_value, + }) + + def test_model_order_search(self): + """Check that custom orders are applied when querying a model.""" + ORDERS = { + 'id asc': ['Banana #1', 'Banana #2', 'Banana #3'], + 'id desc': ['Banana #3', 'Banana #2', 'Banana #1'], + 'x_color asc, id asc': ['Banana #2', 'Banana #3', 'Banana #1'], + 'x_color asc, id desc': ['Banana #3', 'Banana #2', 'Banana #1'], + 'x_length asc, id': ['Banana #2', 'Banana #1', 'Banana #3'], + } + for order, names in ORDERS.items(): + self.bananas_model.order = order + self.assertEqual(self.env['x_bananas']._order, order) + + bananas = self.env['x_bananas'].search([]) + self.assertEqual(bananas.mapped('x_name'), names, 'failed to order by %s' % order) diff --git a/odoo/addons/base/views/ir_model_views.xml b/odoo/addons/base/views/ir_model_views.xml index 6d27c2851c8..1505b517150 100644 --- a/odoo/addons/base/views/ir_model_views.xml +++ b/odoo/addons/base/views/ir_model_views.xml @@ -31,6 +31,7 @@ + From 00205aae2a9f361eb1fc781d77ea4adb5e0a7b48 Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Thu, 30 Jan 2020 15:35:32 +0000 Subject: [PATCH 5/5] [IMP] base: support `group_expand` for custom fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow a naive `group_expand` for manual fields where having the attribute set to True causes the ORM to include all records from the relation model of the m2o field in the read_group. This is particularly useful for custom m2o fields which represent stages - a grouped list view or a kanban view should include all possible stage, not only the currently used values. Co-Authored-By: Raphaël Collet --- odoo/addons/base/models/ir_model.py | 7 +++++ odoo/addons/base/tests/test_ir_model.py | 38 +++++++++++++++++++++++ odoo/addons/base/views/ir_model_views.xml | 3 ++ odoo/models.py | 5 +++ 4 files changed, 53 insertions(+) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 1813e76b26c..a33f80e9d9b 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -397,6 +397,12 @@ class IrModelFields(models.Model): "specified as a Python expression defining a list of triplets. " "For example: [('color','=','red')]") groups = fields.Many2many('res.groups', 'ir_model_fields_group_rel', 'field_id', 'group_id') # CLEANME unimplemented field (empty table) + group_expand = fields.Boolean(string="Expand Groups", + help="If checked, all the records of the target model will be included\n" + "in a grouped result (e.g. 'Group By' filters, Kanban columns, etc.).\n" + "Note that it can significantly reduce performance if the target model\n" + "of the field contains a lot of records; usually used on models with\n" + "few records (e.g. Stages, Job Positions, Event Types, etc.).") selectable = fields.Boolean(default=True) modules = fields.Char(compute='_in_modules', string='In Apps', help='List of modules in which the field is defined') relation_table = fields.Char(help="Used for custom many2many fields to define a custom relation table name") @@ -994,6 +1000,7 @@ class IrModelFields(models.Model): attrs['comodel_name'] = field_data['relation'] attrs['ondelete'] = field_data['on_delete'] attrs['domain'] = safe_eval(field_data['domain'] or '[]') + attrs['group_expand'] = '_read_group_expand_full' if field_data['group_expand'] else None elif field_data['ttype'] == 'one2many': if not self.pool.loaded and not ( field_data['relation'] in self.env and ( diff --git a/odoo/addons/base/tests/test_ir_model.py b/odoo/addons/base/tests/test_ir_model.py index eb52e8c8ad6..bfa6ea7fd5f 100644 --- a/odoo/addons/base/tests/test_ir_model.py +++ b/odoo/addons/base/tests/test_ir_model.py @@ -186,6 +186,19 @@ class TestIrModel(SavepointCase): cls.registry.enter_test_mode(cls.cr) cls.addClassCleanup(cls.registry.leave_test_mode) + # model and records for banana stages + cls.env['ir.model'].create({ + 'name': 'Banana Ripeness', + 'model': 'x_banana_ripeness', + 'field_id': [ + (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}), + ] + }) + # stage values are pairs (id, display_name) + cls.ripeness_green = cls.env['x_banana_ripeness'].name_create('Green') + cls.ripeness_okay = cls.env['x_banana_ripeness'].name_create('Okay, I guess?') + cls.ripeness_gone = cls.env['x_banana_ripeness'].name_create('Walked away on its own') + # model and records for bananas cls.bananas_model = cls.env['ir.model'].create({ 'name': 'Bananas', @@ -194,6 +207,9 @@ class TestIrModel(SavepointCase): (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}), (0, 0, {'name': 'x_length', 'ttype': 'float', 'field_description': 'Length'}), (0, 0, {'name': 'x_color', 'ttype': 'integer', 'field_description': 'Color'}), + (0, 0, {'name': 'x_ripeness_id', 'ttype': 'many2one', + 'field_description': 'Ripeness','relation': 'x_banana_ripeness', + 'group_expand': True}) ] }) # add non-stored field that is not valid in order @@ -206,6 +222,8 @@ class TestIrModel(SavepointCase): 'depends': 'x_color', 'compute': "for banana in self:\n banana['x_is_yellow'] = banana.x_color == 9" }) + # default stage is ripeness_green + cls.env['ir.default'].set('x_bananas', 'x_ripeness_id', cls.ripeness_green[0]) cls.env['x_bananas'].create([{ 'x_name': 'Banana #1', 'x_length': 3.14159, @@ -271,3 +289,23 @@ class TestIrModel(SavepointCase): bananas = self.env['x_bananas'].search([]) self.assertEqual(bananas.mapped('x_name'), names, 'failed to order by %s' % order) + + def test_group_expansion(self): + """Check that the basic custom group expansion works.""" + groups = self.env['x_bananas'].read_group(domain=[], + fields=['x_ripeness_id'], + groupby=['x_ripeness_id']) + expected = [{ + 'x_ripeness_id': self.ripeness_green, + 'x_ripeness_id_count': 3, + '__domain': [('x_ripeness_id', '=', self.ripeness_green[0])], + }, { + 'x_ripeness_id': self.ripeness_okay, + 'x_ripeness_id_count': 0, + '__domain': [('x_ripeness_id', '=', self.ripeness_okay[0])], + }, { + 'x_ripeness_id': self.ripeness_gone, + 'x_ripeness_id_count': 0, + '__domain': [('x_ripeness_id', '=', self.ripeness_gone[0])], + }] + self.assertEqual(groups, expected, 'should include 2 empty ripeness stages') diff --git a/odoo/addons/base/views/ir_model_views.xml b/odoo/addons/base/views/ir_model_views.xml index 1505b517150..f534def632b 100644 --- a/odoo/addons/base/views/ir_model_views.xml +++ b/odoo/addons/base/views/ir_model_views.xml @@ -274,6 +274,9 @@ attrs="{'required': [('ttype','in',['many2one','one2many','many2many'])], 'readonly': [('ttype','not in',['many2one','one2many','many2many'])], 'invisible': [('ttype','not in',['many2one','one2many','many2many'])]}"/> + diff --git a/odoo/models.py b/odoo/models.py index 9eee8d3b354..4b04263bba0 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1754,6 +1754,11 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): """ cls.pool._clear_cache() + @api.model + def _read_group_expand_full(self, groups, domain, order): + """Extend the group to include all targer records by default.""" + return groups.search([], order=order) + @api.model def _read_group_fill_results(self, domain, groupby, remaining_groupbys, aggregated_fields, count_field,