From bea0e9beb7d2af48c9757aaa4b399077cb51d164 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 29 Jun 2020 11:57:16 +0100 Subject: [PATCH 1/4] [misc] RoomManagerTests: explicitly pass a cb to joinProject/joinDoc --- services/real-time/test/unit/js/RoomManagerTests.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/real-time/test/unit/js/RoomManagerTests.js b/services/real-time/test/unit/js/RoomManagerTests.js index 3aee509af5..a202720ec7 100644 --- a/services/real-time/test/unit/js/RoomManagerTests.js +++ b/services/real-time/test/unit/js/RoomManagerTests.js @@ -113,7 +113,7 @@ describe('RoomManager', function () { }) return describe('when there are other clients in the project room', function () { - beforeEach(function () { + beforeEach(function (done) { this.RoomManager._clientsInRoom .withArgs(this.client, this.project_id) .onFirstCall() @@ -121,7 +121,7 @@ describe('RoomManager', function () { .onSecondCall() .returns(124) this.client.join = sinon.stub() - return this.RoomManager.joinProject(this.client, this.project_id) + this.RoomManager.joinProject(this.client, this.project_id, done) }) it('should join the room using the id', function () { @@ -174,7 +174,7 @@ describe('RoomManager', function () { }) return describe('when there are other clients in the doc room', function () { - beforeEach(function () { + beforeEach(function (done) { this.RoomManager._clientsInRoom .withArgs(this.client, this.doc_id) .onFirstCall() @@ -182,7 +182,7 @@ describe('RoomManager', function () { .onSecondCall() .returns(124) this.client.join = sinon.stub() - return this.RoomManager.joinDoc(this.client, this.doc_id) + this.RoomManager.joinDoc(this.client, this.doc_id, done) }) it('should join the room using the id', function () { From 4d01c02946c9f8372b822af54c53bef29f084fa6 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 29 Jun 2020 17:15:29 +0100 Subject: [PATCH 2/4] [misc] WebsocketControllerTests: explicitly pass a cb to leaveProject ...and fix the stubbing of `io.sockets.clients`. We were running the assertions prior to the actual completion of the leaveProject. Which in turn shadowed the broken test setup -- `io.sockets.clients()` was returning the `this.clientsInRoom=[]` from a previous context. --- .../test/unit/js/WebsocketControllerTests.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/services/real-time/test/unit/js/WebsocketControllerTests.js b/services/real-time/test/unit/js/WebsocketControllerTests.js index 8d8a39bb74..3cf4100951 100644 --- a/services/real-time/test/unit/js/WebsocketControllerTests.js +++ b/services/real-time/test/unit/js/WebsocketControllerTests.js @@ -42,6 +42,7 @@ describe('WebsocketController', function () { leave: sinon.stub() } return (this.WebsocketController = SandboxedModule.require(modulePath, { + globals: { console }, requires: { './WebApiManager': (this.WebApiManager = {}), './AuthorizationManager': (this.AuthorizationManager = {}), @@ -393,9 +394,19 @@ describe('WebsocketController', function () { }) describe('when the project is not empty', function () { - beforeEach(function () { + beforeEach(function (done) { this.clientsInRoom = ['mock-remaining-client'] - return this.WebsocketController.leaveProject(this.io, this.client) + this.io = { + sockets: { + clients: (room_id) => { + if (room_id !== this.project_id) { + throw 'expected room_id to be project_id' + } + return this.clientsInRoom + } + } + } + return this.WebsocketController.leaveProject(this.io, this.client, done) }) return it('should not flush the project in the document updater', function () { From a77222470bd3f4502700ba780fd18ccfcc7e9d53 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 29 Jun 2020 18:11:33 +0100 Subject: [PATCH 3/4] [misc] WebsocketControllerTests: always pass cb to updateClientPosition ...and fix the inconsistent async behaviour for logged-in vs anonymous users in the app. --- .../real-time/app/js/WebsocketController.js | 3 +- .../test/unit/js/WebsocketControllerTests.js | 63 ++++++++++++------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/services/real-time/app/js/WebsocketController.js b/services/real-time/app/js/WebsocketController.js index 92632cc154..451c514336 100644 --- a/services/real-time/app/js/WebsocketController.js +++ b/services/real-time/app/js/WebsocketController.js @@ -380,7 +380,8 @@ module.exports = WebsocketController = { // Don't store anonymous users in redis to avoid influx if (!user_id || user_id === 'anonymous-user') { cursorData.name = '' - callback() + // consistent async behaviour + setTimeout(callback) } else { cursorData.name = first_name && last_name diff --git a/services/real-time/test/unit/js/WebsocketControllerTests.js b/services/real-time/test/unit/js/WebsocketControllerTests.js index 3cf4100951..b6792a1cc2 100644 --- a/services/real-time/test/unit/js/WebsocketControllerTests.js +++ b/services/real-time/test/unit/js/WebsocketControllerTests.js @@ -78,7 +78,9 @@ describe('WebsocketController', function () { } } this.privilegeLevel = 'owner' - this.ConnectedUsersManager.updateUserPosition = sinon.stub().callsArg(4) + this.ConnectedUsersManager.updateUserPosition = sinon + .stub() + .callsArgAsync(4) this.isRestrictedUser = true this.WebApiManager.joinProject = sinon .stub() @@ -213,7 +215,9 @@ describe('WebsocketController', function () { } } this.privilegeLevel = 'owner' - this.ConnectedUsersManager.updateUserPosition = sinon.stub().callsArg(4) + this.ConnectedUsersManager.updateUserPosition = sinon + .stub() + .callsArgAsync(4) this.isRestrictedUser = true this.WebApiManager.joinProject = sinon .stub() @@ -964,7 +968,7 @@ describe('WebsocketController', function () { this.WebsocketLoadBalancer.emitToRoom = sinon.stub() this.ConnectedUsersManager.updateUserPosition = sinon .stub() - .callsArgWith(4) + .callsArgAsync(4) this.AuthorizationManager.assertClientCanViewProjectAndDoc = sinon .stub() .callsArgWith(2, null) @@ -976,7 +980,7 @@ describe('WebsocketController', function () { }) describe('with a logged in user', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.ol_context = { project_id: this.project_id, first_name: (this.first_name = 'Douglas'), @@ -984,9 +988,8 @@ describe('WebsocketController', function () { email: (this.email = 'joe@example.com'), user_id: (this.user_id = 'user-id-123') } - this.WebsocketController.updateClientPosition(this.client, this.update) - return (this.populatedCursorData = { + this.populatedCursorData = { doc_id: this.doc_id, id: this.client.publicId, name: `${this.first_name} ${this.last_name}`, @@ -994,7 +997,12 @@ describe('WebsocketController', function () { column: this.column, email: this.email, user_id: this.user_id - }) + } + this.WebsocketController.updateClientPosition( + this.client, + this.update, + done + ) }) it("should send the update to the project room with the user's name", function () { @@ -1036,7 +1044,7 @@ describe('WebsocketController', function () { }) describe('with a logged in user who has no last_name set', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.ol_context = { project_id: this.project_id, first_name: (this.first_name = 'Douglas'), @@ -1044,9 +1052,8 @@ describe('WebsocketController', function () { email: (this.email = 'joe@example.com'), user_id: (this.user_id = 'user-id-123') } - this.WebsocketController.updateClientPosition(this.client, this.update) - return (this.populatedCursorData = { + this.populatedCursorData = { doc_id: this.doc_id, id: this.client.publicId, name: `${this.first_name}`, @@ -1054,7 +1061,12 @@ describe('WebsocketController', function () { column: this.column, email: this.email, user_id: this.user_id - }) + } + this.WebsocketController.updateClientPosition( + this.client, + this.update, + done + ) }) it("should send the update to the project room with the user's name", function () { @@ -1096,7 +1108,7 @@ describe('WebsocketController', function () { }) describe('with a logged in user who has no first_name set', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.ol_context = { project_id: this.project_id, first_name: undefined, @@ -1104,9 +1116,8 @@ describe('WebsocketController', function () { email: (this.email = 'joe@example.com'), user_id: (this.user_id = 'user-id-123') } - this.WebsocketController.updateClientPosition(this.client, this.update) - return (this.populatedCursorData = { + this.populatedCursorData = { doc_id: this.doc_id, id: this.client.publicId, name: `${this.last_name}`, @@ -1114,7 +1125,12 @@ describe('WebsocketController', function () { column: this.column, email: this.email, user_id: this.user_id - }) + } + this.WebsocketController.updateClientPosition( + this.client, + this.update, + done + ) }) it("should send the update to the project room with the user's name", function () { @@ -1155,7 +1171,7 @@ describe('WebsocketController', function () { }) }) describe('with a logged in user who has no names set', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.ol_context = { project_id: this.project_id, first_name: undefined, @@ -1165,7 +1181,8 @@ describe('WebsocketController', function () { } return this.WebsocketController.updateClientPosition( this.client, - this.update + this.update, + done ) }) @@ -1185,13 +1202,14 @@ describe('WebsocketController', function () { }) describe('with an anonymous user', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.ol_context = { project_id: this.project_id } return this.WebsocketController.updateClientPosition( this.client, - this.update + this.update, + done ) }) @@ -1214,13 +1232,16 @@ describe('WebsocketController', function () { }) return describe('when the client has disconnected', function () { - beforeEach(function () { + beforeEach(function (done) { this.client.disconnected = true this.AuthorizationManager.assertClientCanViewProjectAndDoc = sinon.stub() return this.WebsocketController.updateClientPosition( this.client, this.update, - this.callback + (...args) => { + this.callback(...args) + done(args[0]) + } ) }) From 8601084a2eb89f4863272db116f8ef5febb80e11 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 1 Jul 2020 11:36:36 +0100 Subject: [PATCH 4/4] [misc] WebsocketControllerTests: remove debugging code --- services/real-time/test/unit/js/WebsocketControllerTests.js | 1 - 1 file changed, 1 deletion(-) diff --git a/services/real-time/test/unit/js/WebsocketControllerTests.js b/services/real-time/test/unit/js/WebsocketControllerTests.js index b6792a1cc2..7796ca7275 100644 --- a/services/real-time/test/unit/js/WebsocketControllerTests.js +++ b/services/real-time/test/unit/js/WebsocketControllerTests.js @@ -42,7 +42,6 @@ describe('WebsocketController', function () { leave: sinon.stub() } return (this.WebsocketController = SandboxedModule.require(modulePath, { - globals: { console }, requires: { './WebApiManager': (this.WebApiManager = {}), './AuthorizationManager': (this.AuthorizationManager = {}),