diff --git a/addons/account/models/account_move_line.py b/addons/account/models/account_move_line.py index 86a527b5150..9760d5f8787 100644 --- a/addons/account/models/account_move_line.py +++ b/addons/account/models/account_move_line.py @@ -1251,7 +1251,7 @@ class AccountMoveLine(models.Model): # ------------------------------------------------------------------------- # CRUD/ORM # ------------------------------------------------------------------------- - def invalidate_model(self, fnames=None): + def invalidate_model(self, fnames=None, flush=True): # Invalidate cache of related moves if fnames is None or 'move_id' in fnames: field = self._fields['move_id'] @@ -1259,16 +1259,16 @@ class AccountMoveLine(models.Model): move_ids = {id_ for id_ in self.env.cache.get_values(lines, field) if id_} if move_ids: self.env['account.move'].browse(move_ids).invalidate_recordset() - return super().invalidate_model(fnames) + return super().invalidate_model(fnames, flush) - def invalidate_recordset(self, fnames=None): + def invalidate_recordset(self, fnames=None, flush=True): # Invalidate cache of related moves if fnames is None or 'move_id' in fnames: field = self._fields['move_id'] move_ids = {id_ for id_ in self.env.cache.get_values(self, field) if id_} if move_ids: self.env['account.move'].browse(move_ids).invalidate_recordset() - return super().invalidate_recordset(fnames) + return super().invalidate_recordset(fnames, flush) @api.model def search_read(self, domain=None, fields=None, offset=0, limit=None, order=None): diff --git a/addons/bus/models/bus_presence.py b/addons/bus/models/bus_presence.py index 72e1983fcb7..4da96e7b9b4 100644 --- a/addons/bus/models/bus_presence.py +++ b/addons/bus/models/bus_presence.py @@ -17,7 +17,7 @@ AWAY_TIMER = 1800 # 30 minutes class BusPresence(models.Model): """ User Presence Its status is 'online', 'away' or 'offline'. This model should be a one2one, but is not - attached to res_users to avoid database concurrence errors. Since the 'update' method is executed + attached to res_users to avoid database concurrence errors. Since the 'update_presence' method is executed at each poll, if the user have multiple opened tabs, concurrence errors can happend, but are 'muted-logged'. """ @@ -34,7 +34,7 @@ class BusPresence(models.Model): self.env.cr.execute("CREATE UNIQUE INDEX IF NOT EXISTS bus_presence_user_unique ON %s (user_id) WHERE user_id IS NOT NULL" % self._table) @api.model - def update(self, inactivity_period, identity_field, identity_value): + def update_presence(self, inactivity_period, identity_field, identity_value): """ Updates the last_poll and last_presence of the current user :param inactivity_period: duration in milliseconds """ @@ -44,7 +44,7 @@ class BusPresence(models.Model): # Hide transaction serialization errors, which can be ignored, the presence update is not essential # The errors are supposed from presence.write(...) call only with tools.mute_logger('odoo.sql_db'): - self._update(inactivity_period=inactivity_period, identity_field=identity_field, identity_value=identity_value) + self._update_presence(inactivity_period=inactivity_period, identity_field=identity_field, identity_value=identity_value) # commit on success self.env.cr.commit() except OperationalError as e: @@ -54,7 +54,7 @@ class BusPresence(models.Model): raise @api.model - def _update(self, inactivity_period, identity_field, identity_value): + def _update_presence(self, inactivity_period, identity_field, identity_value): presence = self.search([(identity_field, '=', identity_value)], limit=1) # compute last_presence timestamp last_presence = datetime.datetime.now() - datetime.timedelta(milliseconds=inactivity_period) diff --git a/addons/bus/models/ir_websocket.py b/addons/bus/models/ir_websocket.py index 22a56297a3e..847d7a76b46 100644 --- a/addons/bus/models/ir_websocket.py +++ b/addons/bus/models/ir_websocket.py @@ -40,7 +40,7 @@ class IrWebsocket(models.AbstractModel): def _update_bus_presence(self, inactivity_period, im_status_ids_by_model): if self.env.user and not self.env.user._is_public(): - self.env['bus.presence'].update( + self.env['bus.presence'].update_presence( inactivity_period, identity_field='user_id', identity_value=self.env.uid diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index dad5d39eaec..89189d5c4cc 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -774,7 +774,7 @@ class Lead(models.Model): return result @api.model - def search(self, args, offset=0, limit=None, order=None, count=False): + def search(self, domain, offset=0, limit=None, order=None, count=False): """ Override to support ordering on my_activity_date_deadline. Ordering through web client calls search_read with an order parameter set. @@ -805,7 +805,7 @@ class Lead(models.Model): side effects. Search_count is not affected by this override. """ if count or not order or 'my_activity_date_deadline' not in order: - return super(Lead, self).search(args, offset=offset, limit=limit, order=order, count=count) + return super(Lead, self).search(domain, offset=offset, limit=limit, order=order, count=count) order_items = [order_item.strip().lower() for order_item in (order or self._order).split(',')] # Perform a read_group on my activities to get a mapping lead_id / deadline @@ -820,7 +820,7 @@ class Lead(models.Model): ) my_lead_mapping = dict((item['res_id'], item['date_deadline']) for item in my_lead_activities) my_lead_ids = list(my_lead_mapping.keys()) - my_lead_domain = expression.AND([[('id', 'in', my_lead_ids)], args]) + my_lead_domain = expression.AND([[('id', 'in', my_lead_ids)], domain]) my_lead_order = ', '.join(item for item in order_items if 'my_activity_date_deadline' not in item) # Search leads linked to those activities and order them. See docstring @@ -848,7 +848,7 @@ class Lead(models.Model): lead_order = ', '.join(item for item in order_items if 'my_activity_date_deadline' not in item) other_lead_res = super(Lead, self).search( - expression.AND([[('id', 'not in', my_lead_ids_skip)], args]), + expression.AND([[('id', 'not in', my_lead_ids_skip)], domain]), offset=lead_offset, limit=lead_limit, order=lead_order, count=count ) return self.browse(my_lead_ids_keep) + other_lead_res diff --git a/addons/hr/models/hr_employee.py b/addons/hr/models/hr_employee.py index 31656094c05..310248674f8 100644 --- a/addons/hr/models/hr_employee.py +++ b/addons/hr/models/hr_employee.py @@ -219,10 +219,10 @@ class HrEmployeePrivate(models.Model): user_id=responsible_user_id) employees_scheduled.write({'work_permit_scheduled_activity': True}) - def read(self, fields, load='_classic_read'): + def read(self, fields=None, load='_classic_read'): if self.check_access_rights('read', raise_exception=False): return super(HrEmployeePrivate, self).read(fields, load=load) - private_fields = set(fields).difference(self.env['hr.employee.public']._fields.keys()) + private_fields = set(fields or self._fields).difference(self.env['hr.employee.public']._fields) if private_fields: raise AccessError(_('The fields "%s" you try to read is not available on the public employee profile.') % (','.join(private_fields))) return self.env['hr.employee.public'].browse(self.ids).read(fields, load=load) diff --git a/addons/hr_attendance/models/hr_attendance.py b/addons/hr_attendance/models/hr_attendance.py index 08d6c714d17..526a803fa2c 100644 --- a/addons/hr_attendance/models/hr_attendance.py +++ b/addons/hr_attendance/models/hr_attendance.py @@ -293,5 +293,5 @@ class HrAttendance(models.Model): self._update_overtime(attendances_dates) @api.returns('self', lambda value: value.id) - def copy(self): + def copy(self, default=None): raise exceptions.UserError(_('You cannot duplicate an attendance.')) diff --git a/addons/mail/models/ir_websocket.py b/addons/mail/models/ir_websocket.py index 9b6b65673cf..9af56e5ed8b 100644 --- a/addons/mail/models/ir_websocket.py +++ b/addons/mail/models/ir_websocket.py @@ -46,4 +46,4 @@ class IrWebsocket(models.AbstractModel): guest_sudo = self.env['mail.guest']._get_guest_from_request(req).sudo() if not guest_sudo: return - guest_sudo.env['bus.presence'].update(inactivity_period, identity_field='guest_id', identity_value=guest_sudo.id) + guest_sudo.env['bus.presence'].update_presence(inactivity_period, identity_field='guest_id', identity_value=guest_sudo.id) diff --git a/odoo/addons/test_access_rights/models.py b/odoo/addons/test_access_rights/models.py index 01be7059dc5..770f1142e6f 100644 --- a/odoo/addons/test_access_rights/models.py +++ b/odoo/addons/test_access_rights/models.py @@ -43,10 +43,10 @@ class ObjCateg(models.Model): name = fields.Char(required=True) - def search(self, args, **kwargs): + def search(self, domain, *args, **kwargs): if self.env.context.get('only_media'): - args += [('name', '=', 'Media')] - return super(ObjCateg, self).search(args, **kwargs) + domain += [('name', '=', 'Media')] + return super(ObjCateg, self).search(domain, **kwargs) class FakeTicket(models.Model): diff --git a/odoo/addons/test_lint/tests/__init__.py b/odoo/addons/test_lint/tests/__init__.py index 8a1444a60d3..f7908d8a6e3 100644 --- a/odoo/addons/test_lint/tests/__init__.py +++ b/odoo/addons/test_lint/tests/__init__.py @@ -8,5 +8,6 @@ from . import test_l10n from . import test_manifests from . import test_markers from . import test_onchange_domains +from . import test_override_signatures from . import test_pofile from . import test_pylint diff --git a/odoo/addons/test_lint/tests/test_override_signatures.py b/odoo/addons/test_lint/tests/test_override_signatures.py new file mode 100644 index 00000000000..2211f450e95 --- /dev/null +++ b/odoo/addons/test_lint/tests/test_override_signatures.py @@ -0,0 +1,155 @@ +import collections +import inspect +import itertools + +import odoo +from odoo.tests.common import get_db_name, tagged +from .lint_case import LintCase + + +POSITIONAL_ONLY = inspect.Parameter.POSITIONAL_ONLY +POSITIONAL_OR_KEYWORD = inspect.Parameter.POSITIONAL_OR_KEYWORD +VAR_POSITIONAL = inspect.Parameter.VAR_POSITIONAL +KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY +VAR_KEYWORD = inspect.Parameter.VAR_KEYWORD + + +failure_message = """\ +Invalid override of {model}.{method} in {child_module}, {message}. + +Original definition in {parent_module}: + def {method}{original_signature} + +Incompatible definition in {child_module}: + def {method}{override_signature}""" + + +methods_to_sanitize = { + method_name + for method_name in dir(odoo.models.BaseModel) + if not method_name.startswith('_') +} - { + # Not yet sanitized... + 'write', 'create', 'default_get', 'init' +} + + +class HitMiss: + def __init__(self): + self.hit = 0 + self.miss = 0 + @property + def ratio(self): + return self.hit / (self.hit + self.miss) +counter = collections.defaultdict(HitMiss) + + +def get_odoo_module_name(python_module_name): + if python_module_name.startswith('odoo.addons.'): + return python_module_name.split('.')[2] + if python_module_name == 'odoo.models': + return 'odoo' + + return python_module_name + + +def assert_valid_override(parent_signature, child_signature): + pparams = parent_signature.parameters + cparams = child_signature.parameters + + # parent and child have exact same signature + if pparams == cparams: + return + + # child overrides is exactly (self, *args, **kwargs) + if [cp.kind for cp in cparams.values()] == [VAR_POSITIONAL, VAR_KEYWORD]: + return + + # parent has *args/**kwargs: child can define new custom args/kwargs + parent_has_varargs = any(pp.kind == VAR_POSITIONAL for pp in pparams.values()) + parent_has_varkwargs = any(pp.kind == VAR_KEYWORD for pp in pparams.values()) + + # child has *args/**kwargs: all unknown args/kargs are delegated + child_has_varargs = any(cp.kind == VAR_POSITIONAL for cp in cparams.values()) + child_has_varkwargs = any(cp.kind == VAR_KEYWORD for cp in cparams.values()) + + # check positionals + pposparams = [(pp_name, pp) for pp_name, pp in pparams.items() + if pp.kind in (POSITIONAL_ONLY, POSITIONAL_OR_KEYWORD)] + cposparams = [(cp_name, cp) for cp_name, cp in cparams.items() + if cp.kind in (POSITIONAL_ONLY, POSITIONAL_OR_KEYWORD)] + if len(cposparams) < len(pposparams): + assert child_has_varargs, "missing positional parameters" + assert cposparams == pposparams[:len(cposparams)], "wrong positional parameters" + elif len(cposparams) > len(pposparams): + assert parent_has_varargs, "too many positional parameters" + assert cposparams[:len(pposparams)] == pposparams, "wrong positional parameters" + else: + assert cposparams == pposparams, "wrong positional parameters" + + # check keywords + pkwparams = {(pp_name, pp) for pp_name, pp in pparams.items() + if pp.kind in (POSITIONAL_OR_KEYWORD, KEYWORD_ONLY)} + ckwparams = {(cp_name, cp) for cp_name, cp in cparams.items() + if cp.kind in (POSITIONAL_OR_KEYWORD, KEYWORD_ONLY)} + if ckwparams < pkwparams: + assert child_has_varkwargs, "missing keyword parameters" + elif ckwparams > pkwparams: + assert parent_has_varkwargs, "too many keyword parameters" + elif ckwparams != pkwparams: + assert child_has_varkwargs and parent_has_varkwargs, "wrong keyword parameters" + + +@tagged('-at_install', 'post_install') +class TestLintOverrideSignatures(LintCase): + def test_lint_override_signature(self): + self.failureException = TypeError + registry = odoo.registry(get_db_name()) + for model_name, model_cls in registry.items(): + for method_name, _ in inspect.getmembers(model_cls, inspect.isroutine): + if method_name not in methods_to_sanitize: + continue + + # Find the original function definition + reverse_mro = reversed(model_cls.mro()[1:-1]) + for parent_class in reverse_mro: + method = getattr(parent_class, method_name, None) + if callable(method): + break + + parent_module = get_odoo_module_name(parent_class.__module__) + original_signature = inspect.signature(method) + + iter_parent, iter_children = itertools.tee(reverse_mro) + iter_parent = itertools.chain([parent_class], iter_parent) + + # Assert that all child classes correctly override the method + for parent_class, child_class in zip(iter_parent, iter_children): + if method_name not in child_class.__dict__: + continue + override = getattr(child_class, method_name) + + child_module = get_odoo_module_name(child_class.__module__) + override_signature = inspect.signature(override) + + with self.subTest(module=child_module, model=model_name, method=method_name): + try: + assert_valid_override(original_signature, override_signature) + counter[method_name].hit += 1 + except AssertionError as exc: + counter[method_name].miss += 1 + raise TypeError(failure_message.format( + message=exc.args[0], + model=model_name, + method=method_name, + child_module=child_module, + parent_module=parent_module, + original_signature=original_signature, + override_signature=override_signature, + )) from None + + #with open('/tmp/odoo-override-signatures.md', 'w') as f: + # f.write('method|hit|miss|ratio\n') + # f.write('------|---|----|-----\n') + # for method_name, hm in sorted(counter.items(), key=lambda x: x[1].miss): + # f.write(f'{method_name}|{hm.hit}|{hm.miss}|{hm.ratio:.3f}\n')