diff --git a/addons/http_routing/models/ir_http.py b/addons/http_routing/models/ir_http.py index 5469310228f..c0fd4a46a19 100644 --- a/addons/http_routing/models/ir_http.py +++ b/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 diff --git a/addons/test_website/controllers/main.py b/addons/test_website/controllers/main.py index 1973e363653..1e8ed2df24f 100644 --- a/addons/test_website/controllers/main.py +++ b/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' diff --git a/addons/test_website/tests/__init__.py b/addons/test_website/tests/__init__.py index b7044f07875..08a794bfaab 100644 --- a/addons/test_website/tests/__init__.py +++ b/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 diff --git a/addons/test_website/tests/test_performance.py b/addons/test_website/tests/test_performance.py new file mode 100644 index 00000000000..cd06dfda860 --- /dev/null +++ b/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) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index 8019abba5c4..49cc8160a06 100644 --- a/addons/website/controllers/main.py +++ b/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 diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 80ec1d9f0c7..ae68a21e174 100644 --- a/addons/website/models/ir_http.py +++ b/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 diff --git a/addons/website/models/ir_ui_view.py b/addons/website/models/ir_ui_view.py index 08152d59d6f..bc4a24205af 100644 --- a/addons/website/models/ir_ui_view.py +++ b/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 diff --git a/addons/website/models/website.py b/addons/website/models/website.py index 610f198d4d6..d1abd719fa9 100644 --- a/addons/website/models/website.py +++ b/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' diff --git a/addons/website/models/website_page.py b/addons/website/models/website_page.py index 392f4899257..e287bc22b0c 100644 --- a/addons/website/models/website_page.py +++ b/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([ diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index f3c4834a0a7..2e02135116a 100644 --- a/addons/website/tests/test_performance.py +++ b/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': ' \ - \ -
\ - \ - ', - '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': ' \ + \ +
\ + \ + ' % (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 = '
I am a blank page
' - 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) diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 1cc56cb0383..ba771a95753 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/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 '' diff --git a/odoo/addons/base/models/res_lang.py b/odoo/addons/base/models/res_lang.py index 0b58daf5411..dcf901e1a22 100644 --- a/odoo/addons/base/models/res_lang.py +++ b/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):