From 98e3ca4e671dbf45a4db9e730a46e65497e46df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Tue, 12 Feb 2019 14:20:50 +0000 Subject: [PATCH] [FIX] mail: throttle batched `channel_fetched` RPCs Revision on https://github.com/odoo/odoo/commit/844fb2b51ea7cfa557b19466c0a53dde19433e7a The commit above added the seen icon on messages in a channel. However, since this feature, there was a performance issue when opening the systray messaging menu with several pinned channels: lots of RPCs are made, almost 1 for each preview in the messaging menu. The RPCs comes from the reception of the last message in the channel, to let the current user notify that he has received the messages in the channel. Those RPCs were made for each channel, thus the more pinned channels the more RPCs were made when opening the messaging menu. This commit fixes the issue by throttling and batching the RPCs. To do so, instead of letting the channel model handle the RPC `channel_fetched`, the mail service now handles it. This has the advantage to throttle the RPC globally and to batch the requests to notify that messages have been received on channels. closes odoo/odoo#31026 --- addons/mail/models/mail_channel.py | 27 +++++++------- .../static/src/js/models/threads/channel.js | 35 +----------------- .../threads/mixins/channel_seen_mixin.js | 36 ++++++------------- .../static/src/js/services/mail_manager.js | 31 ++++++++++++++++ 4 files changed, 56 insertions(+), 73 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 2b5bfe2a2db..180a995a68f 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -745,20 +745,21 @@ class Channel(models.Model): @api.multi def channel_fetched(self): """ Broadcast the channel_fetched notification to channel members + :param channel_ids : list of channel id that has been fetched by current user """ - self.ensure_one() - if not self.channel_message_ids.ids: - return - last_message_id = self.channel_message_ids.ids[0] # zero is the index of the last message - self.env['mail.channel.partner'].search([('channel_id', 'in', self.ids), ('partner_id', '=', self.env.user.partner_id.id)]).write({ - 'fetched_message_id': last_message_id, - }) - data = { - 'info': 'channel_fetched', - 'last_message_id': last_message_id, - 'partner_id': self.env.user.partner_id.id, - } - self.env['bus.bus'].sendmany([[(self._cr.dbname, 'mail.channel', self.id), data]]) + for channel in self: + if not channel.channel_message_ids.ids: + return + last_message_id = channel.channel_message_ids.ids[0] # zero is the index of the last message + self.env['mail.channel.partner'].search([('channel_id', '=', channel.id), ('partner_id', '=', self.env.user.partner_id.id)]).write({ + 'fetched_message_id': last_message_id, + }) + data = { + 'info': 'channel_fetched', + 'last_message_id': last_message_id, + 'partner_id': self.env.user.partner_id.id, + } + self.env['bus.bus'].sendmany([[(self._cr.dbname, 'mail.channel', channel.id), data]]) @api.multi def channel_invite(self, partner_ids): diff --git a/addons/mail/static/src/js/models/threads/channel.js b/addons/mail/static/src/js/models/threads/channel.js index 7db3474ebb2..a2316edb7be 100644 --- a/addons/mail/static/src/js/models/threads/channel.js +++ b/addons/mail/static/src/js/models/threads/channel.js @@ -387,23 +387,9 @@ var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, { */ _markAsRead: function () { var superDef = this._super.apply(this, arguments); - var seenDef = this._throttleNotifySeen(); + var seenDef = this._notifySeen(); return $.when(superDef, seenDef); }, - /** - * @override {mail.model.ThreadSeenMixin} - * @private - * @returns {$.Promise} - */ - _notifyFetched: function () { - return this._rpc({ - model: 'mail.channel', - method: 'channel_fetched', - args: [[this._id]], - }, { - shadow: true - }); - }, /** * @override {mail.model.ThreadTypingMixin} * @private @@ -419,25 +405,6 @@ var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, { kwargs: { is_typing: params.typing }, }, { shadow: true }); }, - /** - * @override {mail.model.ThreadSeenMixin} - * @private - * @returns {$.Promise} resolved with ID of last seen message - */ - _notifySeen: function () { - var self = this; - this._cancelThrottledNotifyFetched(); - return this._rpc({ - model: 'mail.channel', - method: 'channel_seen', - args: [[this._id]], - }, { - shadow: true - }).then(function (lastSeenMessageID) { - self._lastSeenMessageID = lastSeenMessageID; - return lastSeenMessageID; - }); - }, /** * Prepare and send a message to the server on this channel. * diff --git a/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js b/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js index 8a59db70aa2..022994673d2 100644 --- a/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js +++ b/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js @@ -1,8 +1,6 @@ odoo.define('mail.model.ChannelSeenMixin', function (require) { "use strict"; -var CCThrottleFunction = require('mail.model.CCThrottleFunction'); - var session = require('web.session'); /** @@ -25,12 +23,6 @@ var ChannelSeenMixin = { init: function (params) { this._seenPartnersInfo = params.data.seen_partners_info || []; - this._throttleNotifySeen = _.throttle(this._notifySeen.bind(this), 3000); - this._throttleNotifyFetched = CCThrottleFunction({ - duration: 3000, - func: this._notifyFetched.bind(this), - }); - this.on('message_added', this, this._onSeenMessageAdded); }, @@ -225,27 +217,19 @@ var ChannelSeenMixin = { //-------------------------------------------------------------------------- /** - * @private - */ - _cancelThrottledNotifyFetched: function () { - this._throttleNotifyFetched.cancel(); - this._throttleNotifyFetched.clear(); - }, - /** - * @abstract - * @private - * @returns {$.Promise} - */ - _notifyFetched: function () { - return $.when(); - }, - /** - * @abstract * @private * @returns {$.Promise} resolved with ID of last seen message */ _notifySeen: function () { - return $.when(); + var self = this; + return this._rpc({ + model: 'mail.channel', + method: 'channel_seen', + args: [[this._id]], + }, { shadow: true }).then(function (lastSeenMessageID) { + self._lastSeenMessageID = lastSeenMessageID; + return lastSeenMessageID; + }); }, //-------------------------------------------------------------------------- @@ -256,7 +240,7 @@ var ChannelSeenMixin = { * @private */ _onSeenMessageAdded: function () { - this._throttleNotifyFetched(); + this.call('mail_service', 'notifyChannelFetched', { channelID: this._id }); }, }; diff --git a/addons/mail/static/src/js/services/mail_manager.js b/addons/mail/static/src/js/services/mail_manager.js index 6a49cdf0183..2234ce29af7 100644 --- a/addons/mail/static/src/js/services/mail_manager.js +++ b/addons/mail/static/src/js/services/mail_manager.js @@ -52,6 +52,12 @@ var MailManager = AbstractService.extend({ */ start: function () { this._super.apply(this, arguments); + + this._throttleNotifyChannelFetched = _.throttle(this._notifyChannelFetched.bind(this), 3000); + // List of channel that have requested the service to notify when + // it has been fetched, so that throttled RPCs batch the ids. + this._toNotifyChannelFetchedIDs = []; + this._initializeInternalState(); this._listenOnBuses(); this._fetchMailStateFromServer(); @@ -337,6 +343,14 @@ var MailManager = AbstractService.extend({ return $.when(); } }, + /** + * @param {Object} data + * @param {integer} data.channelID + */ + notifyChannelFetched: function (data) { + this._toNotifyChannelFetchedIDs = _.uniq(this._toNotifyChannelFetchedIDs.concat(data.channelID)); + this._throttleNotifyChannelFetched(); + }, /** * @param {integer|string} threadID */ @@ -920,6 +934,23 @@ var MailManager = AbstractService.extend({ _makeMessage: function (data) { return new Message(this, data); }, + /** + * @private + * @returns {$.Promise} + */ + _notifyChannelFetched: function () { + var channelIDs = this._toNotifyChannelFetchedIDs; + this._toNotifyChannelFetchedIDs = []; + if (_.isEmpty(channelIDs)) { + // no channel to notify fetched + return $.when(); + } + return this._rpc({ + model: 'mail.channel', + method: 'channel_fetched', + args: [channelIDs], + }, { shadow: true }); + }, /** * shows a popup to notify a new received message. * This will also rename the browser tab if this is not the active tab.