From f6ca054238ea5f0e555fa1c937f120aab2218235 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 12 Oct 2023 10:00:19 +0200 Subject: [PATCH 1/3] [FIX] mail: lessen recompute on res.company model 'email_formatted' on 'res.company' model depends on its formatted catchall and its underlying partner formatted email. However we do not need to give 'partner.id.email_formatted' trigger in the compute methods as it is not stored and thus do not need to recompute it for every change of linked partners in DB; it is valid for the current transaction. It allows to lessen invalidation of computed fields. Task-3557985 (Mail: Message cleanup (shortcode, link)) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#138938 --- addons/mail/models/res_company.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/mail/models/res_company.py b/addons/mail/models/res_company.py index 9714aa68f10..1b450e75f16 100644 --- a/addons/mail/models/res_company.py +++ b/addons/mail/models/res_company.py @@ -36,7 +36,7 @@ class Company(models.Model): company.catchall_email = '' company.catchall_formatted = '' - @api.depends('partner_id.email_formatted', 'catchall_formatted') + @api.depends('partner_id', 'catchall_formatted') def _compute_email_formatted(self): for company in self: if company.partner_id.email_formatted: From 2bf4ad912f62aa189ca01164d549b5cfd77e486f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 16 Oct 2023 13:54:35 +0200 Subject: [PATCH 2/3] [REM] mail: remove useless link between message and shortcodes There is a o2m / m2o link between mail.message and mail.shortcode. However this link is not used: 'messages' (m2o from shortcode to message) is never set. Anyway there is no usage of this link: shortcodes are replaced in message body and that's it. Task-3557985 (Mail: Message cleanup (shortcode, link)) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#138938 --- addons/mail/models/mail_message.py | 1 - addons/mail/models/mail_shortcode.py | 1 - addons/mail/models/mail_thread.py | 2 +- addons/test_mail/tests/test_message_post.py | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 186b3fbca0e..94ff8661ce5 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -178,7 +178,6 @@ class Message(models.Model): # as the cache value for this inverse one2many is up-to-date. # Besides for new messages, and messages never sending emails, there was no mail, and it was searching for nothing. mail_ids = fields.One2many('mail.mail', 'mail_message_id', string='Mails', groups="base.group_system") - canned_response_ids = fields.One2many('mail.shortcode', 'message_ids', string="Canned Responses", store=False) reaction_ids = fields.One2many('mail.message.reaction', 'message_id', string="Reactions", groups="base.group_system") @api.depends('body', 'subject') diff --git a/addons/mail/models/mail_shortcode.py b/addons/mail/models/mail_shortcode.py index f250b11be2e..337238b7681 100644 --- a/addons/mail/models/mail_shortcode.py +++ b/addons/mail/models/mail_shortcode.py @@ -19,5 +19,4 @@ class MailShortcode(models.Model): substitution = fields.Text('Substitution', required=True, help="Content that will automatically replace the shortcut of your choosing. This content can still be adapted before sending your message.") description = fields.Char('Description') - message_ids = fields.Many2one('mail.message', string="Messages", store=False) last_used = fields.Datetime('Last Used', help="Last time this shortcode was used") diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 48a23bbd4dc..5ace9f997da 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2746,7 +2746,7 @@ class MailThread(models.AbstractModel): for values in values_list: create_values = dict(values) # Avoid warnings about non-existing fields - for x in ('from', 'to', 'cc', 'canned_response_ids'): + for x in ('from', 'to', 'cc'): create_values.pop(x, None) create_values['partner_ids'] = [Command.link(pid) for pid in (create_values.get('partner_ids') or [])] create_values_list.append(create_values) diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 8147c4bf832..c5042bf8a10 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -499,7 +499,6 @@ class TestMessageNotify(TestMessagePostCommon): for parameters in [ {'message_type': 'comment'}, - {'canned_response_ids': []}, {'child_ids': []}, {'mail_ids': []}, {'notification_ids': []}, From 84e6596fa1901b90874fb18020e8b687209e4334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 16 Oct 2023 16:01:12 +0200 Subject: [PATCH 3/3] [REF] mail: improve link preview code (and perf) Improve link preview code * remove useless code indirections; * improve performance of link creation, cleanly support batch-creation (could lead to 98+5 URLs to the same domain creation within default 10 seconds instead of 98+1 but we feel it's ok); * name methods according to ORM; * use tools for html empty; Also cleanup link preview tests: concatenate tests when possible, improve setup, use real-life scenarios (using 'message_post' notably), remove useless data creation. Task-3557985 (Mail: Message cleanup (shortcode, link)) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#138938 --- addons/mail/controllers/link_preview.py | 6 +- addons/mail/models/mail_link_preview.py | 60 ++++++-------- addons/mail/tests/test_link_preview.py | 104 ++++++++++-------------- 3 files changed, 73 insertions(+), 97 deletions(-) diff --git a/addons/mail/controllers/link_preview.py b/addons/mail/controllers/link_preview.py index b290990433e..5fe6fce5352 100644 --- a/addons/mail/controllers/link_preview.py +++ b/addons/mail/controllers/link_preview.py @@ -18,8 +18,8 @@ class LinkPreviewController(http.Controller): if not message.is_current_user_or_guest_author and not guest.env.user._is_admin(): return if clear: - guest.env["mail.link.preview"]._clear_link_previews(message.sudo()) - guest.env["mail.link.preview"].sudo()._create_link_previews(message) + message.sudo().link_preview_ids._unlink_and_notify() + guest.env["mail.link.preview"].sudo()._create_from_message_and_notify(message) @http.route("/mail/link_preview/delete", methods=["POST"], type="json", auth="public") @add_guest_to_context @@ -30,4 +30,4 @@ class LinkPreviewController(http.Controller): return if not link_preview_sudo.message_id.is_current_user_or_guest_author and not guest.env.user._is_admin(): return - link_preview_sudo._delete_and_notify() + link_preview_sudo._unlink_and_notify() diff --git a/addons/mail/models/mail_link_preview.py b/addons/mail/models/mail_link_preview.py index 5aff9473f63..736397fcc5d 100644 --- a/addons/mail/models/mail_link_preview.py +++ b/addons/mail/models/mail_link_preview.py @@ -7,7 +7,7 @@ from lxml import html from urllib.parse import urlparse import requests -from odoo import api, models, fields +from odoo import api, models, fields, tools from odoo.addons.mail.tools import link_preview @@ -27,48 +27,38 @@ class LinkPreview(models.Model): create_date = fields.Datetime(index=True) @api.model - def _clear_link_previews(self, message): - message.link_preview_ids._delete_and_notify() - - @api.model - def _create_link_previews(self, message): - if not message.body: - return - tree = html.fromstring(message.body) - urls = tree.xpath('//a[not(@data-oe-model)]/@href') + def _create_from_message_and_notify(self, message): + if tools.is_html_empty(message.body): + return self + urls = set(html.fromstring(message.body).xpath('//a[not(@data-oe-model)]/@href')) link_previews = self.env['mail.link.preview'] requests_session = requests.Session() # Some websites are blocking non browser user agent. requests_session.headers.update({ 'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Firefox/91.0' }) - for url in set(urls): - if len(link_previews) >= 5: + link_preview_values = [] + for url in list(urls): + if preview := link_preview.get_link_preview_from_url(url, requests_session): + preview['message_id'] = message.id + link_preview_values.append(preview) + if len(link_preview_values) > 5: break - link_previews |= self.env['mail.link.preview']._create_link_preview(url, message.id, requests_session) - if not link_previews: - return - self.env['bus.bus']._sendone(message._bus_notification_target(), 'mail.record/insert', { - 'LinkPreview': link_previews._link_preview_format() - }) + if link_preview_values: + link_previews = self.env['mail.link.preview'].create(link_preview_values) + self.env['bus.bus']._sendone(message._bus_notification_target(), 'mail.record/insert', { + 'LinkPreview': link_previews._link_preview_format() + }) - @api.model - def _create_link_preview(self, url, message_id, request_session): - if self._is_domain_throttled(url): - return self.env['mail.link.preview'] - link_preview_data = link_preview.get_link_preview_from_url(url, request_session) - if link_preview_data: - link_preview_data['message_id'] = message_id - return self.create(link_preview_data) - return self.env['mail.link.preview'] - - def _delete_and_notify(self): - notifications = [] - for link_preview in self: - notifications.append((link_preview.message_id._bus_notification_target(), 'mail.link.preview/delete', { - 'id': link_preview.id, - 'message_id': link_preview.message_id.id, - })) + def _unlink_and_notify(self): + if not self: + return True + notifications = [ + ( + link_preview.message_id._bus_notification_target(), + 'mail.link.preview/delete', {'id': link_preview.id, 'message_id': link_preview.message_id.id} + ) for link_preview in self + ] self.env['bus.bus']._sendmany(notifications) self.unlink() diff --git a/addons/mail/tests/test_link_preview.py b/addons/mail/tests/test_link_preview.py index 6ffcb0c5056..8d4822ab04c 100644 --- a/addons/mail/tests/test_link_preview.py +++ b/addons/mail/tests/test_link_preview.py @@ -1,28 +1,25 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from functools import partial -import io -from odoo.addons.mail.tests.common import mail_new_test_user -from odoo.addons.mail.tests.common import MailCommon -from odoo.addons.mail.tools import link_preview +from markupsafe import Markup from unittest.mock import patch + +import io import requests -discuss_channel_new_test_user = partial(mail_new_test_user, context={'discuss_channel_nosubscribe': False}) +from odoo.addons.mail.tests.common import MailCommon +from odoo.addons.mail.tools import link_preview +from odoo.tests import tagged +@tagged('mail_link_preview', 'mail_message') class TestLinkPreview(MailCommon): @classmethod def setUpClass(cls): super().setUpClass() - cls.user_1 = discuss_channel_new_test_user( - cls.env, login='user_1', - name='User 1', - groups='base.group_user') - - cls.thread = cls.env['res.partner'].create({'name': 'a partner'}) + cls.test_partner = cls.env['res.partner'].create({'name': 'a partner'}) + cls.existing_message = cls.test_partner.message_post(body='Test') cls.title = 'Test title' cls.og_title = 'Test OG title' cls.og_description = 'Test OG description' @@ -77,58 +74,13 @@ class TestLinkPreview(MailCommon): """ return self._patched_get_html('image/png', content) - def test_01_link_preview_throttle(self): - with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html): - throttle = int(self.env['ir.config_parameter'].sudo().get_param('mail.link_preview_throttle', 99)) - link_previews = [] - for _ in range(throttle): - link_previews.append({'source_url': self.source_url, 'message_id': 1}) - self.env['mail.link.preview'].create(link_previews) - message = self.env['mail.message'].create({ - 'model': self.thread._name, - 'res_id': self.thread.id, - 'body': f'Nothing link', - }) - self.env['mail.link.preview']._create_link_previews(message) - link_preview_count = self.env['mail.link.preview'].search_count([('source_url', '=', self.source_url)]) - self.assertEqual(link_preview_count, throttle + 1) - - def test_02_link_preview_create(self): - with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html): - message = self.env['mail.message'].create({ - 'model': self.thread._name, - 'res_id': self.thread.id, - 'body': f'Nothing link', - }) - self.env['mail.link.preview']._create_link_previews(message) - self.assertBusNotifications( - [(self.cr.dbname, 'res.partner', self.env.user.partner_id.id)], - message_items=[{ - 'type': 'mail.record/insert', - 'payload': { - 'LinkPreview': [{ - 'id': link_preview.id, - 'message': {'id': message.id}, - 'image_mimetype': False, - 'og_description': self.og_description, - 'og_image': self.og_image, - 'og_mimetype': False, - 'og_title': self.og_title, - 'og_type': False, - 'og_site_name': False, - 'source_url': self.source_url, - }] for link_preview in message.link_preview_ids - } - }] - ) - def test_get_link_preview_from_url(self): test_cases = [ (self._patch_with_og_properties, self.source_url), (self._patch_without_og_properties, self.source_url), (self._patch_with_image_mimetype, self.og_image), ] - test_asserts = [ + expected_values = [ { 'og_description': self.og_description, 'og_image': self.og_image, @@ -154,7 +106,41 @@ class TestLinkPreview(MailCommon): }, ] session = requests.Session() - for (get_patch, url), expected in zip(test_cases, test_asserts): + for (get_patch, url), expected in zip(test_cases, expected_values): with self.subTest(get_patch=get_patch, url=url, expected=expected), patch.object(requests.Session, 'get', get_patch): preview = link_preview.get_link_preview_from_url(url, session) self.assertEqual(preview, expected) + + def test_link_preview(self): + with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html): + throttle = int(self.env['ir.config_parameter'].sudo().get_param('mail.link_preview_throttle', 99)) + self.env['mail.link.preview'].create([ + {'source_url': self.source_url, 'message_id': self.existing_message.id} + for _ in range(throttle) + ]) + message = self.test_partner.message_post( + body=Markup(f'Nothing link'), + ) + self.env['mail.link.preview']._create_from_message_and_notify(message) + link_preview_count = self.env['mail.link.preview'].search_count([('source_url', '=', self.source_url)]) + self.assertEqual(link_preview_count, throttle + 1) + self.assertBusNotifications( + [(self.cr.dbname, 'res.partner', self.env.user.partner_id.id)], + message_items=[{ + 'type': 'mail.record/insert', + 'payload': { + 'LinkPreview': [{ + 'id': link_preview.id, + 'message': {'id': message.id}, + 'image_mimetype': False, + 'og_description': self.og_description, + 'og_image': self.og_image, + 'og_mimetype': False, + 'og_title': self.og_title, + 'og_type': False, + 'og_site_name': False, + 'source_url': self.source_url, + }] for link_preview in message.link_preview_ids + } + }] + )