[IMP] website,* : reduce number of sql queries on generic code

Reduce number of queries on base method like dispatch, or base controller like /web/content

Part of task-2211013

Following
    https://github.com/odoo/odoo/pull/46925
    https://github.com/odoo/odoo/pull/47257

Most of these small queries are done in part of code called a lot of times.
Next step is to be more 'controller' specific, and reduce query / imp perf
for specific page /my /blog /forum /shop ...

Courtesy of Rco and AL for help and validation

closes odoo/odoo#48165

Signed-off-by: Jérémy Kersten (jke) <jke@openerp.com>
Co-authored-by: Jérémy Kersten <jke@odoo.com>
Co-authored-by: Romain Derie <rde@odoo.com>

addons/http_routing/models/ir_http.py

@@ -411,7 +411,7 @@ class IrHttp(models.AbstractModel):
                 lang = preferred_lang or cls._get_default_lang()
 
             request.lang = lang
-            context['lang'] = lang.code
+            context['lang'] = lang._get_cached('code')
 
             # bind modified context
             request.context = context

addons/test_website/controllers/main.py

@@ -119,3 +119,9 @@ class WebsiteTest(Home):
     @http.route(['/get_post_nomultilang'], type='http', auth="public", methods=['GET', 'POST'], website=True, multilang=False, sitemap=False)
     def get_post_method_no_multilang(self, **kw):
         return request.make_response('get_post_nomultilang')
+
+    # Test Perfs
+
+    @http.route(['/empty_controller_test'], type='http', auth='public', website=True, multilang=False, sitemap=False)
+    def empty_controller_test(self, **kw):
+        return 'Basic Controller Content'

addons/test_website/tests/__init__.py

@@ -2,9 +2,10 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import test_controller_args
-from . import test_is_multilang
-from . import test_reset_views
 from . import test_error
-from . import test_views_during_module_operation
+from . import test_is_multilang
 from . import test_multi_company
+from . import test_performance
+from . import test_reset_views
 from . import test_session
+from . import test_views_during_module_operation

addons/test_website/tests/test_performance.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.addons.website.tests.test_performance import UtilPerf
+
+
+class TestPerformance(UtilPerf):
+    def test_10_perf_sql_website_controller_minimalist(self):
+        url = '/empty_controller_test'
+        self.assertEqual(self._get_url_hot_query(url), 3)

addons/website/controllers/main.py

@@ -68,6 +68,9 @@ class Website(Home):
 
     @http.route('/', type='http', auth="public", website=True, sitemap=True)
     def index(self, **kw):
+        # prefetch all menus (it will prefetch website.page too)
+        top_menu = request.website.menu_id
+
         homepage = request.website.homepage_id
         if homepage and (homepage.sudo().is_visible or request.env.user.has_group('base.group_user')) and homepage.url != '/':
             return request.env['ir.http'].reroute(homepage.url)
@@ -76,7 +79,6 @@ class Website(Home):
         if website_page:
             return website_page
         else:
-            top_menu = request.website.menu_id
             first_menu = top_menu and top_menu.child_id and top_menu.child_id.filtered(lambda menu: menu.is_visible)
             if first_menu and first_menu[0].url not in ('/', '', '#') and (not (first_menu[0].url.startswith(('/?', '/#', ' ')))):
                 return request.redirect(first_menu[0].url)
@@ -485,7 +487,10 @@ class WebsiteBinary(http.Controller):
                 kw['unique'] = unique
         return Binary().content_image(**kw)
 
+    # if not icon provided in DOM, browser tries to access /favicon.ico, eg when opening an order pdf
     @http.route(['/favicon.ico'], type='http', auth='public', website=True, multilang=False, sitemap=False)
     def favicon(self, **kw):
-        # when opening a pdf in chrome, chrome tries to open the default favicon url
-        return self.content_image(model='website', id=str(request.website.id), field='favicon', **kw)
+        website = request.website
+        response = request.redirect(website.image_url(website, 'favicon'), code=301)
+        response.headers['Cache-Control'] = 'public, max-age=%s' % (365 * 24 * 60)
+        return response

addons/website/models/ir_http.py

@@ -132,8 +132,8 @@ class Http(models.AbstractModel):
         if not request.session.uid:
             env = api.Environment(request.cr, SUPERUSER_ID, request.context)
             website = env['website'].get_current_website()
-            if website and website.user_id:
-                request.uid = website.user_id.id
+            request.uid = website and website._get_cached('user_id')
+
         if not request.uid:
             super(Http, cls)._auth_method_public()
 
@@ -199,8 +199,9 @@ class Http(models.AbstractModel):
         # context (eg: /shop), and it's not going to propagate to the global context of the tab
         # If the company of the website is not in the allowed companies of the user, set the main
         # company of the user.
-        if request.website.company_id in request.env.user.company_ids:
-            context['allowed_company_ids'] = request.website.company_id.ids
+        website_company_id = request.website._get_cached('company_id')
+        if website_company_id in request.env.user.company_ids.ids:
+            context['allowed_company_ids'] = [website_company_id]
         else:
             context['allowed_company_ids'] = request.env.user.company_id.ids
 
@@ -222,7 +223,7 @@ class Http(models.AbstractModel):
     @classmethod
     def _get_default_lang(cls):
         if getattr(request, 'website', False):
-            return request.website.default_lang_id
+            return request.env['res.lang'].browse(request.website._get_cached('default_lang_id'))
         return super(Http, cls)._get_default_lang()
 
     @classmethod
@@ -239,9 +240,12 @@ class Http(models.AbstractModel):
         published_domain = page_domain
         # specific page first
         page = request.env['website.page'].sudo().search(published_domain, order='website_id asc', limit=1)
+        if page:
+            # prefetch all menus (it will prefetch website.page too)
+            request.website.menu_id
         if page and (request.website.is_publisher() or page.is_visible):
             _, ext = os.path.splitext(req_page)
-            return request.render(page.get_view_identifier(), {
+            return request.render(page.view_id.id, {
                 'deletable': True,
                 'main_object': page,
             }, mimetype=_guess_mimetype(ext))
@@ -363,7 +367,7 @@ class Http(models.AbstractModel):
         if request.env.user.has_group('website.group_website_publisher'):
             session_info.update({
                 'website_id': request.website.id,
-                'website_company_id': request.website.company_id.id,
+                'website_company_id': request.website._get_cached('company_id'),
             })
         return session_info
 

addons/website/models/ir_ui_view.py

@@ -450,12 +450,9 @@ class View(models.Model):
                 website=request.website,
                 url_for=url_for,
                 res_company=request.website.company_id.sudo(),
-                default_lang_code=request.env['ir.http']._get_default_lang().code,
                 languages=request.env['res.lang'].get_available(),
                 translatable=translatable,
                 editable=editable,
-                # retrocompatibility, remove me in master
-                menu_data={'children': []} if request.website.is_user() else None,
             ))
 
         return qcontext

addons/website/models/website.py

@@ -143,7 +143,8 @@ class Website(models.Model):
                 # don't add child menu if parent is forbidden
                 if menu.parent_id and menu.parent_id in menus:
                     menu.parent_id._cache['child_id'] += (menu.id,)
-
+            # prefetch every website.page and ir.ui.view at once
+            menus.mapped('is_visible')
             website.menu_id = menus and menus.filtered(lambda m: not m.parent_id)[0].id or False
 
     # self.env.uid for ir.rule groups on menu
@@ -263,6 +264,8 @@ class Website(models.Model):
         # Bootstrap default menu hierarchy, create a new minimalist one if no default
         default_menu = self.env.ref('website.main_menu')
         self.copy_menu_hierarchy(default_menu)
+        home_menu = self.env['website.menu'].search([('website_id', '=', self.id), ('url', '=', '/')])
+        home_menu.page_id = self.homepage_id
 
     def copy_menu_hierarchy(self, top_menu):
         def copy_menu(menu, t_menu):
@@ -946,6 +949,18 @@ class Website(models.Model):
         # if the current URL is indeed canonical or not.
         return current_url == canonical_url
 
+    @tools.ormcache('self.id')
+    def _get_cached_values(self):
+        self.ensure_one()
+        return {
+            'user_id': self.user_id.id,
+            'company_id': self.company_id.id,
+            'default_lang_id': self.default_lang_id.id,
+        }
+
+    def _get_cached(self, field):
+        return self._get_cached_values()[field]
+
 
 class BaseModel(models.AbstractModel):
     _inherit = 'base'

addons/website/models/website_page.py

@@ -58,10 +58,6 @@ class Page(models.Model):
 
         return most_specific_page == page_to_test
 
-    def get_view_identifier(self):
-        """ Get identifier of this page view that may be used to render it """
-        return self.view_id.id
-
     def get_page_properties(self):
         self.ensure_one()
         res = self.read([

addons/website/tests/test_performance.py

@@ -17,24 +17,7 @@ EXTRA_REQUEST = 5
 """
 
 
-class TestWebsitePerformance(HttpCase):
-
-    def setUp(self):
-        super().setUp()
-
-        self.page = self.env['website.page'].create({
-            'url': '/sql_page',
-            'name': 'Page Test SQL Perf',
-            'type': 'qweb',
-            'arch': '<t name="Contact Us" t-name="website.contactus"> \
-                       <t t-call="website.layout"> \
-                         <div id="wrap"><div class="oe_structure"/></div> \
-                       </t> \
-                     </t>',
-            'key': 'website.page_test_perf_sql',
-            'is_published': True,
-        })
-
+class UtilPerf(HttpCase):
     def _get_url_hot_query(self, url):
         # ensure worker is in hot state
         self.url_open(url)
@@ -42,21 +25,98 @@ class TestWebsitePerformance(HttpCase):
 
         sql_count = self.registry.test_cr.sql_log_count
         self.url_open(url)
-        return self.registry.test_cr.sql_log_count - sql_count
+        return self.registry.test_cr.sql_log_count - sql_count - EXTRA_REQUEST
+
+
+class TestStandardPerformance(UtilPerf):
+    def test_10_perf_sql_img_controller(self):
+        self.authenticate('demo', 'demo')
+        url = '/web/image/res.country/4/image'
+        self.assertEqual(self._get_url_hot_query(url), 4)
+        url = '/web/image/res.users/2/image_256'
+        self.assertEqual(self._get_url_hot_query(url), 7)
+
+    def test_20_perf_sql_img_controller_bis(self):
+        url = '/web/image/website/1/favicon'
+        self.assertEqual(self._get_url_hot_query(url), 4)
+        self.authenticate('portal', 'portal')
+        self.assertEqual(self._get_url_hot_query(url), 4)
+
+
+class TestWebsitePerformance(UtilPerf):
+
+    def setUp(self):
+        super().setUp()
+        self.page, self.menu = self._create_page_with_menu('/sql_page')
+
+    def _create_page_with_menu(self, url):
+        name = url[1:]
+        website = self.env['website'].browse(1)
+        page = self.env['website.page'].create({
+            'url': url,
+            'name': name,
+            'type': 'qweb',
+            'arch': '<t name="%s" t-name="website.page_test_%s"> \
+                       <t t-call="website.layout"> \
+                         <div id="wrap"><div class="oe_structure"/></div> \
+                       </t> \
+                     </t>' % (name, name),
+            'key': 'website.page_test_%s' % name,
+            'is_published': True,
+            'website_id': website.id,
+            'track': False,
+        })
+        menu = self.env['website.menu'].create({
+            'name': name,
+            'url': url,
+            'page_id': page.id,
+            'website_id': website.id,
+            'parent_id': website.menu_id.id
+        })
+        return (page, menu)
 
     def test_10_perf_sql_queries_page(self):
-        # standard website.page
-        expected_sql = 14 + EXTRA_REQUEST
-        self.assertEqual(self._get_url_hot_query(self.page.url), expected_sql)
+        # standard untracked website.page
+        self.assertEqual(self._get_url_hot_query(self.page.url), 12)
+        self.menu.unlink()
+        self.assertEqual(self._get_url_hot_query(self.page.url), 14)
+
+    def test_15_perf_sql_queries_page(self):
+        # standard tracked website.page
+        self.page.track = True
+        self.assertEqual(self._get_url_hot_query(self.page.url), 20)
+        self.menu.unlink()
+        self.assertEqual(self._get_url_hot_query(self.page.url), 22)
 
     def test_20_perf_sql_queries_homepage(self):
         # homepage "/" has its own controller
-        # add 5 queries for test environment (RELEASE/SAVEPOINT/ROLLBACK test_cursor)
-        expected_sql = 21 + EXTRA_REQUEST
-        self.assertEqual(self._get_url_hot_query('/'), expected_sql)
+        self.assertEqual(self._get_url_hot_query('/'), 19)
 
     def test_30_perf_sql_queries_page_no_layout(self):
         # website.page with no call to layout templates
         self.page.arch = '<div>I am a blank page</div>'
-        expected_sql = 8 + EXTRA_REQUEST
-        self.assertEqual(self._get_url_hot_query(self.page.url), expected_sql)
+        self.assertEqual(self._get_url_hot_query(self.page.url), 9)
+
+    def test_40_perf_sql_queries_page_multi_level_menu(self):
+        # menu structure should not impact SQL requests
+        _, menu_a = self._create_page_with_menu('/a')
+        _, menu_aa = self._create_page_with_menu('/aa')
+        _, menu_b = self._create_page_with_menu('/b')
+        _, menu_bb = self._create_page_with_menu('/bb')
+        _, menu_bbb = self._create_page_with_menu('/bbb')
+        _, menu_bbbb = self._create_page_with_menu('/bbbb')
+        _, menu_bbbbb = self._create_page_with_menu('/bbbbb')
+        self._create_page_with_menu('c')
+        menu_bbbbb.parent_id = menu_bbbb
+        menu_bbbb.parent_id = menu_bbb
+        menu_bbb.parent_id = menu_bb
+        menu_bb.parent_id = menu_b
+        menu_aa.parent_id = menu_a
+
+        self.assertEqual(self._get_url_hot_query(self.page.url), 12)
+
+    def test_50_perf_sql_web_content(self):
+        # assets route /web/content/..
+        self.url_open('/')  # create assets attachments
+        assets_url = self.env['ir.attachment'].search([('url', '=like', '/web/content/%/web.assets_common%.js')], limit=1).url
+        self.assertEqual(self._get_url_hot_query(assets_url), 2)

odoo/addons/base/models/ir_http.py

@@ -19,7 +19,7 @@ import werkzeug.utils
 
 import odoo
 from odoo import api, http, models, tools, SUPERUSER_ID
-from odoo.exceptions import AccessDenied, AccessError
+from odoo.exceptions import AccessDenied, AccessError, MissingError
 from odoo.http import request, content_disposition
 from odoo.tools import consteq, pycompat
 from odoo.tools.mimetypes import guess_mimetype
@@ -296,34 +296,38 @@ class IrHttp(models.AbstractModel):
             record = self.env[model].browse(int(id))
 
         # obj exists
-        if not record or not record.exists() or field not in record:
+        if not record or field not in record:
             return None, 404
 
-        if model == 'ir.attachment':
-            record_sudo = record.sudo()
-            if access_token and not consteq(record_sudo.access_token or '', access_token):
-                return None, 403
-            elif (access_token and consteq(record_sudo.access_token or '', access_token)):
-                record = record_sudo
-            elif record_sudo.public:
-                record = record_sudo
-            elif self.env.user.has_group('base.group_portal'):
-                # Check the read access on the record linked to the attachment
-                # eg: Allow to download an attachment on a task from /my/task/task_id
-                record.check('read')
-                record = record_sudo
-
-        # check read access
         try:
-            # We have prefetched some fields of record, among which the field
-            # 'write_date' used by '__last_update' below. In order to check
-            # access on record, we have to invalidate its cache first.
-            record._cache.clear()
-            record['__last_update']
-        except AccessError:
-            return None, 403
+            if model == 'ir.attachment':
+                record_sudo = record.sudo()
+                if access_token and not consteq(record_sudo.access_token or '', access_token):
+                    return None, 403
+                elif (access_token and consteq(record_sudo.access_token or '', access_token)):
+                    record = record_sudo
+                elif record_sudo.public:
+                    record = record_sudo
+                elif self.env.user.has_group('base.group_portal'):
+                    # Check the read access on the record linked to the attachment
+                    # eg: Allow to download an attachment on a task from /my/task/task_id
+                    record.check('read')
+                    record = record_sudo
 
-        return record, 200
+            # check read access
+            try:
+                # We have prefetched some fields of record, among which the field
+                # 'write_date' used by '__last_update' below. In order to check
+                # access on record, we have to invalidate its cache first.
+                if not record.env.su:
+                    record._cache.clear()
+                record['__last_update']
+            except AccessError:
+                return None, 403
+
+            return record, 200
+        except MissingError:
+            return None, 404
 
     @classmethod
     def _binary_ir_attachment_redirect_content(cls, record, default_mimetype='application/octet-stream'):
@@ -366,11 +370,16 @@ class IrHttp(models.AbstractModel):
 
         field_def = record._fields[field]
         if field_def.type == 'binary' and field_def.attachment:
-            field_attachment = self.env['ir.attachment'].sudo().search_read(domain=[('res_model', '=', model), ('res_id', '=', record.id), ('res_field', '=', field)], fields=['datas', 'mimetype', 'checksum'], limit=1)
-            if field_attachment:
-                mimetype = field_attachment[0]['mimetype']
-                content = field_attachment[0]['datas']
-                filehash = field_attachment[0]['checksum']
+            if model != 'ir.attachment':
+                field_attachment = self.env['ir.attachment'].sudo().search_read(domain=[('res_model', '=', model), ('res_id', '=', record.id), ('res_field', '=', field)], fields=['datas', 'mimetype', 'checksum'], limit=1)
+                if field_attachment:
+                    mimetype = field_attachment[0]['mimetype']
+                    content = field_attachment[0]['datas']
+                    filehash = field_attachment[0]['checksum']
+            else:
+                mimetype = record['mimetype']
+                content = record['datas']
+                filehash = record['checksum']
 
         if not content:
             content = record[field] or ''

odoo/addons/base/models/res_lang.py

@@ -213,6 +213,19 @@ class Lang(models.Model):
         langs = self.with_context(active_test=False).search([])
         return sorted([(lang.code, lang.url_code, lang.name) for lang in langs], key=itemgetter(2))
 
+    @tools.ormcache('self.id')
+    def _get_cached_values(self):
+        self.ensure_one()
+        return {
+            'id': self.id,
+            'code': self.code,
+            'url_code': self.url_code,
+            'name': self.name,
+        }
+
+    def _get_cached(self, field):
+        return self._get_cached_values()[field]
+
     @api.model
     @tools.ormcache('code')
     def _lang_code_to_urlcode(self, code):