From 8e081ec61fa305d224853ef588043373284271b7 Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Thu, 23 Apr 2020 12:49:22 +0000 Subject: [PATCH] [IMP] website_blog: avoid exists() in loop `tags_list()` is called in loops to get nicely formatted URL in blog tag button but doing an exists here will do everytime an SQL query regardless of the cache. We can safely remove the call to `exists()` as the `tag_ids` parameter is coming from the controller `active_tag_ids` key which is already ensuring the exists condition. It make also the number of requests determinists for test with a random number of tags. From between 200 ~ 300 queries (depending number of tags)-> 116 task-2211013 Co-authored-by: Romain Derie Co-authored-by: Jeremy Kersten --- addons/website_blog/controllers/main.py | 14 +++++++------ addons/website_blog/tests/test_performance.py | 20 +++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/addons/website_blog/controllers/main.py b/addons/website_blog/controllers/main.py index e4990b7413f..c013644c677 100644 --- a/addons/website_blog/controllers/main.py +++ b/addons/website_blog/controllers/main.py @@ -22,12 +22,12 @@ class WebsiteBlog(http.Controller): _post_comment_per_page = 10 def tags_list(self, tag_ids, current_tag): - tag_ids = list(tag_ids) # required to avoid using the same list + tag_ids = list(tag_ids) # required to avoid using the same list if current_tag in tag_ids: tag_ids.remove(current_tag) else: tag_ids.append(current_tag) - tag_ids = request.env['blog.tag'].browse(tag_ids).exists() + tag_ids = request.env['blog.tag'].browse(tag_ids) return ','.join(slug(tag) for tag in tag_ids) def nav_list(self, blog=None): @@ -57,6 +57,7 @@ class WebsiteBlog(http.Controller): def _prepare_blog_values(self, blogs, blog=False, date_begin=False, date_end=False, tags=False, state=False, page=False): """ Prepare all values to display the blogs index page or one specific blog""" BlogPost = request.env['blog.post'] + BlogTag = request.env['blog.tag'] # prepare domain domain = request.website.website_domain() @@ -66,15 +67,16 @@ class WebsiteBlog(http.Controller): if date_begin and date_end: domain += [("post_date", ">=", date_begin), ("post_date", "<=", date_end)] - active_tag_ids = tags and [unslug(tag)[1] for tag in tags.split(',')] or [] + active_tags = BlogTag if active_tag_ids: - fixed_tag_slug = ",".join(slug(t) for t in request.env['blog.tag'].browse(active_tag_ids).exists()) + active_tags = BlogTag.browse(active_tag_ids).exists() + fixed_tag_slug = ",".join(slug(t) for t in active_tags) if fixed_tag_slug != tags: new_url = request.httprequest.full_path.replace("/tag/%s" % tags, "/tag/%s" % fixed_tag_slug, 1) if new_url != request.httprequest.full_path: # check that really replaced and avoid loop return request.redirect(new_url, 301) - domain += [('tag_ids', 'in', active_tag_ids)] + domain += [('tag_ids', 'in', active_tags.ids)] if request.env.user.has_group('website.group_website_designer'): count_domain = domain + [("website_published", "=", True), ("post_date", "<=", fields.Datetime.now())] @@ -127,7 +129,7 @@ class WebsiteBlog(http.Controller): 'pager': pager, 'posts': posts.with_prefetch(post_ids), 'tag': tags, - 'active_tag_ids': active_tag_ids, + 'active_tag_ids': active_tags.ids, 'domain': domain, 'state_info': state and {"state": state, "published": published_count, "unpublished": unpublished_count}, 'blogs': blogs, diff --git a/addons/website_blog/tests/test_performance.py b/addons/website_blog/tests/test_performance.py index 25a29b5e479..456404d1ad0 100644 --- a/addons/website_blog/tests/test_performance.py +++ b/addons/website_blog/tests/test_performance.py @@ -2,11 +2,12 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.addons.website.tests.test_performance import UtilPerf +import random class TestBlogPerformance(UtilPerf): def test_10_perf_sql_blog_standard_data(self): - self.assertEqual(self._get_url_hot_query('/blog'), 50) + self.assertEqual(self._get_url_hot_query('/blog'), 40) def test_20_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] @@ -18,4 +19,19 @@ class TestBlogPerformance(UtilPerf): for blog_post in blog_posts: blog_post.tag_ids += blog_tags blog_tags = blog_tags[:-1] - self.assertEqual(self._get_url_hot_query('/blog'), 116) + self.assertEqual(self._get_url_hot_query('/blog'), 38) + + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 61) + + def test_30_perf_sql_blog_bigger_data_random(self): + BlogPost = self.env['blog.post'] + BlogTag = self.env['blog.tag'] + blogs = self.env['blog.blog'].search([]) + blog_tags = BlogTag.create([{'name': 'New Blog Tag Test %s' % i} for i in range(1, 50)]) + BlogPost.create([{'name': 'New Blog Post Test %s' % i, 'is_published': True, 'blog_id': blogs[random.randint(0, 1)].id} for i in range(1, 100)]) + blog_posts = BlogPost.search([]) + for blog_post in blog_posts: + blog_post.write({'tag_ids': [[6, 0, random.choices(blog_tags.ids, k=random.randint(0, len(blog_tags)))]]}) + self.assertEqual(self._get_url_hot_query('/blog'), 38) + + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 61)