From 86b449ad48d72889105b7947c83c3af343a55837 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:02 +0000 Subject: [PATCH 01/12] [FIX] website: set correct m2o relation for home menu When creating a website, we: 1. Create the website 2. Bootstrap a homepage 3. Copy the generic menu hierarchy tree for that website But since we just bootstraped a homepage for the website, the home menu was not using that page but the generic one from the xml data instead. That's an issue for tests recently introduced in master that needs the relation to be correctly set for the query count to work as expected. task-2211013 --- addons/website/models/website.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/addons/website/models/website.py b/addons/website/models/website.py index 610f198d4d6..e13fd743146 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -263,6 +263,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): From af68fcde4858b6a1e6c672b55f1465bf29e371cc Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:08 +0000 Subject: [PATCH 02/12] [IMP] website: clean and add perf tests - Refactor for easier readability - Add a test to ensure menu hierarchy scale correctly. - Add tests for image controllers - Add test for assets controllers task-2211013 --- addons/test_website/controllers/main.py | 6 + addons/test_website/tests/__init__.py | 7 +- addons/test_website/tests/test_performance.py | 10 ++ addons/website/tests/test_performance.py | 114 +++++++++++++----- 4 files changed, 107 insertions(+), 30 deletions(-) create mode 100644 addons/test_website/tests/test_performance.py 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..8495c1af274 --- /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), 4) diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index f3c4834a0a7..b42fa130148 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), 5) + url = '/web/image/res.users/2/image_256' + self.assertEqual(self._get_url_hot_query(url), 8) + + def test_20_perf_sql_img_controller_bis(self): + url = '/web/image/website/1/favicon' + self.assertEqual(self._get_url_hot_query(url), 6) + self.authenticate('portal', 'portal') + self.assertEqual(self._get_url_hot_query(url), 5) + + +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), 14) + 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), 22) + 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('/'), 21) 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), 8) + + 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), 22) + + 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), 6) From a5a57321c785d0c0718f413aeb8a149fbb164f95 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:14 +0000 Subject: [PATCH 03/12] [IMP] website: perf - reduce SQL queries for the website menu Render page flow: - Search website.page in _serve_page for requested URL - Check if the page is visible with `is_visible` - `is_visible` will get the `ir.ui.view` of the page Note that this is not 'cachable' as being visible depends of now() for the publishing date - Render layout which is getting the website menus Note that this can't be cached either as a menu visibility might also depends of it's website.page visibility which depends of now() - Checking menus `is_visible` fetch the menus pages and those pages views. Before this commit, every level of menu hierarchy would add 2 queries. This can be avoided by prefetching everything in once during the dispatch if we know we will render the website menu. In such a case, read `is_visible` on every website menus will fetch every pages, views and menus in the lowest SQL queries possible, eg one query for every table. For a 3 level menus, requests goes from 20 to 12 to load a untracked page. task-2211013 --- addons/website/models/website.py | 3 ++- addons/website/tests/test_performance.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/addons/website/models/website.py b/addons/website/models/website.py index e13fd743146..99b2ec78b3e 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 diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index b42fa130148..9dcca13c472 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -113,7 +113,7 @@ class TestWebsitePerformance(UtilPerf): menu_bb.parent_id = menu_b menu_aa.parent_id = menu_a - self.assertEqual(self._get_url_hot_query(self.page.url), 22) + self.assertEqual(self._get_url_hot_query(self.page.url), 14) def test_50_perf_sql_web_content(self): # assets route /web/content/.. From d52c2bbe191da1e660837cdaaff47313347d1284 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:19 +0000 Subject: [PATCH 04/12] [IMP] website: perf - clear cache only when needed Before this commit, the cache would always been cleared even if not needed. Now, we only clear cache when needed. Util: Introduced with https://github.com/odoo/odoo/commit/9920f20e4c7753bc17bea71dea3a90f7de687196#diff-1407a8ce197a04eefaefa26c127a4418R327 Changed with 0d5fcdc1c7d1 task-2211013 --- addons/website/tests/test_performance.py | 2 +- odoo/addons/base/models/ir_http.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index 9dcca13c472..c257de62b88 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -119,4 +119,4 @@ class TestWebsitePerformance(UtilPerf): # 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), 6) + self.assertEqual(self._get_url_hot_query(assets_url), 5) diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 1cc56cb0383..52ba58d5651 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/odoo/addons/base/models/ir_http.py @@ -318,7 +318,8 @@ class IrHttp(models.AbstractModel): # 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() + if not record.env.su: + record._cache.clear() record['__last_update'] except AccessError: return None, 403 From 7ec403a1e3899a8dafae1dfc8ad623fcf8cad79f Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:25 +0000 Subject: [PATCH 05/12] [REM] website: remove unused qcontext keys task-2211013 --- addons/website/models/ir_ui_view.py | 3 --- 1 file changed, 3 deletions(-) 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 From 2c09e00a64a6f565d56c16c0b72549bc6c680b6a Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:32 +0000 Subject: [PATCH 06/12] [IMP] website: perf - prefetch menus when serving a page If we are rendering a page, the menus will most likely be rendered as well. Note that even in a 403 case when the page is found but is not visible, the menus will also be shown on the 403 page. Fetching the menus before accessing the requested page will prefetch that page as well in one go if that page is in the menus, without any costs for the pages not in the menus. There is a tradeoff for 'non-layout' pages, which only occurs in advanced technical cases: 1. Create a page with specific extension as name suffix (page.css) in which case the page will be bootstraped accordingly, without call to layout. 2. Remove the call to layout in HTML editor or backend 3. Remove the call to submenus template in HTML editor or backend For those cases, the menus will be prefetched for no reason. Note that homepage '/' is rendered through a controller, same logic is applied there. task-2211013 --- addons/website/controllers/main.py | 4 +++- addons/website/models/ir_http.py | 3 +++ addons/website/tests/test_performance.py | 10 +++++----- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index 8019abba5c4..df2bf628660 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) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 80ec1d9f0c7..c850a3a4e0b 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -239,6 +239,9 @@ 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(), { diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index c257de62b88..92e1616f29a 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -77,25 +77,25 @@ class TestWebsitePerformance(UtilPerf): def test_10_perf_sql_queries_page(self): # standard untracked website.page - self.assertEqual(self._get_url_hot_query(self.page.url), 14) + 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), 22) + 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 - self.assertEqual(self._get_url_hot_query('/'), 21) + 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
' - self.assertEqual(self._get_url_hot_query(self.page.url), 8) + self.assertEqual(self._get_url_hot_query(self.page.url), 10) def test_40_perf_sql_queries_page_multi_level_menu(self): # menu structure should not impact SQL requests @@ -113,7 +113,7 @@ class TestWebsitePerformance(UtilPerf): menu_bb.parent_id = menu_b menu_aa.parent_id = menu_a - self.assertEqual(self._get_url_hot_query(self.page.url), 14) + self.assertEqual(self._get_url_hot_query(self.page.url), 12) def test_50_perf_sql_web_content(self): # assets route /web/content/.. From bd4a44cece6cc51d7358ec3861633bb53db53005 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:39 +0000 Subject: [PATCH 07/12] [IMP] base: perf - try/except instead of exists Most of the time, the record exists, but we still ensure it does, every single time, making an SQL query. Doing a try catch will result in the same behavior, but won't make that query most of the time. task-2211013 --- addons/website/tests/test_performance.py | 10 ++--- odoo/addons/base/models/ir_http.py | 55 +++++++++++++----------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index 92e1616f29a..5cca6ad64b1 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -32,15 +32,15 @@ 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), 5) + 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), 8) + 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), 6) - self.authenticate('portal', 'portal') self.assertEqual(self._get_url_hot_query(url), 5) + self.authenticate('portal', 'portal') + self.assertEqual(self._get_url_hot_query(url), 4) class TestWebsitePerformance(UtilPerf): @@ -119,4 +119,4 @@ class TestWebsitePerformance(UtilPerf): # 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), 5) + self.assertEqual(self._get_url_hot_query(assets_url), 4) diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 52ba58d5651..ece858f5a0f 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,35 +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. - if not record.env.su: - 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'): From b03a4180be6138ffd4fc5ccfcbe6a44516bd83d0 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 26 Mar 2020 18:06:45 +0000 Subject: [PATCH 08/12] [REM] website: remove useless method Was introduced with bae7e32e4b for `website_version` which doesn't exist anymore. task-2211013 --- addons/website/models/ir_http.py | 2 +- addons/website/models/website_page.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index c850a3a4e0b..a704dbb2dbe 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -244,7 +244,7 @@ class Http(models.AbstractModel): 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)) 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([ From 6d6b505598413b0d766a8394c7131351ade64266 Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Thu, 26 Mar 2020 18:06:51 +0000 Subject: [PATCH 09/12] [IMP] website: perf - add cache for website frequent fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dispatching layer of Odoo (common to any call) can avoid a query on website if those 3 values are cached. Note that for complex business flows, there won't be any gain since website infos will have to be fetched at some point. About user_id: In most cases, for a website page you will need to browse the website btw so we don't remove this request previously. But in case of a simple image, the controller /web/content need the public user just to set the request.uid in auth_public but no other info from website. With this commit, we don't do the 'select * from website where id in()' request on each controller declared in auth='public' but use the user_id from the cache. (Invalidated by the write on website_id) task-2211013 Co-authored-by: Jérémy Kersten Co-authored-by: Romain Derie --- addons/test_website/tests/test_performance.py | 2 +- addons/website/models/ir_http.py | 13 +++++++------ addons/website/models/website.py | 12 ++++++++++++ addons/website/tests/test_performance.py | 4 ++-- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/addons/test_website/tests/test_performance.py b/addons/test_website/tests/test_performance.py index 8495c1af274..cd06dfda860 100644 --- a/addons/test_website/tests/test_performance.py +++ b/addons/test_website/tests/test_performance.py @@ -7,4 +7,4 @@ 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), 4) + self.assertEqual(self._get_url_hot_query(url), 3) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index a704dbb2dbe..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 @@ -366,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/website.py b/addons/website/models/website.py index 99b2ec78b3e..d1abd719fa9 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -949,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/tests/test_performance.py b/addons/website/tests/test_performance.py index 5cca6ad64b1..b7683e96068 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -38,7 +38,7 @@ class TestStandardPerformance(UtilPerf): def test_20_perf_sql_img_controller_bis(self): url = '/web/image/website/1/favicon' - self.assertEqual(self._get_url_hot_query(url), 5) + self.assertEqual(self._get_url_hot_query(url), 4) self.authenticate('portal', 'portal') self.assertEqual(self._get_url_hot_query(url), 4) @@ -119,4 +119,4 @@ class TestWebsitePerformance(UtilPerf): # 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), 4) + self.assertEqual(self._get_url_hot_query(assets_url), 3) From 1606d22d40cd431a02e73bb0661a192bb12edf85 Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Thu, 26 Mar 2020 18:06:57 +0000 Subject: [PATCH 10/12] [FIX] base: perf - binary_record, don't rebrowse ir.attachment If model is already an ir.attachment we don't need to do a new search_read. We can use it record directly. After this commit, we don't do extra request if we already have the info, else we don't change the behavior. task-2211013 --- addons/website/tests/test_performance.py | 2 +- odoo/addons/base/models/ir_http.py | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index b7683e96068..0cbc7ce214d 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -119,4 +119,4 @@ class TestWebsitePerformance(UtilPerf): # 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), 3) + 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 ece858f5a0f..ba771a95753 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/odoo/addons/base/models/ir_http.py @@ -370,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 '' From a65d27cce98c1b0d303eeaf8f066fe4983afc7a8 Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Thu, 26 Mar 2020 18:07:03 +0000 Subject: [PATCH 11/12] [IMP] website: perf - favicon.ico cached for 1 year Note that this route is only used when no icon is set in DOM, such as accessing a PDF after an order. task-2211013 --- addons/website/controllers/main.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index df2bf628660..49cc8160a06 100644 --- a/addons/website/controllers/main.py +++ b/addons/website/controllers/main.py @@ -487,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 From 26c4de2f18be2b4e944ee255236d9a5a1710d94c Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Thu, 26 Mar 2020 18:07:20 +0000 Subject: [PATCH 12/12] [IMP] http_routing: use cached code from lang In case of short controller that don't use qweb template, we don't need anyhting else that this url code that don't change frequently. It make only sense for model like lang, website, ... that will not change frequently. And are called on each call by the dispatcher. In case of a website page, we will btw browse lang later, but in case of small controller like /favicon.ico, or page without qweb, ... we can just use the same from last query. --- addons/http_routing/models/ir_http.py | 2 +- addons/website/tests/test_performance.py | 2 +- odoo/addons/base/models/res_lang.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) 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/website/tests/test_performance.py b/addons/website/tests/test_performance.py index 0cbc7ce214d..2e02135116a 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -95,7 +95,7 @@ class TestWebsitePerformance(UtilPerf): 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
' - self.assertEqual(self._get_url_hot_query(self.page.url), 10) + 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 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):