[IMP] base: faster get_bindings

The cache key of _get_bindings was not super efficient.
The result of _get_bindings is cached,
but its performance was altered by the cache
key which requires to fetch the user groups for each call to
_get_bindings.
Besides, as there is a lot of possible group
combination, this resulted in a lot of possible cache keys,
and therefore a lot of cached values.

This revision aims to make _get_bindings more efficient
by:

- do not use the groups in the cache keys (less cached values)
- filter out actions not available to the user groups after
  retrieving them from the cache
- use has_group to do the above, which is itself cached as well,
  and therefore do not need to fetch the user groups
  at each call to get_bindings.

In addition, move get_bindings from `get_view`
to `get_views`. If there was 3 views asked by `get_views`
(let's say kanban, list, form)
`get_bindings` was being called 3 times, through `get_view`
with each time the same arguments and therefore the same result :-).
Moving it to `get_views` allows to call it only once for all view types
requested, and for the web client it doesn't change much,
as it always request the toolbar/get_bindings through `get_views` only.

In addition, add the lang to the cache of _get_bindings.
it was actually a bug not to put it: if you had 2 users
with the same group set, using 2 different languages,
the user accessing first the get_bindings would cache
the action names within his language, and then the second
user would see the action name within the language of the first user
:-).

Before
```py
In [1]: %time for i in range(1000): self.env['ir.actions.actions'].get_bindings('res.partner'); self.env.invalidate_all();
CPU times: user 790 ms, sys: 104 ms, total: 893 ms
Wall time: 1.7 s
```

After
```py
In [1]: %time for i in range(1000): self.env['ir.actions.actions'].get_bindings('res.partner'); self.env.invalidate_all();
CPU times: user 23.5 ms, sys: 9.12 ms, total: 32.7 ms
Wall time: 36.9 ms
```

Part-of: odoo/odoo#99417

addons/board/models/board.py

@@ -32,10 +32,14 @@ class Board(models.AbstractModel):
         if custom_view:
             res.update({'custom_view_id': custom_view.id,
                         'arch': custom_view.arch})
-        res.update({
-            'arch': self._arch_preprocessing(res['arch']),
-            'toolbar': {'print': [], 'action': [], 'relate': []}
-        })
+        res['arch'] = self._arch_preprocessing(res['arch'])
+        return res
+
+    @api.model
+    def get_views(self, views, options=None):
+        res = super().get_views(views, options)
+        for view in res['views'].values():
+            view['toolbar'] = {'print': [], 'action': [], 'relate': []}
         return res
 
     @api.model

addons/hr/models/res_users.py

@@ -179,18 +179,9 @@ class User(models.Model):
         # Note: limit the `sudo` to the only action of "editing own profile" action in order to
         # avoid breaking `groups` mecanism on res.users form view.
         profile_view = self.env.ref("hr.res_users_view_form_profile")
-        original_user = self.env.user
         if profile_view and view_id == profile_view.id:
             self = self.with_user(SUPERUSER_ID)
         result = super(User, self).get_view(view_id, view_type, **options)
-        # Due to using the SUPERUSER the result will contain action that the user may not have access too
-        # here we filter out actions that requires special implicit rights to avoid having unusable actions
-        # in the dropdown menu.
-        if options.get('toolbar') and self.env.user != original_user:
-            self = self.with_user(original_user.id)
-            if not self.user_has_groups("base.group_erp_manager"):
-                change_password_action = self.env.ref("base.change_password_wizard_action")
-                result['toolbar']['action'] = [act for act in result['toolbar']['action'] if act['id'] != change_password_action.id]
         return result
 
     @api.model_create_multi

addons/hr/tests/test_self_user_access.py

@@ -86,7 +86,7 @@ class TestSelfAccessProfile(TestHrCommon):
             'user_id': james.id,
         })
         view = self.env.ref('hr.res_users_view_form_profile')
-        available_actions = james.get_view(view.id, toolbar=True)['toolbar']['action']
+        available_actions = james.get_views([(view.id, 'form')], {'toolbar': True})['views']['form']['toolbar']['action']
         change_password_action = self.env.ref("base.change_password_wizard_action")
 
         self.assertFalse(any(x['id'] == change_password_action.id for x in available_actions))
@@ -99,7 +99,7 @@ class TestSelfAccessProfile(TestHrCommon):
             'user_id': john.id,
         })
         view = self.env.ref('hr.res_users_view_form_profile')
-        available_actions = john.get_view(view.id, toolbar=True)['toolbar']['action']
+        available_actions = john.get_views([(view.id, 'form')], {'toolbar': True})['views']['form']['toolbar']['action']
         self.assertTrue(any(x['id'] == change_password_action.id for x in available_actions))
 
 

odoo/addons/base/models/ir_actions.py

@@ -4,6 +4,7 @@
 import odoo
 from odoo import api, fields, models, tools, _, Command
 from odoo.exceptions import MissingError, ValidationError, AccessError
+from odoo.tools import frozendict
 from odoo.tools.safe_eval import safe_eval, test_python_expr
 from odoo.tools.float_utils import float_compare
 from odoo.http import request
@@ -87,24 +88,40 @@ class IrActions(models.Model):
 
     @api.model
     def get_bindings(self, model_name):
-        return self._get_bindings(model_name, bool(request) and request.session.debug)
-
-    @tools.ormcache('frozenset(self.env.user.groups_id.ids)', 'model_name', 'debug')
-    def _get_bindings(self, model_name, debug=False):
         """ Retrieve the list of actions bound to the given model.
 
            :return: a dict mapping binding types to a list of dict describing
                     actions, where the latter is given by calling the method
                     ``read`` on the action record.
         """
+        result = {}
+        for action_type, all_actions in self._get_bindings(model_name).items():
+            actions = []
+            for action in all_actions:
+                action = dict(action)
+                groups = action.pop('groups_id', None)
+                if groups and not self.user_has_groups(groups):
+                    # the user may not perform this action
+                    continue
+                res_model = action.pop('res_model', None)
+                if res_model and not self.env['ir.model.access'].check(
+                    res_model,
+                    mode='read',
+                    raise_exception=False
+                ):
+                    # the user won't be able to read records
+                    continue
+                actions.append(action)
+            if actions:
+                result[action_type] = actions
+        return result
+
+    @tools.ormcache('model_name', 'self.env.lang')
+    def _get_bindings(self, model_name):
         cr = self.env.cr
-        IrModelAccess = self.env['ir.model.access']
 
         # discard unauthorized actions, and read action definitions
         result = defaultdict(list)
-        user_groups = self.env.user.groups_id
-        if not debug:
-            user_groups -= self.env.ref('base.group_no_one')
 
         self.env.flush_all()
         cr.execute("""
@@ -117,25 +134,22 @@ class IrActions(models.Model):
         for action_id, action_model, binding_type in cr.fetchall():
             try:
                 action = self.env[action_model].sudo().browse(action_id)
-                action_groups = getattr(action, 'groups_id', ())
-                action_model = getattr(action, 'res_model', False)
-                if action_groups and not action_groups & user_groups:
-                    # the user may not perform this action
-                    continue
-                if action_model and not IrModelAccess.check(action_model, mode='read', raise_exception=False):
-                    # the user won't be able to read records
-                    continue
                 fields = ['name', 'binding_view_types']
-                if 'sequence' in action._fields:
-                    fields.append('sequence')
-                result[binding_type].append(action.read(fields)[0])
-            except (AccessError, MissingError):
+                for field in ('groups_id', 'res_model', 'sequence'):
+                    if field in action._fields:
+                        fields.append(field)
+                action = action.read(fields)[0]
+                if action.get('groups_id'):
+                    groups = self.env['res.groups'].browse(action['groups_id'])
+                    action['groups_id'] = ','.join(ext_id for ext_id in groups._ensure_xml_id().values())
+                result[binding_type].append(frozendict(action))
+            except (MissingError):
                 continue
 
         # sort actions by their sequence if sequence available
         if result.get('action'):
-            result['action'] = sorted(result['action'], key=lambda vals: vals.get('sequence', 0))
-        return result
+            result['action'] = tuple(sorted(result['action'], key=lambda vals: vals.get('sequence', 0)))
+        return frozendict(result)
 
     @api.model
     def _for_xml_id(self, full_xml_id):

odoo/addons/base/models/ir_module.py

@@ -159,12 +159,12 @@ class Module(models.Model):
     _order = 'application desc,sequence,name'
 
     @api.model
-    def get_view(self, view_id=None, view_type='form', **options):
-        res = super().get_view(view_id, view_type, **options)
-        if view_type == 'form' and res.get('toolbar',False):
+    def get_views(self, views, options=None):
+        res = super().get_views(views, options)
+        if res['views'].get('form', {}).get('toolbar'):
             install_id = self.env.ref('base.action_server_module_immediate_install').id
-            action = [rec for rec in res['toolbar']['action'] if rec.get('id', False) != install_id]
-            res['toolbar'] = {'action': action}
+            action = [rec for rec in res['views']['form']['toolbar']['action'] if rec.get('id', False) != install_id]
+            res['views']['form']['toolbar'] = {'action': action}
         return res
 
     @classmethod

odoo/addons/base/models/res_users.py

@@ -215,6 +215,26 @@ class Groups(models.Model):
             self.env['ir.model.access'].call_cache_clearing_methods()
         return super(Groups, self).write(vals)
 
+    def _ensure_xml_id(self):
+        """Return the groups external identifiers, creating the external identifier for groups missing one"""
+        result = self.get_external_id()
+        missings = {group_id: f'__custom__.group_{group_id}' for group_id, ext_id in result.items() if not ext_id}
+        if missings:
+            self.env['ir.model.data'].create(
+                [
+                    {
+                        'name': name.split('.')[1],
+                        'model': 'res.groups',
+                        'res_id': group_id,
+                        'module': name.split('.')[0],
+                    }
+                    for group_id, name in missings.items()
+                ]
+            )
+            result.update(missings)
+
+        return result
+
 
 class ResUsersLog(models.Model):
     _name = 'res.users.log'

odoo/addons/test_action_bindings/tests/test_bindings.py

@@ -9,8 +9,8 @@ class TestActionBindings(common.TransactionCase):
         # first make sure there is no bound action
         self.env.ref('base.action_partner_merge').unlink()
         bindings = Actions.get_bindings('res.partner')
-        self.assertFalse(bindings['action'])
-        self.assertFalse(bindings['report'])
+        self.assertFalse(bindings.get('action'))
+        self.assertFalse(bindings.get('report'))
 
         # create action bindings, and check the returned bindings
         action1 = self.env.ref('base.action_attachment')
@@ -53,19 +53,19 @@ class TestBindingViewFilters(common.TransactionCase):
     def test_act_window(self):
         A = self.env['tab.a']
 
-        form_act = A.get_view(toolbar=True)['toolbar']['action']
+        form_act = A.get_views([(False, 'form')], {'toolbar': True})['views']['form']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in form_act],
             ['Action 1', 'Action 2', 'Action 3'],
             "forms should have all actions")
 
-        list_act = A.get_view(view_type='tree', toolbar=True)['toolbar']['action']
+        list_act = A.get_views([(False, 'list')], {'toolbar': True})['views']['list']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in list_act],
             ['Action 1', 'Action 3'],
             "lists should not have the form-only action")
 
-        kanban_act = A.get_view(view_type='kanban', toolbar=True)['toolbar']['action']
+        kanban_act = A.get_views([(False, 'kanban')], {'toolbar': True})['views']['kanban']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in kanban_act],
             ['Action 1'],
@@ -74,19 +74,19 @@ class TestBindingViewFilters(common.TransactionCase):
     def test_act_record(self):
         B = self.env['tab.b']
 
-        form_act = B.get_view(toolbar=True)['toolbar']['action']
+        form_act = B.get_views([(False, 'form')], {'toolbar': True})['views']['form']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in form_act],
             ['Record 1', 'Record 2', 'Record 3'],
             "forms should have all actions")
 
-        list_act = B.get_view(view_type='tree', toolbar=True)['toolbar']['action']
+        list_act = B.get_views([(False, 'list')], {'toolbar': True})['views']['list']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in list_act],
             ['Record 1', 'Record 3'],
             "lists should not have the form-only action")
 
-        kanban_act = B.get_view(view_type='kanban', toolbar=True)['toolbar']['action']
+        kanban_act = B.get_views([(False, 'kanban')], {'toolbar': True})['views']['kanban']['toolbar']['action']
         self.assertEqual(
             [a['name'] for a in kanban_act],
             ['Record 1'],

odoo/models.py

@@ -1657,6 +1657,24 @@ class BaseModel(metaclass=MetaModel):
 
         result['models'] = {model: self.env[model].fields_get() for model in models}
 
+        # Add related action information if asked
+        if options.get('toolbar'):
+            for view in result['views'].values():
+                view['toolbar'] = {}
+
+            bindings = self.env['ir.actions.actions'].get_bindings(self._name)
+            for action_type, key in (('report', 'print'), ('action', 'action')):
+                for action in bindings.get(action_type, []):
+                    view_types = (
+                        action['binding_view_types'].split(',')
+                        if action.get('binding_view_types')
+                        else result['views'].keys()
+                    )
+                    for view_type in view_types:
+                        view_type = view_type if view_type != 'tree' else 'list'
+                        if view_type in result['views']:
+                            result['views'][view_type]['toolbar'].setdefault(key, []).append(action)
+
         return result
 
     @api.model
@@ -1671,7 +1689,6 @@ class BaseModel(metaclass=MetaModel):
             - bool load_filters: returns the model's filters (for search views)
             - bool mobile: true if the web client is currently using the responsive mobile view
               (to use kanban views instead of list views for x2many fields)
-            - bool toolbar: true to include contextual actions
         :return: architecture of the view as an etree node, and the browse record of the view used
         :rtype: tuple
         :raise AttributeError:
@@ -1729,7 +1746,6 @@ class BaseModel(metaclass=MetaModel):
             - load_filters: returns the model's filters (for search views)
             - mobile: true if the web client is currently using the responsive mobile view
               (to use kanban views instead of list views for x2many fields)
-            - toolbar: true to include contextual actions
         :return: composition of the requested view (including inherited views and extensions)
         :rtype: dict
         :raise AttributeError:
@@ -1760,22 +1776,6 @@ class BaseModel(metaclass=MetaModel):
             'models': models,
         }
 
-        # Add related action information if asked
-        if options.get('toolbar') and view_type != 'search':
-            vt = 'list' if view_type == 'tree' else view_type
-            bindings = self.env['ir.actions.actions'].get_bindings(self._name)
-            resreport = [action
-                         for action in bindings['report']
-                         if vt in (action.get('binding_view_types') or vt).split(',')]
-            resaction = [action
-                         for action in bindings['action']
-                         if vt in (action.get('binding_view_types') or vt).split(',')]
-
-            result['toolbar'] = {
-                'print': resreport,
-                'action': resaction,
-            }
-
         if options.get('load_filters') and view_type == 'search':
             result['filters'] = self.env['ir.filters'].get_filters(self._name, options.get('action_id'))
 
@@ -1823,7 +1823,7 @@ class BaseModel(metaclass=MetaModel):
             'Method `fields_view_get` is deprecated, use `get_view` instead',
             DeprecationWarning, stacklevel=2,
         )
-        result = self.get_view(view_id, view_type, toolbar=toolbar, submenu=submenu)
+        result = self.get_views([(view_id, view_type)], {'toolbar': toolbar, 'submenu': submenu})['views'][view_type]
         node = etree.fromstring(result['arch'])
         view_fields = set(el.get('name') for el in node.xpath('.//field[not(ancestor::field)]'))
         result['fields'] = self.fields_get(view_fields)