pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[MERGE] mail, test_mail: remove aggregation from message_format

Browse Source 
Remove aggregation from `message_format `and `_message_read_dict_postprocess`.
There should be no functional change in this PR.

Purpose
=======

The existing code working with "tree" for aggregation and doing explicit "read"
or "search" for related fields does not seem necessary since the ORM of v13.

Considering that it adds complexity to the method, if its original purpose is
not necessary anymore, it would be better to change it.

The notable changes of the ORM that would justify this task are:
 - single cache, shared between current user and sudo (which is called a lot in
   those methods and might explain why the "tree" mechanism was put in place
   originally)
 - m2o and o2m being kept consistent with each other, which allows accessing o2m
   through fields directly (instead of having to use "search" to ensure getting
   consistent data)

Explanation
===========

Doing "read" or "search" could be counter-productive if the data are already in
cache because using those methods will lead to a query no matter what.
The ideal way is to simply access the fields through the records.

As for the "tree" and aggregation in general, the prefetch is automatically
taking care of that when iterating.

task-2180311

closes odoo/odoo#43841

Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
This commit is contained in:
Odoo's Mergebot
2020-03-09 16:04:48 +01:00
committed by GitHub
parent 0712c5222c e3afe4d067
commit a4e1b7c44e
5 changed files with 209 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_message.py
+62 -110
View File
@@ -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 [
addons/portal/models/mail_message.py
+5 -7
View File
@@ -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
addons/portal_rating/models/mail_message.py
+5 -5
View File
@@ -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
addons/test_mail/tests/test_performance.py
+125 -12
View File
@@ -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='<p>Test Post Performances with an inbox ping</p>',
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': '<p>Test 0</p>',
'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': '<p>Test 1</p>',
'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='<p>Test body <img src="cid:cid1"> <img src="cid:cid2"></p>',
odoo/models.py
+12 -3
View File
@@ -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