From 26444fcaefa86f37f216cc334f9d7bda35ee58a2 Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Tue, 6 Sep 2022 08:28:34 +0000 Subject: [PATCH] [FIX] bus: keep last notification id during refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the websocket were introduced, the last notification id was persisted in the local storage in order to use it even after a page reload. The last notification id is now stored on the `SharedWorker` which means this information is lost if the last opened tab is reloaded. This commit fixes this issue by storing this information by the mean of the localStorage. Since the localStorage is not accessible from the worker global scope, the bus service will store this information and relay it to the worker when starting. The worker will now wait to receive the last notification id before subscribing. closes odoo/odoo#99621 Related: odoo/enterprise#31124 Signed-off-by: Sébastien Theys (seb) --- addons/bus/models/__init__.py | 1 + addons/bus/models/ir_http.py | 18 ++++++ addons/bus/static/src/services/bus_service.js | 16 ++++- .../static/src/workers/websocket_worker.js | 6 +- addons/bus/static/tests/bus_tests.js | 60 +++++++++++++++++++ .../bus/static/tests/helpers/mock_server.js | 6 +- .../static/tests/websocket_worker_tests.js | 2 +- 7 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 addons/bus/models/ir_http.py diff --git a/addons/bus/models/__init__.py b/addons/bus/models/__init__.py index f309e7d023b..d5caf4d02f6 100644 --- a/addons/bus/models/__init__.py +++ b/addons/bus/models/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from . import bus from . import bus_presence +from . import ir_http from . import ir_model from . import ir_websocket from . import res_users diff --git a/addons/bus/models/ir_http.py b/addons/bus/models/ir_http.py new file mode 100644 index 00000000000..8c369326d88 --- /dev/null +++ b/addons/bus/models/ir_http.py @@ -0,0 +1,18 @@ +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import models +from odoo.http import request + + +class IrHttp(models.AbstractModel): + _inherit = 'ir.http' + + def session_info(self): + result = super().session_info() + result['dbuuid'] = request.env['ir.config_parameter'].sudo().get_param('database.uuid') + return result + + def get_frontend_session_info(self): + result = super().get_frontend_session_info() + result['dbuuid'] = request.env['ir.config_parameter'].sudo().get_param('database.uuid') + return result diff --git a/addons/bus/static/src/services/bus_service.js b/addons/bus/static/src/services/bus_service.js index 4bc35e8ac38..c538972f715 100644 --- a/addons/bus/static/src/services/bus_service.js +++ b/addons/bus/static/src/services/bus_service.js @@ -4,6 +4,7 @@ import { WEBSOCKET_CLOSE_CODES } from "@bus/workers/websocket_worker"; import { browser } from "@web/core/browser/browser"; import { registry } from '@web/core/registry'; +import { session } from '@web/session'; const { EventBus } = owl; const NO_POPUP_CLOSE_CODES = [ @@ -23,9 +24,13 @@ const NO_POPUP_CLOSE_CODES = [ * @emits notification */ export const busService = { - dependencies: ['localization', 'notification'], + dependencies: ['localization', 'multi_tab', 'notification'], - start(env) { + start(env, { multi_tab: multiTab }) { + if (multiTab.getSharedValue('dbuuid') !== session.dbuuid) { + multiTab.setSharedValue('dbuuid', session.dbuuid); + multiTab.removeSharedValue('last_notification_id'); + } const bus = new EventBus(); const workerClass = 'SharedWorker' in window ? browser.SharedWorker : browser.Worker; const worker = new workerClass('/bus/websocket_worker_bundle', { @@ -58,7 +63,8 @@ export const busService = { * @param {{type: WorkerEvent, data: any}[]} messageEv.data */ function handleMessage(messageEv) { - const { type, data } = messageEv.data; + const { type } = messageEv.data; + let { data } = messageEv.data; // Do not trigger the connection lost pop up if the reconnecting // event is caused by a session expired/keep_alive_timeout. if (type === 'reconnecting' && !NO_POPUP_CLOSE_CODES.includes(data.closeCode)) { @@ -69,6 +75,9 @@ export const busService = { } else if (type === 'reconnect' && removeConnectionLostNotification) { removeConnectionLostNotification(); removeConnectionLostNotification = null; + } else if (type === 'notification') { + multiTab.setSharedValue('last_notification_id', data[data.length - 1].id); + data = data.map(notification => notification.message); } bus.trigger(type, data); } @@ -79,6 +88,7 @@ export const busService = { } else { worker.addEventListener('message', handleMessage); } + send('update_last_notification_id', multiTab.getSharedValue('last_notification_id', 0)); browser.addEventListener('unload', () => send('leave')); diff --git a/addons/bus/static/src/workers/websocket_worker.js b/addons/bus/static/src/workers/websocket_worker.js index f7a6e99ba8e..1e0cd83b95f 100644 --- a/addons/bus/static/src/workers/websocket_worker.js +++ b/addons/bus/static/src/workers/websocket_worker.js @@ -81,7 +81,6 @@ export class WebsocketWorker { this._onClientMessage(messagePort, ev.data); }; this.channelsByClient.set(messagePort, []); - this._updateChannels(); } /** @@ -122,6 +121,9 @@ export class WebsocketWorker { return this._deleteChannel(client, data); case 'force_update_channels': return this._forceUpdateChannels(); + case 'update_last_notification_id': + this.lastNotificationId = data; + this._updateChannels(); } } @@ -240,7 +242,7 @@ export class WebsocketWorker { _onWebsocketMessage(messageEv) { const notifications = JSON.parse(messageEv.data); this.lastNotificationId = notifications[notifications.length - 1].id; - this.broadcast('notification', notifications.map(notification => notification.message)); + this.broadcast('notification', notifications); } /** diff --git a/addons/bus/static/tests/bus_tests.js b/addons/bus/static/tests/bus_tests.js index 72d8f7609a8..9e20716f7e2 100644 --- a/addons/bus/static/tests/bus_tests.js +++ b/addons/bus/static/tests/bus_tests.js @@ -10,6 +10,7 @@ const { patchWebsocketWorkerWithCleanup } = require("@bus/../tests/helpers/mock_ const { browser } = require("@web/core/browser/browser"); const { registry } = require("@web/core/registry"); +const { session } = require('@web/session'); const { makeDeferred, nextTick, patchWithCleanup } = require("@web/../tests/helpers/utils"); const { makeTestEnv } = require('@web/../tests/helpers/mock_env'); const { createWebClient } = require("@web/../tests/webclient/helpers"); @@ -355,6 +356,65 @@ QUnit.module('Bus', { await nextTick(); assert.containsNone(document.body, '.o_notification'); }); + + QUnit.test('Last notification id is passed to the worker on service start', async function (assert) { + const pyEnv = await startServer(); + let updateLastNotificationDeferred = makeDeferred(); + patchWebsocketWorkerWithCleanup({ + _onClientMessage(_, { action, data }) { + assert.step(`${action} - ${data}`); + updateLastNotificationDeferred.resolve(); + }, + }); + await makeTestEnv(); + await updateLastNotificationDeferred; + // First bus service has never received notifications thus the + // default is 0. + assert.verifySteps(['update_last_notification_id - 0']); + + pyEnv['bus.bus']._sendmany([ + ['lambda', 'notifType', 'beta'], + ['lambda', 'notifType', 'beta'], + ]); + // let the bus service store the last notification id. + await nextTick(); + + updateLastNotificationDeferred = makeDeferred(); + await makeTestEnv(); + await updateLastNotificationDeferred; + // Second bus service sends the last known notification id. + assert.verifySteps([`update_last_notification_id - 1`]); + }); + + QUnit.test('Last notification id reset after db change', async function (assert) { + const pyEnv = await startServer(); + let updateLastNotificationDeferred = makeDeferred(); + patchWebsocketWorkerWithCleanup({ + _onClientMessage(_, { action, data }) { + assert.step(`${action} - ${data}`); + updateLastNotificationDeferred.resolve(); + }, + }); + await makeTestEnv(); + await updateLastNotificationDeferred; + // First bus service has never received notifications thus the + // default is 0. + assert.verifySteps(['update_last_notification_id - 0']); + + pyEnv['bus.bus']._sendmany([ + ['lambda', 'notifType', 'beta'], + ['lambda', 'notifType', 'beta'], + ]); + // let the bus service store the last notification id. + await nextTick(); + // dbuuid change should reset last notification id. + patchWithCleanup(session, { dbuuid: 'ABCDE-FGHIJ-KLMNO' }); + + updateLastNotificationDeferred = makeDeferred(); + await makeTestEnv(); + await updateLastNotificationDeferred; + assert.verifySteps([`update_last_notification_id - 0`]); + }); }); }); diff --git a/addons/bus/static/tests/helpers/mock_server.js b/addons/bus/static/tests/helpers/mock_server.js index 558665adfa1..43340b552b9 100644 --- a/addons/bus/static/tests/helpers/mock_server.js +++ b/addons/bus/static/tests/helpers/mock_server.js @@ -15,6 +15,7 @@ patch(MockServer.prototype, 'bus', { this.websocketWorker = patchWebsocketWorkerWithCleanup(); this.pendingLongpollingPromise = null; this.notificationsToBeResolved = []; + this.lastBusNotificationId = 0; }, //-------------------------------------------------------------------------- @@ -56,10 +57,13 @@ patch(MockServer.prototype, 'bus', { * @param {Array} notifications */ _mockBusBus__sendmany(notifications) { + if (!notifications.length) { + return; + } const values = []; for (const notification of notifications) { const [type, payload] = notification.slice(1, notification.length); - values.push({ payload, type }); + values.push({ id: this.lastBusNotificationId++, message: { payload, type }}); if (this.debug) { console.log("%c[bus]", "color: #c6e; font-weight: bold;", type, payload); } diff --git a/addons/bus/static/tests/websocket_worker_tests.js b/addons/bus/static/tests/websocket_worker_tests.js index 51ab185825f..deea4eb7b68 100644 --- a/addons/bus/static/tests/websocket_worker_tests.js +++ b/addons/bus/static/tests/websocket_worker_tests.js @@ -81,7 +81,7 @@ QUnit.test('notification event is broadcasted', async function (assert) { broadcast(type, message) { if (type === 'notification') { assert.step(`broadcast ${type}`); - assert.deepEqual(message, notifications.map(notif => notif.message)); + assert.deepEqual(message, notifications); } }, });