[REF] mail: optimize notification recipients data fetch through SQL query

In this commit we update _notify method of mail.message to fetch most data
related to notification process in a single query.  Indeed posting a
message can be a costly process, especially when involving several partners,
channels, depending on their notification status, user groups, ... Most
of the required data can be fetched in a single hand-tuned query.

Purpose is to lessen query count. Side effect of this commit is that code
is less recordset based and therefore more low-level. It is also harder to
customize the behavior through inheritance as it implies rewriting a bit
more of code.

This query is located in a private method on follower model. It takes
parameters allowing to either fetch data related to followers of a given
subtype, either related to some partners and channels. First use case is
related to the process of posting a message with a subtype that triggers
notification of followers. Second use case is related to notifying some
specific partners or channels of a given message. It takes notably into
consideration internal subtypes and active field of partners, allowing to
correctly handle corner case for deactivated followers and portal / share
partners.

Notification process is divided into two steps. First one is to compute
recipient data. Second one is to apply the result of this computation and
call the various notification mechanisms accordingly. Doing so allows to
easily re-call notification process or compute some data differently
if necessary.

API of notification methods is updated to allow passing notably the values
used to create the newly-posted message. It allows in some simple case to
avoid browsing message and partners when no notification is necessary. It
saves some queries in simple case like posting messages with specific
subtypes not often followed or logging notes without mentioning people.

This commit allows to save a few queries on several performance tests as
indicated by updated performance counters. Future commits will gradually
propagate the use of this data to further lessen query count.

This commit is linked to task ID 47934 and PR #24033. No functional change
should occur with this commit.
This commit is contained in:
Thibault Delavallée
2018-08-07 15:42:45 +02:00
parent 40922cc027
commit 6de76cc4f1
4 changed files with 208 additions and 62 deletions
+89
View File
@@ -76,6 +76,95 @@ class Followers(models.Model):
# Private tools methods to fetch followers data
# --------------------------------------------------
def _get_recipient_data(self, records, subtype_id, pids=None, cids=None):
""" Private method allowing to fetch recipients data based on a subtype.
Purpose of this method is to fetch all data necessary to notify recipients
in a single query. It fetches data from
* followers (partners and channels) of records that follow the given
subtype if records and subtype are set;
* partners if pids is given;
* channels if cids is given;
:param records: fetch data from followers of records that follow subtype_id;
:param subtype_id: mail.message.subtype to check against followers;
:param pids: additional set of partner IDs from which to fetch recipient data;
:param cids: additional set of channel IDs from which to fetch recipient data;
:return: list of recipient data which is a tuple containing
partner ID (void if channel ID),
channel ID (void if partner ID),
active value (always True for channels),
share status of partner (void as irrelevant if channel ID),
notification status of partner or channel (email or inbox),
user groups of partner (void as irrelevant if channel ID),
"""
if records and subtype_id:
query = """
WITH sub_followers AS (
SELECT fol.id, fol.partner_id, fol.channel_id, subtype.internal
FROM mail_followers fol
RIGHT JOIN mail_followers_mail_message_subtype_rel subrel
ON subrel.mail_followers_id = fol.id
RIGHT JOIN mail_message_subtype subtype
ON subtype.id = subrel.mail_message_subtype_id
WHERE subrel.mail_message_subtype_id = %%s AND fol.res_model = %%s AND fol.res_id IN %%s
)
SELECT partner.id as pid, NULL AS cid,
partner.active as active, partner.partner_share as pshare, NULL as ctype,
users.notification_type AS notif, array_agg(groups.id) AS groups
FROM res_partner partner
LEFT JOIN res_users users ON users.partner_id = partner.id
LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id
LEFT JOIN res_groups groups ON groups.id = groups_rel.gid
WHERE EXISTS (
SELECT partner_id FROM sub_followers
WHERE sub_followers.channel_id IS NULL
AND sub_followers.partner_id = partner.id
AND (sub_followers.internal <> TRUE OR partner.partner_share <> TRUE)
) %s
GROUP BY partner.id, users.notification_type
UNION
SELECT NULL AS pid, channel.id AS cid,
TRUE as active, NULL AS pshare, channel.channel_type AS ctype,
CASE WHEN channel.email_send = TRUE THEN 'email' ELSE 'inbox' END AS notif, NULL AS groups
FROM mail_channel channel
WHERE EXISTS (
SELECT channel_id FROM sub_followers WHERE partner_id IS NULL AND sub_followers.channel_id = channel.id
) %s
""" % ('OR partner.id IN %s' if pids else '', 'OR channel.id IN %s' if cids else '')
params = [subtype_id, records._name, tuple(records.ids)]
if pids:
params.append(tuple(pids))
if cids:
params.append(tuple(cids))
self.env.cr.execute(query, tuple(params))
res = self.env.cr.fetchall()
elif pids or cids:
params, query_pid, query_cid = [], '', ''
if pids:
query_pid = """
SELECT partner.id as pid, NULL AS cid,
partner.active as active, partner.partner_share as pshare, NULL as ctype,
users.notification_type AS notif, NULL AS groups
FROM res_partner partner
LEFT JOIN res_users users ON users.partner_id = partner.id
WHERE partner.id IN %s"""
params.append(tuple(pids))
if cids:
query_cid = """
SELECT NULL AS pid, channel.id AS cid,
TRUE as active, NULL AS pshare, channel.channel_type AS ctype,
CASE when channel.email_send = TRUE then 'email' else 'inbox' end AS notif, NULL AS groups
FROM mail_channel channel WHERE channel.id IN %s """
params.append(tuple(cids))
query = ' UNION'.join(x for x in [query_pid, query_cid] if x)
self.env.cr.execute(query, tuple(params))
res = self.env.cr.fetchall()
else:
res = []
return res
def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False):
""" Private method allowing to fetch follower data from several documents of a given model.
Followers can be filtered given partner IDs and channel IDs.
+92 -43
View File
@@ -976,46 +976,92 @@ class Message(models.Model):
#------------------------------------------------------
@api.multi
def _notify(self, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True):
""" Compute recipients to notify based on specified recipients and document
followers. Delegate notification to partners to send emails and bus notifications
and to channels to broadcast messages on channels """
group_user = self.env.ref('base.group_user')
# have a sudoed copy to manipulate partners (public can go here with website modules like forum / blog / ... )
self_sudo = self.sudo()
def _notify(self, record, msg_vals, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True):
""" Main notification method. This method basically does two things
* call ``_notify_compute_recipients`` that computes recipients to
notify based on message record or message creation values if given
(to optimize performance if we already have data computed);
* call ``_notify_recipients`` that performs the notification process;
:param record: record on which the message is posted, if any;
:param msg_vals: dictionary of values used to create the message. If given
it is used instead of accessing ``self`` to lesen query count in some
simple cases where no notification is actually required;
:param force_send: tells whether to send notification emails within the
current transaction or to use the email queue;
:param send_after_commit: if force_send, tells whether to send emails after
the transaction has been committed using a post-commit hook;
:param model_description: optional data used in notification process (see
notification templates);
:param mail_auto_delete: delete notification emails once sent;
"""
msg_vals = msg_vals if msg_vals else {}
rdata = self._notify_compute_recipients(record, msg_vals)
return self._notify_recipients(
rdata, record, msg_vals,
force_send=force_send, send_after_commit=send_after_commit,
model_description=model_description, mail_auto_delete=mail_auto_delete)
@api.multi
def _notify_compute_recipients(self, record, msg_vals):
""" Compute recipients to notify based on subtype and followers. This
method returns data structured as expected for ``_notify_recipients``. """
msg_sudo = self.sudo()
pids = [x[1] for x in msg_vals.get('partner_ids')] if 'partner_ids' in msg_vals else msg_sudo.partner_ids.ids
cids = [x[1] for x in msg_vals.get('channel_ids')] if 'channel_ids' in msg_vals else msg_sudo.channel_ids.ids
subtype_id = msg_vals.get('subtype_id') if 'subtype_id' in msg_vals else msg_sudo.subtype_id.id
recipient_data = {
'partners': [],
'channels': [],
}
res = self.env['mail.followers']._get_recipient_data(record, subtype_id, pids, cids)
author_id = msg_vals.get('author_id') or self.author_id.id if res else False
for pid, cid, active, pshare, ctype, notif, groups in res:
if pid and pid == author_id: # do not notify the author of its own messages
continue
if pid:
pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups}
if notif == 'inbox':
recipient_data['partners'].append(dict(pdata, notif=notif, type='user'))
else:
if not pshare and notif: # has an user and is not shared, is therefore user
recipient_data['partners'].append(dict(pdata, notif='email', type='user'))
elif pshare and notif: # has an user but is shared, is therefore portal
recipient_data['partners'].append(dict(pdata, notif='email', type='portal'))
else: # has no user, is therefore customer
recipient_data['partners'].append(dict(pdata, notif='email', type='customer'))
elif cid:
recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype})
return recipient_data
@api.multi
def _notify_recipients(self, rdata, record, msg_vals,
force_send=False, send_after_commit=True,
model_description=False, mail_auto_delete=True):
""" Main method implementing the notification process.
:param rdata: dict containing recipients data: {
'partners': list of dict containing partner data: id, share status (boolean),
notification type ('email' or 'inbox'), type (main classification group used
in notification process), groups (user groups)
'channels': list of dict containing channel data: id, notification type
('email' for mailing list otherwise 'inbox'), type (channel_type)
}
"""
self.ensure_one()
partners_sudo = self_sudo.partner_ids
channels_sudo = self_sudo.channel_ids
# all followers of the mail.message document have to be added as partners and notified
# and filter to employees only if the subtype is internal
if self_sudo.subtype_id and self.model and self.res_id:
followers = self_sudo.env['mail.followers'].search([
('res_model', '=', self.model),
('res_id', '=', self.res_id),
('subtype_ids', 'in', self_sudo.subtype_id.id),
])
if self_sudo.subtype_id.internal:
followers = followers.filtered(lambda fol: fol.channel_id or (fol.partner_id.user_ids and group_user in fol.partner_id.user_ids[0].mapped('groups_id')))
channels_sudo |= followers.mapped('channel_id')
partners_sudo |= followers.mapped('partner_id')
email_cids = [r['id'] for r in rdata['channels'] if r['notif'] == 'email']
email_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'email']
inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox']
# remove author from notified partners
if not self._context.get('mail_notify_author', False) and self_sudo.author_id:
partners_sudo = partners_sudo - self_sudo.author_id
# list channels and partner by notification type
email_channels = channels_sudo.filtered(lambda channel: channel.email_send)
notif_partners = partners_sudo.filtered(lambda partner: 'inbox' in partner.mapped('user_ids.notification_type'))
email_partner = partners_sudo - notif_partners
#update message, with maybe custom values
message_values = {}
if channels_sudo:
message_values['channel_ids'] = [(6, 0, channels_sudo.ids)]
if partners_sudo:
message_values['needaction_partner_ids'] = [(6, 0, partners_sudo.ids)]
if rdata['channels']:
message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])]
if rdata['partners']:
message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners']])]
if self.model and self.res_id and hasattr(self.env[self.model], 'message_get_message_notify_values'):
message_values.update(self.env[self.model].browse(self.res_id).message_get_message_notify_values(self, message_values))
if message_values:
@@ -1024,16 +1070,18 @@ class Message(models.Model):
# notify partners and channels
# those methods are called as SUPERUSER because portal users posting messages
# have no access to partner model. Maybe propagating a real uid could be necessary.
if email_channels or email_partner:
partners_sudo.search([
if email_cids or email_pids:
self.env['res.partner'].sudo().search([
'|',
('id', 'in', (email_partner).ids),
('channel_ids', 'in', email_channels.ids),
('email', '!=', self_sudo.author_id.email or self_sudo.email_from),
('id', 'in', email_pids),
('channel_ids', 'in', email_cids),
('email', '!=', self.author_id.email or self.email_from),
])._notify(self, force_send=force_send, send_after_commit=send_after_commit, model_description=model_description, mail_auto_delete=mail_auto_delete)
notif_partners._notify_by_chat(self)
channels_sudo._notify(self)
self.env['res.partner'].sudo().browse(inbox_pids)._notify_by_chat(self)
if rdata['channels']:
self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(self)
return True
@@ -1098,7 +1146,8 @@ class Message(models.Model):
})
# proceed with notification process to send notification emails and Inbox messages
for message in self:
message._notify()
record = self.env[message.model].browse(message.res_id) if message.model and message.res_id else None
message._notify(record, {})
@api.multi
def _moderate_send_reject_email(self, subject, comment):
+13 -5
View File
@@ -2014,6 +2014,7 @@ class MailThread(models.AbstractModel):
'parent_id': parent_id,
'subtype_id': subtype_id,
'partner_ids': [(4, pid) for pid in partner_ids],
'channel_ids': kwargs.get('channel_ids', []),
'add_sign': add_sign
})
if notif_layout:
@@ -2033,16 +2034,23 @@ class MailThread(models.AbstractModel):
self._message_post_after_hook(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete)
return new_message
def _message_post_after_hook(self, message, values, model_description=False, mail_auto_delete=True):
def _message_post_after_hook(self, message, msg_vals, model_description=False, mail_auto_delete=True):
""" Hook to add custom behavior after having posted the message. Both
message and computed value are given, to try to lessen query count by
using already-computed values instead of having to rebrowse things. """
# Notify recipients of the newly-created message (Inbox / Email + channels)
if values.get('moderation_status') != 'pending_moderation':
message._notify(force_send=self.env.context.get('mail_notify_force_send', True), model_description=model_description, mail_auto_delete=mail_auto_delete)
if msg_vals.get('moderation_status') != 'pending_moderation':
message._notify(
self, msg_vals,
force_send=self.env.context.get('mail_notify_force_send', True),
send_after_commit=True,
model_description=model_description,
mail_auto_delete=mail_auto_delete,
)
# Post-process: subscribe author
if values['author_id'] and values['model'] and self.ids and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'):
self._message_subscribe([values['author_id']])
if msg_vals['author_id'] and msg_vals['model'] and self.ids and msg_vals['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'):
self._message_subscribe([msg_vals['author_id']])
else:
message._notify_pending_by_chat()
+14 -14
View File
@@ -173,7 +173,7 @@ class TestAdvMailPerformance(TransactionCase):
'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id,
})
with self.assertQueryCount(margin=1, admin=37, emp=58): # test_mail only: 37 - 58
with self.assertQueryCount(margin=1, admin=27, emp=48): # com runbot: 27 - 47 // test_mail only: 27 - 48
activity.action_feedback(feedback='Zizisse Done !')
@users('admin', 'emp')
@@ -187,7 +187,7 @@ class TestAdvMailPerformance(TransactionCase):
record.write({'name': 'Dupe write'})
with self.assertQueryCount(margin=1, admin=39, emp=60): # test_mail only: 39 - 60
with self.assertQueryCount(margin=1, admin=29, emp=50): # com runbot: 27 - 49 // test_mail only: 29 - 50
record.action_close('Dupe feedback')
self.assertEqual(record.activity_ids, self.env['mail.activity'])
@@ -199,7 +199,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=64, emp=82): # com runbot: 63 - 81 // test_mail only: 64 - 82
with self.assertQueryCount(margin=1, admin=60, emp=78): # com runbot: 59 - 77 // test_mail only: 60 - 78
record.write({
'user_id': self.user_test.id,
})
@@ -209,7 +209,7 @@ class TestAdvMailPerformance(TransactionCase):
def test_message_assignation_inbox(self):
record = self.env['mail.test.track'].create({'name': 'Test'})
with self.assertQueryCount(margin=1, admin=43, emp=54): # test_mail only: 43 - 54
with self.assertQueryCount(margin=1, admin=37, emp=48): # test_mail only: 37 - 48
record.write({
'user_id': self.user_test.id,
})
@@ -229,7 +229,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=22): # com runbot: 13 - 20 // test_mail only: 15 - 22
with self.assertQueryCount(admin=11, emp=18): # com runbot: 7 - 13 // test_mail only: 11 - 18
record.message_post(
body='<p>Test message_post as log</p>',
subtype='mail.mt_note',
@@ -240,7 +240,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=22): # com runbot: 13 - 20 // test_mail only: 15 - 22
with self.assertQueryCount(admin=11, emp=18): # com runbot: 7 - 13 // test_mail only: 11 - 18
record.message_post(
body='<p>Test Post Performances basic</p>',
partner_ids=[],
@@ -253,7 +253,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=61, emp=79): # com runbot: 54 - 72 // test_mail only: 61 - 79
with self.assertQueryCount(margin=1, admin=55, emp=74): # com runbot: 48 - 67 // test_mail only: 55 - 74
record.message_post(
body='<p>Test Post Performances with an email ping</p>',
partner_ids=self.customer.ids,
@@ -265,7 +265,7 @@ class TestAdvMailPerformance(TransactionCase):
def test_message_post_one_inbox_notification(self):
record = self.env['mail.test.simple'].create({'name': 'Test'})
with self.assertQueryCount(margin=1, admin=39, emp=48): # com runbot 37 - 46 // test_mail only: 39 - 48
with self.assertQueryCount(margin=1, admin=32, emp=45): # com runbot 30 - 41 // test_mail only: 32 - 45
record.message_post(
body='<p>Test Post Performances with an inbox ping</p>',
partner_ids=self.user_test.partner_id.ids,
@@ -389,7 +389,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=97, emp=120): # com runbot 90 - 113 // test_mail only: 97 - 120
with self.assertQueryCount(admin=90, emp=112): # com runbot 83 - 105 // test_mail only: 90 - 112
record.message_post(
body='<p>Test Post Performances</p>',
message_type='comment',
@@ -406,7 +406,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=116, emp=151): # com runbot 109 - 144 // test_mail only: 116 - 151
with self.assertQueryCount(admin=109, emp=143): # com runbot 102 - 136 // test_mail only: 109 - 143
record.message_post_with_template(template_id, message_type='comment', composition_mode='comment')
self.assertEqual(record.message_ids[0].body, '<p>Adding stuff on %s</p>' % record.name)
@@ -476,7 +476,7 @@ class TestHeavyMailPerformance(TransactionCase):
})
self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id)
with self.assertQueryCount(admin=66, emp=84): # com runbot: 65 - 83 // test_mail only: 66 - 84
with self.assertQueryCount(admin=62, emp=80): # com runbot: 61 - 79 // test_mail only: 62 - 80
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)
@@ -499,7 +499,7 @@ class TestHeavyMailPerformance(TransactionCase):
customer_id = self.customer.id
user_id = self.user_portal.id
with self.assertQueryCount(margin=1, admin=175, emp=211): # com runbot: 173 - 209 // test_mail only: 175 - 211
with self.assertQueryCount(margin=1, admin=165, emp=200): # com runbot: 163 - 198 // test_mail only: 165 - 200
rec = self.env['mail.test.full'].create({
'name': 'Test',
'umbrella_id': umbrella_id,
@@ -528,7 +528,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=105, emp=124): # com runbot: 104 - 123 // test_mail only: 105 - 124
with self.assertQueryCount(margin=1, admin=100, emp=118): # com runbot: 99 - 117 // test_mail only: 100 - 118
rec.write({
'name': 'Test2',
'umbrella_id': self.umbrella.id,
@@ -566,7 +566,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=111, emp=134): # test_mail only: 111 - 134
with self.assertQueryCount(margin=1, admin=106, emp=127): # test_mail only: 106 - 127
rec.write({
'name': 'Test2',
'umbrella_id': umbrella_id,