diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index eefbbfb66af..012bd3cd298 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -596,11 +596,9 @@ class IrModelFields(models.Model): for field in self: safe_eval(field.domain or '[]') - @api.constrains('name', 'state') + @api.constrains('name') def _check_name(self): for field in self: - if field.state == 'manual' and not field.name.startswith('x_'): - raise ValidationError(_("Custom fields must have a name that starts with 'x_'!")) try: models.check_pg_name(field.name) except ValidationError: @@ -610,6 +608,11 @@ class IrModelFields(models.Model): _sql_constraints = [ ('name_unique', 'UNIQUE(model, name)', "Field names must be unique per model."), ('size_gt_zero', 'CHECK (size>=0)', 'Size of the field cannot be negative.'), + ( + "name_manual_field", + "CHECK (state != 'manual' OR name LIKE 'x_%')", + "Custom fields must have a name that starts with 'x_'!" + ), ] def _related_field(self): diff --git a/odoo/addons/base/tests/test_ir_actions.py b/odoo/addons/base/tests/test_ir_actions.py index 81f96c266be..f9ee8c395ad 100644 --- a/odoo/addons/base/tests/test_ir_actions.py +++ b/odoo/addons/base/tests/test_ir_actions.py @@ -379,13 +379,13 @@ class TestCustomFields(common.TransactionCase): def test_create_custom(self): """ custom field names must be start with 'x_' """ - with self.assertRaises(ValidationError): + with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'): self.create_field('foo') def test_rename_custom(self): """ custom field names must be start with 'x_' """ field = self.create_field('x_foo') - with self.assertRaises(ValidationError): + with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'): field.name = 'foo' def test_create_valid(self): @@ -399,6 +399,26 @@ class TestCustomFields(common.TransactionCase): with self.assertRaises(ValidationError): field.name = 'x_foo bar' + def test_add_field_valid(self): + """ custom field names must start with 'x_', even when bypassing the constraints + + If a user bypasses all constraints to add a custom field not starting by `x_`, + it must not be loaded in the registry. + + This is to forbid users to override class attributes. + """ + field = self.create_field('x_foo') + # Drop the SQL constraint, to bypass it, + # as a user could do through a SQL shell or a `cr.execute` in a server action + self.env.cr.execute("ALTER TABLE ir_model_fields DROP CONSTRAINT ir_model_fields_name_manual_field") + self.env.cr.execute("UPDATE ir_model_fields SET name = 'foo' WHERE id = %s", [field.id]) + with self.assertLogs('odoo.addons.base.models.ir_model') as log_catcher: + # Trick to reload the registry. The above rename done through SQL didn't reload the registry. This will. + field.write({'field_description': 'foo'}) + self.assertIn( + f'The field `{field.name}` is not defined in the `{field.model}` Python class', log_catcher.output[0] + ) + def test_create_unique(self): """ one cannot create two fields with the same name on a given model """ self.create_field('x_foo') diff --git a/odoo/models.py b/odoo/models.py index 770c8dec42c..7ac14d9f25a 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -613,12 +613,28 @@ class BaseModel(metaclass=MetaModel): def _add_field(self, name, field): """ Add the given ``field`` under the given ``name`` in the class """ cls = type(self) - # add field as an attribute and in cls._fields (for reflection) + + # Assert the name is an existing field in the model, or any model in the _inherits + # or a custom field (starting by `x_`) + is_class_field = any( + isinstance(getattr(model, name, None), fields.Field) + for model in [cls] + [self.env.registry[inherit] for inherit in cls._inherits] + ) + if not (is_class_field or name.startswith('x_')): + raise ValidationError( + f"The field `{name}` is not defined in the `{cls._name}` Python class and does not start with 'x_'" + ) + + # Assert the attribute to assign is a Field + if not isinstance(field, fields.Field): + raise ValidationError("You can only add `fields.Field` objects to a model fields") + if not isinstance(getattr(cls, name, field), Field): _logger.warning("In model %r, field %r overriding existing value", cls._name, name) setattr(cls, name, field) field._toplevel = True field.__set_name__(cls, name) + # add field as an attribute and in cls._fields (for reflection) cls._fields[name] = field @api.model