diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py
index 9b1dd26b0ed..cb5b71d10b8 100644
--- a/addons/mail/models/mail_message.py
+++ b/addons/mail/models/mail_message.py
@@ -941,92 +941,63 @@ class Message(models.Model):
# MESSAGE READ / FETCH / FAILURE API
# ------------------------------------------------------
- @api.model
- def _message_read_dict_postprocess(self, messages, message_tree):
- """ Post-processing on values given by message_read. This method will
- handle partners in batch to avoid doing numerous queries.
-
- :param list messages: list of message, as get_dict result
- :param dict message_tree: {[msg.id]: msg browse record as super user}
- """
- # 1. Aggregate partners (author_id and partner_ids), attachments and tracking values
- partners = self.env['res.partner'].sudo()
- attachments = self.env['ir.attachment']
- message_ids = list(message_tree.keys())
- email_notification_tree = {}
- for message in message_tree.values():
- if message.author_id:
- partners |= message.author_id
- # find all notified partners
- email_notification_tree[message.id] = message.notification_ids.filtered(
- lambda n: n.notification_type == 'email' and n.res_partner_id.active and
- (n.notification_status in ('bounce', 'exception', 'canceled') or n.res_partner_id.partner_share))
- if message.attachment_ids:
- attachments |= message.attachment_ids
- partners |= self.env['mail.notification'].concat(*email_notification_tree.values()).mapped('res_partner_id')
- # Read partners as SUPERUSER -> message being browsed as SUPERUSER it is already the case
- partners_names = partners.name_get()
- partner_tree = dict((partner[0], partner) for partner in partners_names)
-
- # 2. Attachments as SUPERUSER, because could receive msg and attachments for doc uid cannot see
- attachments_data = attachments.sudo().read(['id', 'name', 'mimetype'])
+ def _message_format(self, fnames):
+ """Reads values from messages and formats them for the web client."""
+ self.check_access_rule('read')
+ vals_list = self._read_format(fnames)
safari = request and request.httprequest.user_agent.browser == 'safari'
- attachments_tree = dict((attachment['id'], {
- 'id': attachment['id'],
- 'filename': attachment['name'],
- 'name': attachment['name'],
- 'mimetype': 'application/octet-stream' if safari and attachment['mimetype'] and 'video' in attachment['mimetype'] else attachment['mimetype'],
- }) for attachment in attachments_data)
+ for vals in vals_list:
+ message_sudo = self.browse(vals['id']).sudo().with_prefetch(self.ids)
- # 3. Tracking values
- tracking_values = self.env['mail.tracking.value'].sudo().search([('mail_message_id', 'in', message_ids)])
- message_to_tracking = dict()
- tracking_tree = dict.fromkeys(tracking_values.ids, False)
- for tracking in tracking_values:
- groups = tracking.field_groups
- if not groups or self.env.is_superuser() or self.user_has_groups(groups):
- message_to_tracking.setdefault(tracking.mail_message_id.id, list()).append(tracking.id)
- tracking_tree[tracking.id] = {
- 'id': tracking.id,
- 'changed_field': tracking.field_desc,
- 'old_value': tracking.get_old_display_value()[0],
- 'new_value': tracking.get_new_display_value()[0],
- 'field_type': tracking.field_type,
- }
-
- # 4. Update message dictionaries
- for message_dict in messages:
- message_id = message_dict.get('id')
- message = message_tree[message_id]
- if message.author_id:
- author = partner_tree[message.author_id.id]
+ # Author
+ if message_sudo.author_id:
+ author = (message_sudo.author_id.id, message_sudo.author_id.display_name)
else:
- author = (0, message.email_from)
+ author = (0, message_sudo.email_from)
+
+ # Notifications
customer_email_status = (
- (all(n.notification_status == 'sent' for n in message.notification_ids if n.notification_type == 'email') and 'sent') or
- (any(n.notification_status == 'exception' for n in message.notification_ids if n.notification_type == 'email') and 'exception') or
- (any(n.notification_status == 'bounce' for n in message.notification_ids if n.notification_type == 'email') and 'bounce') or
+ (all(n.notification_status == 'sent' for n in message_sudo.notification_ids if n.notification_type == 'email') and 'sent') or
+ (any(n.notification_status == 'exception' for n in message_sudo.notification_ids if n.notification_type == 'email') and 'exception') or
+ (any(n.notification_status == 'bounce' for n in message_sudo.notification_ids if n.notification_type == 'email') and 'bounce') or
'ready'
)
customer_email_data = []
- for notification in email_notification_tree[message.id]:
- customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.notification_status))
+ filtered_notifications = message_sudo.notification_ids.filtered(lambda n:
+ n.notification_type == 'email' and n.res_partner_id.active and
+ (n.notification_status in ('bounce', 'exception', 'canceled') or n.res_partner_id.partner_share)
+ )
+ for notification in filtered_notifications:
+ customer_email_data.append((notification.res_partner_id.id, notification.res_partner_id.display_name, notification.notification_status))
- has_access_to_model = message.model and self.env[message.model].check_access_rights('read', raise_exception=False)
- main_attachment = None
- if message.attachment_ids and message.res_id and issubclass(self.pool[message.model], self.pool['mail.thread']) and has_access_to_model:
- main_attachment = self.env[message.model].browse(message.res_id).message_main_attachment_id
+ # Attachments
+ main_attachment = self.env['ir.attachment']
+ if message_sudo.attachment_ids and message_sudo.res_id and issubclass(self.pool[message_sudo.model], self.pool['mail.thread']):
+ main_attachment = self.env[message_sudo.model].sudo().browse(message_sudo.res_id).message_main_attachment_id
attachment_ids = []
- for attachment in message.attachment_ids:
- if attachment.id in attachments_tree:
- attachments_tree[attachment.id]['is_main'] = main_attachment == attachment
- attachment_ids.append(attachments_tree[attachment.id])
- tracking_value_ids = []
- for tracking_value_id in message_to_tracking.get(message_id, list()):
- if tracking_value_id in tracking_tree:
- tracking_value_ids.append(tracking_tree[tracking_value_id])
+ for attachment in message_sudo.attachment_ids:
+ attachment_ids.append({
+ 'id': attachment.id,
+ 'filename': attachment.name,
+ 'name': attachment.name,
+ 'mimetype': 'application/octet-stream' if safari and attachment.mimetype and 'video' in attachment.mimetype else attachment.mimetype,
+ 'is_main': main_attachment == attachment
+ })
- message_dict.update({
+ # Tracking values
+ tracking_value_ids = []
+ for tracking in message_sudo.tracking_value_ids:
+ groups = tracking.field_groups
+ if not groups or self.env.is_superuser() or self.user_has_groups(groups):
+ tracking_value_ids.append({
+ 'id': tracking.id,
+ 'changed_field': tracking.field_desc,
+ 'old_value': tracking.get_old_display_value()[0],
+ 'new_value': tracking.get_new_display_value()[0],
+ 'field_type': tracking.field_type,
+ })
+
+ vals.update({
'author_id': author,
'customer_email_status': customer_email_status,
'customer_email_data': customer_email_data,
@@ -1034,7 +1005,7 @@ class Message(models.Model):
'tracking_value_ids': tracking_value_ids,
})
- return True
+ return vals_list
def message_fetch_failed(self):
messages = self.search([
@@ -1114,44 +1085,25 @@ class Message(models.Model):
'moderation_status': 'pending_moderation'
}
"""
- message_values = self.read(self._get_message_format_fields())
- message_tree = dict((m.id, m) for m in self.sudo())
- self._message_read_dict_postprocess(message_values, message_tree)
-
- # add subtype data (is_note flag, is_discussion flag , subtype_description). Do it as sudo
- # because portal / public may have to look for internal subtypes
- 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)
+ vals_list = self._message_format(self._get_message_format_fields())
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')
- # fetch notification status
-
- notif_dict = defaultdict(lambda: defaultdict(list))
- notifs = self.env['mail.notification'].sudo().search([('mail_message_id', 'in', list(mid for mid in message_tree)), ('res_partner_id', '!=', False)])
-
- for notif in notifs:
- mid = notif.mail_message_id.id
- if notif.is_read:
- notif_dict[mid]['history_partner_ids'].append(notif.res_partner_id.id)
- else:
- notif_dict[mid]['needaction_partner_ids'].append(notif.res_partner_id.id)
-
- for message in message_values:
- message.update({
- 'needaction_partner_ids': notif_dict[message['id']]['needaction_partner_ids'],
- 'history_partner_ids': notif_dict[message['id']]['history_partner_ids'],
- 'is_note': message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == note_id,
- 'is_discussion': message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == com_id,
- 'subtype_description': message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['description']
+ for vals in vals_list:
+ message_sudo = self.browse(vals['id']).sudo().with_prefetch(self.ids)
+ notifs = message_sudo.notification_ids.filtered(lambda n: n.res_partner_id)
+ vals.update({
+ 'needaction_partner_ids': notifs.filtered(lambda n: not n.is_read).res_partner_id.ids,
+ 'history_partner_ids': notifs.filtered(lambda n: n.is_read).res_partner_id.ids,
+ 'is_note': message_sudo.subtype_id.id == note_id,
+ 'is_discussion': message_sudo.subtype_id.id == com_id,
+ 'subtype_description': message_sudo.subtype_id.description,
+ 'is_notification': vals['message_type'] == 'user_notification',
})
- message['is_notification'] = message['message_type'] == 'user_notification'
-
- if message['model'] and self.env[message['model']]._original_module:
- message['module_icon'] = modules.module.get_module_icon(self.env[message['model']]._original_module)
- return message_values
+ if vals['model'] and self.env[vals['model']]._original_module:
+ vals['module_icon'] = modules.module.get_module_icon(self.env[vals['model']]._original_module)
+ return vals_list
def _get_message_format_fields(self):
return [
diff --git a/addons/portal/models/mail_message.py b/addons/portal/models/mail_message.py
index cfda6d5eaa0..91d40d7a891 100644
--- a/addons/portal/models/mail_message.py
+++ b/addons/portal/models/mail_message.py
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
-from odoo import api, models
+from odoo import models
class MailMessage(models.Model):
@@ -15,12 +15,10 @@ class MailMessage(models.Model):
])
def _portal_message_format(self, fields_list):
- message_values = self.read(fields_list)
- message_tree = dict((m.id, m) for m in self.sudo())
- self._message_read_dict_postprocess(message_values, message_tree)
+ vals_list = self._message_format(fields_list)
IrAttachmentSudo = self.env['ir.attachment'].sudo()
- for message in message_values:
- for attachment in message.get('attachment_ids', []):
+ for vals in vals_list:
+ for attachment in vals.get('attachment_ids', []):
if not attachment.get('access_token'):
attachment['access_token'] = IrAttachmentSudo.browse(attachment['id']).generate_access_token()[0]
- return message_values
+ return vals_list
diff --git a/addons/portal_rating/models/mail_message.py b/addons/portal_rating/models/mail_message.py
index 9f2be5d1a25..20716960503 100644
--- a/addons/portal_rating/models/mail_message.py
+++ b/addons/portal_rating/models/mail_message.py
@@ -13,16 +13,16 @@ class MailMessage(models.Model):
field_list += ['rating_value']
return super(MailMessage, self)._portal_message_format(field_list)
- def _message_read_dict_postprocess(self, message_values, message_tree):
+ def _message_format(self, fnames):
""" Override the method to add information about a publisher comment
on each rating messages if requested, and compute a plaintext value of it.
"""
- res = super(MailMessage, self)._message_read_dict_postprocess(message_values, message_tree)
+ vals_list = super(MailMessage, self)._message_format(fnames)
if self._context.get('rating_include'):
infos = ["id", "publisher_comment", "publisher_id", "publisher_datetime", "message_id"]
related_rating = self.env['rating.rating'].sudo().search([('message_id', 'in', self.ids)]).read(infos)
mid_rating_tree = dict((rating['message_id'][0], rating) for rating in related_rating)
- for values in message_values:
- values["rating"] = mid_rating_tree.get(values['id'], {})
- return res
+ for vals in vals_list:
+ vals["rating"] = mid_rating_tree.get(vals['id'], {})
+ return vals_list
diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py
index 1b1a2ca3915..b952fe73388 100644
--- a/addons/test_mail/tests/test_performance.py
+++ b/addons/test_mail/tests/test_performance.py
@@ -3,7 +3,7 @@
import base64
from odoo.addons.base.tests.common import TransactionCaseWithUserDemo
-from odoo.tests.common import TransactionCase, users, warmup
+from odoo.tests.common import users, warmup
from odoo.tests import tagged
from odoo.tools import mute_logger, formataddr
@@ -253,7 +253,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
@users('__system__', 'emp')
@warmup
- @mute_logger('odoo.addons.mail.models.mail_mail')
+ @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail')
def test_mail_composer(self):
test_record = self.env['mail.test.full'].browse(self.test_record_full.id)
customer_id = self.customer.id
@@ -267,17 +267,17 @@ class TestMailAPIPerformance(BaseMailPerformance):
'partner_ids': [(4, customer_id)],
})
- with self.assertQueryCount(__system__=36, emp=41):
+ with self.assertQueryCount(__system__=31, emp=37):
composer.send_mail()
@users('__system__', 'emp')
@warmup
- @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
+ @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
def test_mail_composer_w_template(self):
test_record = self.env['mail.test.full'].browse(self.test_record_full.id)
test_template = self.env['mail.template'].browse(self.test_template_full.id)
- # TODO XDO/TDE/MAT FIXME non deterministic between 25 and 29 queries
- with self.assertQueryCount(__system__=28, emp=29):
+ # TODO FIXME non deterministic, last check 25 feb 2020. Runbot at most +7 compared to local.
+ with self.assertQueryCount(__system__=27, emp=28):
composer = self.env['mail.compose.message'].with_context({
'default_composition_mode': 'comment',
'default_model': test_record._name,
@@ -286,13 +286,13 @@ class TestMailAPIPerformance(BaseMailPerformance):
}).create({})
composer.onchange_template_id_wrapper()
- with self.assertQueryCount(__system__=44, emp=48):
+ with self.assertQueryCount(__system__=39, emp=44):
composer.send_mail()
# remove created partner to ensure tests are the same each run
self.env['res.partner'].sudo().search([('email', '=', 'nopartner.test@example.com')]).unlink()
- @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
+ @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@users('__system__', 'emp')
@warmup
def test_message_assignation_email(self):
@@ -307,7 +307,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
@warmup
def test_message_assignation_inbox(self):
record = self.env['mail.test.track'].create({'name': 'Test'})
- with self.assertQueryCount(__system__=25, emp=27):
+ with self.assertQueryCount(__system__=19, emp=22):
record.write({
'user_id': self.user_test.id,
})
@@ -345,7 +345,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
message_type='comment',
subtype_xmlid='mail.mt_comment')
- @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
+ @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@users('__system__', 'emp')
@warmup
def test_message_post_one_email_notification(self):
@@ -363,7 +363,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
def test_message_post_one_inbox_notification(self):
record = self.env['mail.test.simple'].create({'name': 'Test'})
- with self.assertQueryCount(__system__=20, emp=22):
+ with self.assertQueryCount(__system__=15, emp=18):
record.message_post(
body='Test Post Performances with an inbox ping
',
partner_ids=self.user_test.partner_id.ids,
@@ -696,6 +696,119 @@ class TestMailComplexPerformance(BaseMailPerformance):
self.assertEqual(rec1.message_ids[2].notified_partner_ids, self.partners | self.user_portal.partner_id)
self.assertEqual(len(rec1.message_ids), 3)
+ @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
+ @users('emp')
+ @warmup
+ def test_message_format(self):
+ """Test performance of `_message_format` and of `message_format` with
+ multiple messages with multiple attachments, different authors, various
+ notifications, and different tracking values.
+ Those messages might not make sense functionally but they are crafted to
+ cover as much of the code as possible in regard to number of queries.
+ """
+ name_field = self.env['ir.model.fields']._get(self.umbrella._name, 'name')
+ customer_id_field = self.env['ir.model.fields']._get(self.umbrella._name, 'customer_id')
+
+ messages = self.env['mail.message'].sudo().create([{
+ 'subject': 'Test 0',
+ 'body': 'Test 0
',
+ 'author_id': self.partners[0].id,
+ 'email_from': self.partners[0].email,
+ 'model': 'mail.test',
+ 'res_id': self.umbrella.id,
+ 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment'),
+ 'attachment_ids': [
+ (0, 0, {
+ 'name': 'test file 0 - %d' % j,
+ 'datas': 'data',
+ }) for j in range(2)
+ ],
+ 'notification_ids': [
+ (0, 0, {
+ 'res_partner_id': self.partners[3].id,
+ 'notification_type': 'inbox',
+ }),
+ (0, 0, {
+ 'res_partner_id': self.partners[4].id,
+ 'notification_type': 'email',
+ 'notification_status': 'exception',
+ }),
+ (0, 0, {
+ 'res_partner_id': self.partners[6].id,
+ 'notification_type': 'email',
+ 'notification_status': 'exception',
+ }),
+ ],
+ 'tracking_value_ids': [
+ (0, 0, {
+ 'field': name_field.id,
+ 'field_desc': 'Name',
+ 'old_value_char': 'old 0',
+ 'new_value_char': 'new 0',
+ }),
+ (0, 0, {
+ 'field': customer_id_field.id,
+ 'field_desc': 'Customer',
+ 'old_value_integer': self.partners[7].id,
+ 'new_value_integer': self.partners[8].id,
+ }),
+ ]
+ }, {
+ 'subject': 'Test 1',
+ 'body': 'Test 1
',
+ 'author_id': self.partners[1].id,
+ 'email_from': self.partners[1].email,
+ 'model': 'mail.test',
+ 'res_id': self.umbrella.id,
+ 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'),
+ 'attachment_ids': [
+ (0, 0, {
+ 'name': 'test file 1 - %d' % j,
+ 'datas': 'data',
+ }) for j in range(2)
+ ],
+ 'notification_ids': [
+ (0, 0, {
+ 'res_partner_id': self.partners[5].id,
+ 'notification_type': 'inbox',
+ }),
+ (0, 0, {
+ 'res_partner_id': self.partners[6].id,
+ 'notification_type': 'email',
+ 'notification_status': 'exception',
+ }),
+ ],
+ 'tracking_value_ids': [
+ (0, 0, {
+ 'field': name_field.id,
+ 'field_desc': 'Name',
+ 'old_value_char': 'old 1',
+ 'new_value_char': 'new 1',
+ }),
+ (0, 0, {
+ 'field': customer_id_field.id,
+ 'field_desc': 'Customer',
+ 'old_value_integer': self.partners[7].id,
+ 'new_value_integer': self.partners[8].id,
+ }),
+ ]
+ }])
+
+ with self.assertQueryCount(emp=6):
+ res = messages.message_format()
+ self.assertEqual(len(res), 2)
+ for message in res:
+ self.assertEqual(len(message['attachment_ids']), 2)
+
+ messages.flush()
+ messages.invalidate_cache()
+
+ with self.assertQueryCount(emp=18):
+ res = messages.message_format()
+ self.assertEqual(len(res), 2)
+ for message in res:
+ self.assertEqual(len(message['attachment_ids']), 2)
+
@tagged('mail_performance')
class TestMailHeavyPerformancePost(BaseMailPerformance):
@@ -815,7 +928,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance):
]
self.attachements = self.env['ir.attachment'].with_user(self.env.user).create(self.vals)
attachement_ids = self.attachements.ids
- with self.assertQueryCount(emp=91):
+ with self.assertQueryCount(emp=86):
self.cr.sql_log = self.warm and self.cr.sql_log_count
record.with_context({}).message_post(
body='Test body 
',
diff --git a/odoo/models.py b/odoo/models.py
index bbec718b5a3..a5d8ddb7900 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -2932,11 +2932,20 @@ Fields:
stored_fields.add(f.name)
self._read(stored_fields)
- # retrieve results from records; this takes values from the cache and
- # computes remaining fields
+ return self._read_format(fnames=fields, load=load)
+
+ def _read_format(self, fnames, load='_classic_read'):
+ """Returns a list of dictionaries mapping field names to their values,
+ with one dictionary per record that exists.
+
+ The output format is similar to the one expected from the `read` method.
+
+ The current method is different from `read` because it retrieves its
+ values from the cache without doing a query when it is avoidable.
+ """
data = [(record, {'id': record._ids[0]}) for record in self]
use_name_get = (load == '_classic_read')
- for name in fields:
+ for name in fnames:
convert = self._fields[name].convert_to_read
for record, vals in data:
# missing records have their vals empty