From bb1ea079205c25007d9fa8b084a0c042e9d8b1b7 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Fri, 6 Oct 2023 17:16:24 +0200 Subject: [PATCH] [FIX] website_{forum_}slides: improve perf of logged_in rules (Introduced in 16.3, 86c6266) --- ISSUE From there, partner_ids and partner_all_ids m2m fields became computed fields in order to return only active records as we read them from the slide.channel.partner table, not checking the active field of that table (not supported by framework). Ir rules of records check if current user's partner is inside partner_ids, or partner_all_ids of a given course. (joined the course for partner_ids, invited or joined the course for partner_all_ids) The issue is that all ir rules searching on these fields lead to very slow loading of slides (channel, slide, homepage) and forum pages (as soon as we have a forum linked to a course). This comes from the fact that in some templates, we read records and evaluate rules on the spot, in python. (for instance tags of courses on homepage) As some courses have tens of thousands of attendees, this evaluation is very slow in python, and there is no quick solution at framework level. Therefore, computing thousands of partners for each access rule linked to a course is too slow. --- SOLUTION The existing fields is_member and is_member_invited are used inside ir rules instead. _search methods are added and we check whether course is inside courses of current user's partner ones. It improved all times a lot, see BENCHMARK below. Tests already exist to assert the search methods indirectly. --- LOCAL BENCHMARK (16.3) - IntelCore i5-8265U CPU 1.60GHz - Firefox ROUTE BEFORE (s) AFTER (s) /slides 5 <0.15 /slides/all 7.6 <0.15 /slides/course_c 4.2 <0.2 /forum 5.5 <0.15 /forum/forum_c 6 <0.25 /forum/forum_c/post_y 4.4 <0.25 /forum/forum_c/post_y/edit_answer 2.5 + 2.5 <0.3 total /forum/forum_c/post_y/edit 2.8 <0.2 /forum/forum_c/post_y/reply 3 (+6 post) <0.35 total /profile/user/id 2.3 <0.4 DETAILS - course_c: 43k attendees - forum_c: forum of course_c - post_y: 2-3 answers - logged user: 'portal', with 7 courses - deactivated GC cron as many populated invited attendees were to be removed, and it can take a lot of time POPULATIONS - 20k forum.forum (avg 2 answers) - 20k mail.message (about 2 answers per post) - 10 forum.forum (+20 on courses) - 20 slide.channel (half 'members' visibility) - 300k slide.channel.partner - 'large' for other models --- LINKS UPG PR: odoo/upgrade#5246 Task-3444633 closes odoo/odoo#139104 X-original-commit: 019d9d4808994cd819d13fab2d6ddfdffeb621e7 Signed-off-by: Thibault Delavallee (tde) --- addons/website_slides/models/slide_channel.py | 47 +++++++++++++------ .../security/website_slides_security.xml | 10 ++-- .../website_slides_forum_security.xml | 6 +-- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/addons/website_slides/models/slide_channel.py b/addons/website_slides/models/slide_channel.py index 74eed962226..34edaa95468 100644 --- a/addons/website_slides/models/slide_channel.py +++ b/addons/website_slides/models/slide_channel.py @@ -427,10 +427,10 @@ class Channel(models.Model): has_requested_access = fields.Boolean(string='Access Requested', compute='_compute_has_requested_access', compute_sudo=False) is_member = fields.Boolean( string='Is Enrolled Attendee', help='Is the attendee actively enrolled.', - compute='_compute_membership_values', compute_sudo=False) + compute='_compute_membership_values', search="_search_is_member") is_member_invited = fields.Boolean( string='Is Invited Attendee', help='Is the invitation for this attendee pending.', - compute='_compute_membership_values', compute_sudo=False) + compute='_compute_membership_values', search="_search_is_member_invited") partner_has_new_content = fields.Boolean(compute='_compute_partner_has_new_content', compute_sudo=False) # karma generation karma_gen_channel_rank = fields.Integer(string='Course ranked', default=5) @@ -533,26 +533,43 @@ class Channel(models.Model): for channel in self: channel.has_requested_access = channel.id in requested_cids - @api.depends('channel_partner_all_ids.partner_id', 'channel_partner_all_ids.member_status') + @api.depends('channel_partner_all_ids.partner_id', 'channel_partner_all_ids.member_status', 'channel_partner_all_ids.active') @api.depends_context('uid') - @api.model def _compute_membership_values(self): if self.env.user._is_public(): self.is_member = False self.is_member_invited = False return - - channel_partners = self.env['slide.channel.partner'].sudo().search([ - ('channel_id', 'in', self.ids), - ('partner_id', '=', self.env.user.partner_id.id), - ]) - active_channel_partners = channel_partners.filtered( - lambda channel_partner: channel_partner.member_status != 'invited') - invitation_pending_channels = (channel_partners - active_channel_partners).channel_id - active_channels = active_channel_partners.channel_id + data = { + member_status: channel_ids + for member_status, channel_ids in self.env['slide.channel.partner'].sudo()._read_group( + [('partner_id', '=', self.env.user.partner_id.id), ('channel_id', 'in', self.ids), ('active', '=', True)], + ['member_status'], ['channel_id:array_agg'] + ) + } + active_channels_ids = data.get('joined', []) + data.get('ongoing', []) + data.get('completed', []) + invitation_pending_channels_ids = data.get('invited', []) for channel in self: - channel.is_member = channel in active_channels - channel.is_member_invited = channel in invitation_pending_channels + channel.is_member = channel.id in active_channels_ids + channel.is_member_invited = channel.id in invitation_pending_channels_ids + + def _search_is_member(self, operator, value): + if operator not in ['=', '!='] or not isinstance(value, bool): + raise NotImplementedError(_('Operation not supported')) + check_has_access = operator == '=' and value or operator == '!=' and not value + return [('id', 'in' if check_has_access else 'not in', self._search_is_member_channel_ids())] + + def _search_is_member_invited(self, operator, value): + if operator not in ['=', '!='] or not isinstance(value, bool): + raise NotImplementedError(_('Operation not supported')) + check_has_access = operator == '=' and value or operator == '!=' and not value + return [('id', 'in' if check_has_access else 'not in', self._search_is_member_channel_ids(invited=True))] + + def _search_is_member_channel_ids(self, invited=False): + return self.env['slide.channel.partner'].sudo()._read_group( + [('partner_id', '=', self.env.user.partner_id.id), ('member_status', '=' if invited else '!=', 'invited'), ('active', '=', True)], + aggregates=['channel_id:array_agg'] + )[0][0] @api.depends('slide_ids.is_category') def _compute_category_and_slide_ids(self): diff --git a/addons/website_slides/security/website_slides_security.xml b/addons/website_slides/security/website_slides_security.xml index 9a7066b5980..bb787ab3235 100644 --- a/addons/website_slides/security/website_slides_security.xml +++ b/addons/website_slides/security/website_slides_security.xml @@ -49,7 +49,9 @@ ('website_published', '=', True), '|', ('visibility', 'in', ('public', 'connected')), - ('partner_all_ids', '=', user.partner_id.id), + '|', + ('is_member_invited', '=', True), + ('is_member', '=', True), ] @@ -142,11 +144,11 @@ '&', '|', ('channel_id.visibility', 'in', ('public','connected')), - ('channel_id.partner_all_ids', 'in', user.partner_id.ids), + ('channel_id.is_member_invited', '=', True), '|', ('is_category', '=', True), ('is_preview', '=', True), - ('channel_id.partner_ids', 'in', user.partner_id.ids), + ('channel_id.is_member', '=', True), ] @@ -238,7 +240,7 @@ Resource: read restricted to channel members and channel responsible - [('slide_id.channel_id.partner_ids', '=', user.partner_id.id)] + [('slide_id.channel_id.is_member', '=', True)] diff --git a/addons/website_slides_forum/security/website_slides_forum_security.xml b/addons/website_slides_forum/security/website_slides_forum_security.xml index 9b1dc50fc01..0f4c8595d23 100644 --- a/addons/website_slides_forum/security/website_slides_forum_security.xml +++ b/addons/website_slides_forum/security/website_slides_forum_security.xml @@ -14,7 +14,7 @@ ('slide_channel_ids.website_published', '=', True), '|', ('slide_channel_ids.visibility', 'in', ('public','connected')), - ('slide_channel_ids.partner_ids', 'in', user.partner_id.id) + ('slide_channel_ids.is_member', '=', True) ] @@ -40,7 +40,7 @@ ('forum_id.slide_channel_ids.website_published', '=', True), '|', ('forum_id.slide_channel_ids.visibility', 'in', ('public','connected')), - ('forum_id.slide_channel_ids.partner_ids', 'in', user.partner_id.id) + ('forum_id.slide_channel_ids.is_member', '=', True) ] @@ -66,7 +66,7 @@ ('forum_id.slide_channel_ids.website_published', '=', True), '|', ('forum_id.slide_channel_ids.visibility', 'in', ('public','connected')), - ('forum_id.slide_channel_ids.partner_ids', 'in', user.partner_id.id) + ('forum_id.slide_channel_ids.is_member', '=', True) ]