From f727e9d9b6a2029b8625ecff229ccd742598322b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 23 Feb 2018 14:36:40 +0100 Subject: [PATCH] [REF] rating: avoid storing rating_value on mail.message Currently the rating value coming from the rating application is stored on the message it belongs to. However storing it is not necessary as there is no direct search using it. Having a computed field is sufficient for all use cases we currently have in Odoo. Removing the store allow to gain queries. Indeed the field is not computed anymore after each message creation meaning we save queries by not having to check existing ratings. It allows to gain a lot of computation as mail.message is a critical model. On the whole community runbot when installing all modules this leads to a gain of more than 14K queries on 585K which means 2.4% of performances increase. Considering the code size of this optimization this is quite an interesting result. Looking at test_mail performance tests we gain several queries (2/3) for each new message which is coherent with the model change. Finally it allows to lessen the performance difference between tests done with test mail only and tests done with other modules already installed. This is especially simple mail thread-enable records. Related to task ID 51523. #Closes #23294. Done with blessing of @jem-odoo . --- addons/rating/models/mail_message.py | 2 +- addons/rating/models/mail_thread.py | 2 +- addons/test_mail/tests/test_performance.py | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/addons/rating/models/mail_message.py b/addons/rating/models/mail_message.py index b0190933d15..f6b3553b7f4 100644 --- a/addons/rating/models/mail_message.py +++ b/addons/rating/models/mail_message.py @@ -8,7 +8,7 @@ class MailMessage(models.Model): _inherit = 'mail.message' rating_ids = fields.One2many('rating.rating', 'message_id', string='Related ratings') - rating_value = fields.Float("Rating Value", compute='_compute_rating_value', store=True) + rating_value = fields.Float("Rating Value", compute='_compute_rating_value', store=False) @api.multi @api.depends('rating_ids', 'rating_ids.rating') diff --git a/addons/rating/models/mail_thread.py b/addons/rating/models/mail_thread.py index 66ca2139465..1b91b819969 100644 --- a/addons/rating/models/mail_thread.py +++ b/addons/rating/models/mail_thread.py @@ -10,7 +10,7 @@ class MailThread(models.AbstractModel): @api.multi @api.returns('self', lambda value: value.id) def message_post(self, **kwargs): - rating_value = kwargs.get('rating_value') + rating_value = kwargs.pop('rating_value', False) message = super(MailThread, self).message_post(**kwargs) if rating_value: ir_model = self.env['ir.model'].sudo().search([('model', '=', self._name)]) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 389503de9bd..232fc5f97ac 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=10, demo=10): # test_mail only: 8 - 8 + with self.assertQueryCount(admin=8, demo=8): # test_mail only: 8 - 8 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=22, demo=22): # test_mail only: 18 - 18 + with self.assertQueryCount(admin=18, demo=18): # test_mail only: 18 - 18 self.env['test_performance.mail'].create({'name': 'X'}) @users('admin', 'emp') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(admin=12, emp=12): # test_mail only: 10 - 10 + with self.assertQueryCount(admin=10, emp=10): # test_mail only: 10 - 10 self.env['mail.test.simple'].create({'name': 'Test'}) @users('admin', 'emp') @@ -137,7 +137,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(admin=12, emp=12): # test_mail only: 10 - 10 + with self.assertQueryCount(admin=10, emp=10): # test_mail only: 10 - 10 model.create({'name': 'Test'}) @users('admin', 'emp') @@ -243,7 +243,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=120, emp=153): # com runbot 118 - 151 // test_mail only: 114 - 146 + with self.assertQueryCount(admin=118, emp=150): # com runbot 116 - 148 // test_mail only: 114 - 146 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -260,7 +260,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=145, emp=193): # com runbot 143 - 191 // test_mail only: 137 - 183 + with self.assertQueryCount(admin=141, emp=187): # com runbot 139 - 185 // test_mail only: 137 - 183 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) @@ -276,7 +276,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=332, emp=394): # test_mail only: 319 - 379 + with self.assertQueryCount(admin=326, emp=386): # test_mail only: 319 - 379 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -315,7 +315,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=235, emp=275): # test_mail only: 228 - 267 + with self.assertQueryCount(admin=233, emp=272): # test_mail only: 228 - 267 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id,