From 1e4ee402a359c0b061439d95b27acc0d446db204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Mar 2018 13:58:44 +0200 Subject: [PATCH 1/3] [IMP] mail: use xmlid_to_res_id instead of ref to find subtype in message_format Deleting comment / note / activity subtypes make the whole system quite impossible to use. Indeed the whole communication mechanism is based on those subtypes that are hardcoded. There are some use of env.ref() about subtypes to try to avoid having those removed. As there is some work to make some xml ids impossible to delete we can already update some code to use xmlid_to_res_id instead of env.ref(). This leads to some gain in query count as there is no exists() that is performed. Gain: 1 query on most tests implying post, because message_format is called by bus notification. About 1.5K queries on community runbot --- addons/mail/models/mail_message.py | 9 ++++--- addons/test_mail/tests/test_performance.py | 28 +++++++++++----------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index e3b10df4096..482afcbe4fe 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -412,10 +412,13 @@ class Message(models.Model): subtype_ids = [msg['subtype_id'][0] for msg in message_values if msg['subtype_id']] subtypes = self.env['mail.message.subtype'].sudo().browse(subtype_ids).read(['internal', 'description','id']) subtypes_dict = dict((subtype['id'], subtype) for subtype in subtypes) - xml_comment_id = self.env.ref('mail.mt_comment').id + + com_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') + note_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note') + for message in message_values: - message['is_note'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['internal'] - message['is_discussion'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == xml_comment_id + message['is_note'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == note_id + message['is_discussion'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == com_id message['subtype_description'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['description'] if message['model'] and self.env[message['model']]._original_module: message['module_icon'] = modules.module.get_module_icon(self.env[message['model']]._original_module) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 2bdce953adb..6a9b5ee501c 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -167,7 +167,7 @@ class TestAdvMailPerformance(TransactionCase): 'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id, }) - with self.assertQueryCount(admin=59, emp=88): # test_mail only: 58 - 87 + with self.assertQueryCount(admin=58, emp=87): # test_mail only: 57 - 86 activity.action_feedback(feedback='Zizisse Done !') @users('admin', 'emp') @@ -181,7 +181,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(admin=59, emp=88): # test_mail only: 58 - 87 + with self.assertQueryCount(admin=58, emp=87): # test_mail only: 57 - 86 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -193,7 +193,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=90, emp=117): # test_mail only: 88 - 115 + with self.assertQueryCount(admin=89, emp=116): # test_mail only: 87 - 114 record.write({ 'user_id': self.user_test.id, }) @@ -203,7 +203,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=50, emp=65): # test_mail only: 50 - 65 + with self.assertQueryCount(admin=49, emp=64): # test_mail only: 49 - 64 record.write({ 'user_id': self.user_test.id, }) @@ -223,7 +223,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=28, emp=39): # test_mail only: 28 - 39 + with self.assertQueryCount(admin=27, emp=38): # test_mail only: 27 - 38 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -234,7 +234,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=28, emp=39): # test_mail only: 28 - 39 + with self.assertQueryCount(admin=27, emp=38): # test_mail only: 27 - 38 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -247,7 +247,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=79, emp=106): # com runbot 77 - 104 // test_mail only: 77 - 104 + with self.assertQueryCount(admin=78, emp=105): # com runbot 76 - 103 // test_mail only: 76 - 103 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -259,7 +259,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=41, emp=53): # com runbot 39 - 51 // test_mail only: 41 - 53 + with self.assertQueryCount(admin=40, emp=52): # com runbot 38 - 50 // test_mail only: 40 - 52 record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -382,7 +382,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=117, emp=148): # com runbot 115 - 146 // test_mail only: 113 - 144 + with self.assertQueryCount(admin=116, emp=147): # com runbot 114 - 145 // test_mail only: 112 - 143 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -399,7 +399,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=140, emp=185): # com runbot 138 - 183 // test_mail only: 136 - 181 + with self.assertQueryCount(admin=139, emp=184): # com runbot 137 - 182 // test_mail only: 135 - 180 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) @@ -471,7 +471,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=92, emp=121): # test_mail only: 90 - 119 + with self.assertQueryCount(admin=91, emp=120): # test_mail only: 89 - 118 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) @@ -494,7 +494,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=322, emp=383): # test_mail only: 315 - 376 + with self.assertQueryCount(admin=320, emp=381): # test_mail only: 313 - 374 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -523,7 +523,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=228, emp=265): # test_mail only: 224 - 261 + with self.assertQueryCount(admin=227, emp=264): # test_mail only: 223 - 260 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -561,7 +561,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=231, emp=269): # test_mail only: 226 - 265 + with self.assertQueryCount(admin=230, emp=268): # test_mail only: 225 - 264 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, From a2511f3a5aeb0ea33c52f96d785f2d8f4da847dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Mar 2018 14:08:36 +0200 Subject: [PATCH 2/3] [IMP] mail: use xmlid_to_res_id instead of ref to find subtype in various post methods As explained in previous commit we can use xmlid_to_res_id instead of ref() which leads to some query gain. We gain 1 query on most post due notably to notify and logging is still improved. This leads to a gain of about 4K queries on community runbot. --- addons/mail/models/mail_thread.py | 6 +++--- addons/test_mail/tests/test_performance.py | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index d41689fdf04..1b52ec94e5d 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -461,7 +461,7 @@ class MailThread(models.AbstractModel): """ self.ensure_one() # get the subtype of the comment Message - subtype_comment = self.env.ref('mail.mt_comment') + subtype_comment = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') # get the ids of the comment and not-comment of the thread # TDE check: sudo on mail.message, to be sure all messages are moved ? @@ -1986,7 +1986,7 @@ class MailThread(models.AbstractModel): 'message_type': 'notification', 'partner_ids': partner_ids, 'model': False, - 'subtype_id': self.env['ir.model.data'].sudo().xmlid_to_res_id('mail.mt_note'), + 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), 'record_name': False, 'reply_to': self.env['mail.thread'].sudo()._notify_get_reply_to([0])[0], 'message_id': tools.generate_tracking_message_id('message-notify'), @@ -2020,7 +2020,7 @@ class MailThread(models.AbstractModel): 'message_type': message_type, 'model': kwargs.get('model', self._name), 'res_id': self.ids[0] if self.ids else False, - 'subtype_id': self.env.ref('mail.mt_note').id, + 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), 'record_name': False, 'reply_to': self.env['mail.thread'].sudo()._notify_get_reply_to([0])[0], 'message_id': tools.generate_tracking_message_id('message-notify'), diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 6a9b5ee501c..cb392019f0b 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -77,7 +77,7 @@ class TestMailPerformance(TransactionCase): 'partner_id': self.env.ref('base.res_partner_12').id, }) - with self.assertQueryCount(admin=8, demo=8): # test_mail only: 8 - 8 + with self.assertQueryCount(admin=7, demo=7): # test_mail only: 7 - 7 record.track = 'X' @users('admin', 'demo') @@ -93,13 +93,13 @@ class TestMailPerformance(TransactionCase): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(admin=18, demo=18): # test_mail only: 18 - 18 + with self.assertQueryCount(admin=16, demo=16): # test_mail only: 16 - 16 self.env['test_performance.mail'].create({'name': 'X'}) @users('admin', 'emp') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(admin=10, emp=10): # test_mail only: 10 - 10 + with self.assertQueryCount(admin=9, emp=9): # test_mail only: 9 - 9 self.env['mail.test.simple'].create({'name': 'Test'}) @users('admin', 'emp') @@ -148,7 +148,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(admin=10, emp=10): # test_mail only: 10 - 10 + with self.assertQueryCount(admin=9, emp=9): # test_mail only: 9 - 9 model.create({'name': 'Test'}) @users('admin', 'emp') @@ -193,7 +193,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=89, emp=116): # test_mail only: 87 - 114 + with self.assertQueryCount(admin=88, emp=115): # test_mail only: 86 - 113 record.write({ 'user_id': self.user_test.id, }) @@ -203,7 +203,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=49, emp=64): # test_mail only: 49 - 64 + with self.assertQueryCount(admin=48, emp=63): # test_mail only: 48 - 63 record.write({ 'user_id': self.user_test.id, }) @@ -213,7 +213,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=3, emp=3): # test_mail only: 3 - 3 + with self.assertQueryCount(admin=2, emp=2): # test_mail only: 2 - 2 record._message_log( body='

Test _message_log

', message_type='comment') @@ -471,7 +471,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=91, emp=120): # test_mail only: 89 - 118 + with self.assertQueryCount(admin=90, emp=119): # test_mail only: 88 - 117 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) @@ -494,7 +494,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=320, emp=381): # test_mail only: 313 - 374 + with self.assertQueryCount(admin=319, emp=380): # com runbot: 319 - 179 // test_mail only: 312 - 373 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -595,7 +595,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=65, emp=92): # test_mail only: 63 - 90 + with self.assertQueryCount(admin=64, emp=91): # test_mail only: 62 - 89 rec.write({ 'name': 'Test2', 'customer_id': customer_id, From 4451e505cd9505ddd862b4312daec26089e9c7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Mar 2018 14:15:11 +0200 Subject: [PATCH 3/3] [IMP] mail: use xmlid_to_res_id instead of ref in remaining places of mail As explained in previous commit we can use xmlid_to_res_id instead of ref() which leads to some query gain. This commit update some other less important places: when setting activities as done and in the composer. This lead to a gain in tests involving activities and composer, including tracking triggering a template. On community runbot this leads to a gain of about 1K queries. --- addons/mail/models/mail_activity.py | 2 +- addons/mail/wizard/mail_compose_message.py | 4 ++-- addons/test_mail/tests/test_performance.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 0f849526b90..2d252f911c4 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -300,7 +300,7 @@ class MailActivity(models.Model): record.message_post_with_view( 'mail.message_activity_done', values={'activity': activity}, - subtype_id=self.env.ref('mail.mt_activities').id, + subtype_id=self.env['ir.model.data'].xmlid_to_res_id('mail.mt_activities'), mail_activity_type_id=activity.activity_type_id.id, ) message |= record.message_ids[0] diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 573e54898b4..4cc06574ec4 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -123,7 +123,7 @@ class MailComposer(models.TransientModel): domain="[('model', '=', model)]") # mail_message updated fields message_type = fields.Selection(default="comment") - subtype_id = fields.Many2one(default=lambda self: self.sudo().env.ref('mail.mt_comment', raise_if_not_found=False).id) + subtype_id = fields.Many2one(default=lambda self: self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment')) @api.multi def check_access_rule(self, operation): @@ -239,7 +239,7 @@ class MailComposer(models.TransientModel): elif wizard.subtype_id: subtype_id = wizard.subtype_id.id else: - subtype_id = self.sudo().env.ref('mail.mt_comment', raise_if_not_found=False).id + subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') for res_ids in sliced_res_ids: batch_mails = Mail diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index cb392019f0b..b8e3b71d249 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -167,7 +167,7 @@ class TestAdvMailPerformance(TransactionCase): 'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id, }) - with self.assertQueryCount(admin=58, emp=87): # test_mail only: 57 - 86 + with self.assertQueryCount(admin=57, emp=86): # test_mail only: 56 - 85 activity.action_feedback(feedback='Zizisse Done !') @users('admin', 'emp') @@ -181,7 +181,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(admin=58, emp=87): # test_mail only: 57 - 86 + with self.assertQueryCount(admin=57, emp=86): # test_mail only: 56 - 85 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -399,7 +399,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=139, emp=184): # com runbot 137 - 182 // test_mail only: 135 - 180 + with self.assertQueryCount(admin=138, emp=183): # com runbot 136 - 181 // test_mail only: 134 - 179 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) @@ -595,7 +595,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=64, emp=91): # test_mail only: 62 - 89 + with self.assertQueryCount(admin=63, emp=90): # test_mail only: 61 - 88 rec.write({ 'name': 'Test2', 'customer_id': customer_id,