[FIX] bus: do not receive all longpolling notifs after disconnect
Revision on bus refactoring: https://github.com/odoo/odoo/commit/6448420c5dd160470e465dee7729d19d8d5e7bab Before this commit, when a user was disconnected for a very long time, he would receive lots of chat notifications on his next login. Here is an example of weird behaviour with this issue: - User folds and unfolds a chat window 50 times - User disconnects for more than 50 seconds - User reconnects ==> the chat window rapidly folds and unfolds itself 50 times! This issue comes from the fact that after 50 seconds without any longpolling, the web client ignores the last tracked notification and sents the ID `-1` to the server. The web client always provides a notification ID to the server on a `longpolling/poll`. Usually, the server returns all notifications of the user with an ID greater than the provided ID. There is an exception with ID `0`, in which all notifications since the last connection of the user are returned. The cause of the issue is a mismatch of the special notification ID between the server and the web client. For the web client, the ID `-1` is used for not tracking any notification, whereas the server uses the ID `0` for this case. As a result, when the server receives the value `-1`, it returns all notifications having an ID greater than `-1`. In other words, it returns all notifications related to this user, even the ones received long time ago! This commit fixes the issue by enforcing the special notification ID `0` on both the webclient and server. Note that this logic is similar to 11.0: - The webclient uses the ID `-1` most of the time to define 'untracked': https://github.com/odoo/odoo/blob/11.0/addons/bus/static/src/js/bus.js#L154 - However, it passes the ID `0` to the server: https://github.com/odoo/odoo/blob/11.0/addons/bus/static/src/js/bus.js#L193
This commit is contained in:
@@ -51,7 +51,7 @@ var CrossTabBus = Longpolling.extend({
|
||||
if (this._callLocalStorage('getItem', 'last_ts', 0) + 50000 < now) {
|
||||
this._callLocalStorage('removeItem', 'last');
|
||||
}
|
||||
this._lastNotificationID = this._callLocalStorage('getItem', 'last', -1);
|
||||
this._lastNotificationID = this._callLocalStorage('getItem', 'last', 0);
|
||||
this.call('local_storage', 'onStorage', this, this._onStorage);
|
||||
},
|
||||
destroy: function () {
|
||||
@@ -304,7 +304,7 @@ var CrossTabBus = Longpolling.extend({
|
||||
|
||||
// last notification id changed
|
||||
if (key === this._generateKey('last')) {
|
||||
this._lastNotificationID = value || -1;
|
||||
this._lastNotificationID = value | 0;
|
||||
}
|
||||
// notifications changed
|
||||
else if (key === this._generateKey('notification')) {
|
||||
|
||||
@@ -77,6 +77,60 @@ QUnit.module('Bus', {
|
||||
parent.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('provide notification ID of 0 by default', function (assert) {
|
||||
// This test is important in order to ensure that we provide the correct
|
||||
// sentinel value 0 when we are not aware of the last notification ID
|
||||
// that we have received. We cannot provide an ID of -1, otherwise it
|
||||
// may likely be handled incorrectly (before this test was written,
|
||||
// it was providing -1 to the server, which in return sent every stored
|
||||
// notifications related to this user).
|
||||
assert.expect(3);
|
||||
|
||||
// Simulate no ID of last notification in the local storage
|
||||
testUtils.patch(LocalStorageServiceMock, {
|
||||
getItem: function (key) {
|
||||
if (key === 'last_ts') {
|
||||
return 0;
|
||||
}
|
||||
return this._super.apply(this, arguments);
|
||||
},
|
||||
});
|
||||
|
||||
var pollDeferred = $.Deferred();
|
||||
var parent = new Widget();
|
||||
testUtils.addMockEnvironment(parent, {
|
||||
data: {},
|
||||
services: {
|
||||
bus_service: BusService,
|
||||
local_storage: LocalStorageServiceMock,
|
||||
},
|
||||
mockRPC: function (route, args) {
|
||||
if (route === '/longpolling/poll') {
|
||||
assert.step(route);
|
||||
assert.strictEqual(args.last, 0,
|
||||
"provided last notification ID should be 0");
|
||||
|
||||
pollDeferred = $.Deferred();
|
||||
pollDeferred.abort = (function () {
|
||||
this.reject({message: "XmlHttpRequestError abort"}, $.Event());
|
||||
}).bind(pollDeferred);
|
||||
return pollDeferred;
|
||||
}
|
||||
return this._super.apply(this, arguments);
|
||||
}
|
||||
});
|
||||
|
||||
var widget = new Widget(parent);
|
||||
widget.appendTo($('#qunit-fixture'));
|
||||
|
||||
// trigger longpolling poll RPC
|
||||
widget.call('bus_service', 'addChannel', 'lambda');
|
||||
assert.verifySteps(['/longpolling/poll']);
|
||||
|
||||
testUtils.unpatch(LocalStorageServiceMock);
|
||||
parent.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('cross tab bus share message from a channel', function (assert) {
|
||||
var done = assert.async();
|
||||
assert.expect(5);
|
||||
|
||||
Reference in New Issue
Block a user