From 058d2fd4f6e2cea0ecb4ed92a06d1602c611a88a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 Apr 2018 15:25:49 +0200 Subject: [PATCH 1/3] [REF] mail: shortcut chat notification if no one to notify Well, it seems we gain a lot of query when avoiding to format a message we actually don't have to send on any bus. Cheers. Formatting message for bus notification is quite costly as it has to fetch data from message, partner, attachments, subtypes, ... meaning a lot of queries. With this commit we avoid calling that method when not necessary. It allows a gain of queries in simple cases when no-one has to be notified as shown in the performance counters. On overall community runbot about 9K queries are saved which is not bad for a 2-lines commit. --- addons/mail/models/res_partner.py | 2 ++ addons/test_mail/tests/test_performance.py | 26 +++++++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index aa7d52b408f..09554230025 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -232,6 +232,8 @@ class Partner(models.Model): @api.multi def _notify_by_chat(self, message): """ Broadcast the message to all the partner since """ + if not self: + return message_values = message.message_format()[0] notifications = [] for partner in self: diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 2b3442608c0..1990b3a225e 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -170,7 +170,7 @@ class TestAdvMailPerformance(TransactionCase): 'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id, }) - with self.assertQueryCount(margin=1, admin=49, emp=73): # test_mail only: 49 - 73 + with self.assertQueryCount(margin=1, admin=37, emp=58): # test_mail only: 37 - 58 activity.action_feedback(feedback='Zizisse Done !') @users('admin', 'emp') @@ -184,7 +184,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(margin=1, admin=51, emp=75): # test_mail only: 51 - 75 + with self.assertQueryCount(margin=1, admin=39, emp=60): # test_mail only: 39 - 60 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -196,7 +196,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=81, emp=105): # test_mail only: 79 - 103 + with self.assertQueryCount(margin=1, admin=68, emp=87): # com runbot: 67 - 85 // test_mail only: 66 - 85 record.write({ 'user_id': self.user_test.id, }) @@ -226,7 +226,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=24, emp=34): # test_mail only: 24 - 34 + with self.assertQueryCount(admin=15, emp=29): # com runbot: 13 - 20 // test_mail only: 15 - 29 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -237,7 +237,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=24, emp=34): # test_mail only: 24 - 34 + with self.assertQueryCount(admin=15, emp=29): # com runbot: 13 - 20 // test_mail only: 15 - 29 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -250,7 +250,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=74, emp=99): # com runbot: 72 - 97 // test_mail only: 72 - 97 + with self.assertQueryCount(margin=1, admin=66, emp=89): # com runbot: 57 - 75 // test_mail only: 64 - 87 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -385,7 +385,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(admin=114, emp=143): # com runbot 112 - 141 // test_mail only: 110 - 139 + with self.assertQueryCount(admin=105, emp=136): # com runbot 96 - 118 // test_mail only: 101 - 132 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -402,7 +402,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(admin=133, emp=174): # com runbot 131 - 172 // test_mail only: 129 - 170 + with self.assertQueryCount(admin=124, emp=167): # com runbot 115 - 150 // test_mail only: 120 - 163 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -472,7 +472,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=82, emp=108): # test_mail only: 80 - 106 + with self.assertQueryCount(admin=70, emp=91): # com runbot: 69 - 89 // test_mail only: 68 - 89 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) @@ -495,7 +495,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(margin=1, admin=235, emp=284): # test_mail only: 228 - 277 + with self.assertQueryCount(margin=1, admin=205, emp=244): # com runbot: 203 - 239 // test_mail only: 199 - 239 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -524,7 +524,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=147, emp=172): # test_mail only: 142 - 167 + with self.assertQueryCount(margin=1, admin=130, emp=151): # com runbot: 129 - 148 // test_mail only: 126 - 147 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -562,7 +562,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=152, emp=180): # test_mail only: 147 - 175 + with self.assertQueryCount(margin=1, admin=136, emp=159): # com runbot: 136 - 158 // test_mail only: 132 - 155 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, @@ -596,7 +596,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=57, emp=78): # test_mail only: 55 - 76 + with self.assertQueryCount(admin=58, emp=79): # test_mail only: 56 - 77 rec.write({ 'name': 'Test2', 'customer_id': customer_id, From 0603dab655cef084f5cce17967722d5b17ca545b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 Apr 2018 16:56:25 +0200 Subject: [PATCH 2/3] [IMP] test_mail: update top counters due to enterprise update. mail_push module in enterprise was adding too much extra queries. mail_push being auto-install in enterprise mail performance tests were done with its overhead taken into account. Commit done in enterprise cleaned a bit the code to lessen mail_push impact allowing to lessen some top counters. Community runbot counters are now nearer enterprise-based counters. This commit is related to enterprise commit https://github.com/odoo/enterprise/commit/0a3ba658f18e03fd76caba86cba7be32d047f0a9 . --- addons/test_mail/tests/test_performance.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 1990b3a225e..a47579e7431 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -196,7 +196,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=68, emp=87): # com runbot: 67 - 85 // test_mail only: 66 - 85 + with self.assertQueryCount(margin=1, admin=68, emp=86): # com runbot: 67 - 85 // test_mail only: 66 - 84 record.write({ 'user_id': self.user_test.id, }) @@ -226,7 +226,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=15, emp=29): # com runbot: 13 - 20 // test_mail only: 15 - 29 + with self.assertQueryCount(admin=15, emp=22): # com runbot: 13 - 20 // test_mail only: 15 - 22 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -237,7 +237,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=15, emp=29): # com runbot: 13 - 20 // test_mail only: 15 - 29 + with self.assertQueryCount(admin=15, emp=22): # com runbot: 13 - 20 // test_mail only: 15 - 22 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -250,7 +250,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=66, emp=89): # com runbot: 57 - 75 // test_mail only: 64 - 87 + with self.assertQueryCount(margin=1, admin=64, emp=82): # com runbot: 57 - 75 // test_mail only: 62 - 80 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -385,7 +385,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(admin=105, emp=136): # com runbot 96 - 118 // test_mail only: 101 - 132 + with self.assertQueryCount(admin=103, emp=125): # com runbot 96 - 118 // test_mail only: 99 - 121 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -402,7 +402,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(admin=124, emp=167): # com runbot 115 - 150 // test_mail only: 120 - 163 + with self.assertQueryCount(admin=122, emp=157): # com runbot 115 - 150 // test_mail only: 118 - 153 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -472,7 +472,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=70, emp=91): # com runbot: 69 - 89 // test_mail only: 68 - 89 + with self.assertQueryCount(admin=70, emp=90): # com runbot: 69 - 89 // test_mail only: 68 - 88 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) @@ -495,7 +495,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(margin=1, admin=205, emp=244): # com runbot: 203 - 239 // test_mail only: 199 - 239 + with self.assertQueryCount(margin=1, admin=205, emp=242): # com runbot: 203 - 239 // test_mail only: 199 - 236 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -524,7 +524,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(margin=1, admin=130, emp=151): # com runbot: 129 - 148 // test_mail only: 126 - 147 + with self.assertQueryCount(margin=1, admin=130, emp=149): # com runbot: 129 - 148 // test_mail only: 126 - 145 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, From c5c369355d095312d44a4793d6b77f7841577c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Apr 2018 16:14:43 +0200 Subject: [PATCH 3/3] [IMP] mail, mass_mailing: clean mail body processing and mail sent post processing Purpose is to lessen query count by avoiding to browse and/or fetch data when not necessary. This commit does two main things * first one is to avoid checking for existing statistics in mass_mailing where only checking existing of a mass_mailing on a mail to send is sufficient. It allows to avoid fetching data from statistics table; * second one is to rewrite part of the post_process code called after sending emails. Using list comprehension and mailing_id existence on mail_mail allows to save several queries on standard post flows; Most tests benefit from this code update. On overall community runbot about 3K queries are saved with those small optimizations. --- addons/mail/models/mail_mail.py | 18 ++++++++++-------- addons/mass_mailing/models/mail_mail.py | 17 +++++++++-------- addons/test_mail/tests/test_performance.py | 20 ++++++++++---------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index c4f5e6678df..fb4f9b76096 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -66,9 +66,10 @@ class MailMail(models.Model): @api.multi def unlink(self): # cascade-delete the parent message for all mails that are not created for a notification - to_cascade = self.search([('notification', '=', False), ('id', 'in', self.ids)]).mapped('mail_message_id') + mail_msg_cascade_ids = [mail.mail_message_id.id for mail in self if not mail.notification] res = super(MailMail, self).unlink() - to_cascade.unlink() + if mail_msg_cascade_ids: + self.env['mail.message'].browse(mail_msg_cascade_ids).unlink() return res @api.model @@ -130,21 +131,22 @@ class MailMail(models.Model): :return: True """ - notif_emails = self.filtered(lambda email: email.notification) - if notif_emails: + mail_message_notif_ids = [mail.mail_message_id.id for mail in self if mail.notification] + if mail_message_notif_ids: notifications = self.env['mail.notification'].search([ - ('mail_message_id', 'in', notif_emails.mapped('mail_message_id').ids), + ('mail_message_id', 'in', mail_message_notif_ids), ('is_email', '=', True)]) - if mail_sent: + if notifications and mail_sent: notifications.write({ 'email_status': 'sent', }) - else: + elif notifications: notifications.write({ 'email_status': 'exception', }) if mail_sent: - self.sudo().filtered(lambda self: self.auto_delete).unlink() + mail_to_delete_ids = [mail.id for mail in self if mail.auto_delete] + self.browse(mail_to_delete_ids).sudo().unlink() return True # ------------------------------------------------------ diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index 5f2d51aa54e..4ec9680c639 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -71,13 +71,13 @@ class MailMail(models.Model): new_href = href.replace(url, url + '/m/' + str(self.statistics_ids[0].id)) body = body.replace(href, new_href) - body = self.env['mail.thread']._replace_local_links(body) - - # generate tracking URL - if self.statistics_ids: + # generate tracking URL tracking_url = self._get_tracking_url() if tracking_url: body = tools.append_content_to_html(body, tracking_url, plaintext=False, container_tag='div') + + body = self.env['mail.thread']._replace_local_links(body) + return body @api.multi @@ -97,8 +97,9 @@ class MailMail(models.Model): @api.multi def _postprocess_sent_message(self, mail_sent=True): for mail in self: - if mail_sent is True and mail.statistics_ids: - mail.statistics_ids.write({'sent': fields.Datetime.now(), 'exception': False}) - elif mail_sent is False and mail.statistics_ids: - mail.statistics_ids.write({'exception': fields.Datetime.now()}) + if mail.mailing_id: + if mail_sent is True and mail.statistics_ids: + mail.statistics_ids.write({'sent': fields.Datetime.now(), 'exception': False}) + elif mail_sent is False and mail.statistics_ids: + mail.statistics_ids.write({'exception': fields.Datetime.now()}) return super(MailMail, self)._postprocess_sent_message(mail_sent=mail_sent) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index a47579e7431..adbe2213e66 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -196,7 +196,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=68, emp=86): # com runbot: 67 - 85 // test_mail only: 66 - 84 + with self.assertQueryCount(margin=1, admin=65, emp=83): # com runbot: 64 - 82 // test_mail only: 65 - 83 record.write({ 'user_id': self.user_test.id, }) @@ -250,7 +250,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=64, emp=82): # com runbot: 57 - 75 // test_mail only: 62 - 80 + with self.assertQueryCount(margin=1, admin=61, emp=79): # com runbot: 54 - 72 // test_mail only: 61 - 79 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -372,7 +372,7 @@ class TestHeavyMailPerformance(TransactionCase): }) mail_ids = mail.ids - with self.assertQueryCount(admin=15, emp=23): # test_mail only: 13 - 21 + with self.assertQueryCount(admin=13, emp=20): # test_mail only: 13 - 20 self.env['mail.mail'].browse(mail_ids).send() self.assertEqual(mail.body_html, '

Test

') @@ -385,7 +385,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(admin=103, emp=125): # com runbot 96 - 118 // test_mail only: 99 - 121 + with self.assertQueryCount(admin=97, emp=119): # com runbot 90 - 112 // test_mail only: 97 - 119 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -402,7 +402,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(admin=122, emp=157): # com runbot 115 - 150 // test_mail only: 118 - 153 + with self.assertQueryCount(admin=116, emp=151): # com runbot 109 - 144 // test_mail only: 116 - 151 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -472,7 +472,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=70, emp=90): # com runbot: 69 - 89 // test_mail only: 68 - 88 + with self.assertQueryCount(admin=67, emp=87): # com runbot: 66 - 86 // test_mail only: 67 - 87 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) @@ -495,7 +495,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(margin=1, admin=205, emp=242): # com runbot: 203 - 239 // test_mail only: 199 - 236 + with self.assertQueryCount(margin=1, admin=196, emp=232): # com runbot: 194 - 231 // test_mail only: 196 - 232 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -524,7 +524,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(margin=1, admin=130, emp=149): # com runbot: 129 - 148 // test_mail only: 126 - 145 + with self.assertQueryCount(margin=1, admin=124, emp=143): # com runbot: 123 - 142 // test_mail only: 124 - 143 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -562,7 +562,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(margin=1, admin=136, emp=159): # com runbot: 136 - 158 // test_mail only: 132 - 155 + with self.assertQueryCount(margin=1, admin=130, emp=153): # test_mail only: 130 - 153 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, @@ -596,7 +596,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=58, emp=79): # test_mail only: 56 - 77 + with self.assertQueryCount(admin=55, emp=76): # test_mail only: 55 - 76 rec.write({ 'name': 'Test2', 'customer_id': customer_id,