From 5282f8f5314c8ab4309c844ea6d34e4ec95dff09 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 24 Feb 2020 13:32:20 +0000 Subject: [PATCH] [misc] synchronous client store using an Object at .ol_context --- .../app/coffee/AuthorizationManager.coffee | 27 +++--- .../app/coffee/HttpController.coffee | 8 +- services/real-time/app/coffee/Router.coffee | 17 ++-- services/real-time/app/coffee/Utils.coffee | 14 --- .../app/coffee/WebsocketController.coffee | 61 ++++++------- .../app/coffee/WebsocketLoadBalancer.coffee | 4 +- .../coffee/AuthorizationManagerTests.coffee | 38 ++++---- .../coffee/WebsocketControllerTests.coffee | 86 ++++++++----------- .../coffee/WebsocketLoadBalancerTests.coffee | 28 +++--- .../unit/coffee/helpers/MockClient.coffee | 7 +- 10 files changed, 113 insertions(+), 177 deletions(-) delete mode 100644 services/real-time/app/coffee/Utils.coffee diff --git a/services/real-time/app/coffee/AuthorizationManager.coffee b/services/real-time/app/coffee/AuthorizationManager.coffee index b4cf854238..50d76537ce 100644 --- a/services/real-time/app/coffee/AuthorizationManager.coffee +++ b/services/real-time/app/coffee/AuthorizationManager.coffee @@ -6,13 +6,10 @@ module.exports = AuthorizationManager = AuthorizationManager._assertClientHasPrivilegeLevel client, ["readAndWrite", "owner"], callback _assertClientHasPrivilegeLevel: (client, allowedLevels, callback = (error) ->) -> - client.get "privilege_level", (error, privilegeLevel) -> - return callback(error) if error? - allowed = (privilegeLevel in allowedLevels) - if allowed - callback null - else - callback new Error("not authorized") + if client.ol_context["privilege_level"] in allowedLevels + callback null + else + callback new Error("not authorized") assertClientCanViewProjectAndDoc: (client, doc_id, callback = (error) ->) -> AuthorizationManager.assertClientCanViewProject client, (error) -> @@ -25,15 +22,15 @@ module.exports = AuthorizationManager = AuthorizationManager._assertClientCanAccessDoc client, doc_id, callback _assertClientCanAccessDoc: (client, doc_id, callback = (error) ->) -> - client.get "doc:#{doc_id}", (error, status) -> - return callback(error) if error? - if status? and status is "allowed" - callback null - else - callback new Error("not authorized") + if client.ol_context["doc:#{doc_id}"] is "allowed" + callback null + else + callback new Error("not authorized") addAccessToDoc: (client, doc_id, callback = (error) ->) -> - client.set("doc:#{doc_id}", "allowed", callback) + client.ol_context["doc:#{doc_id}"] = "allowed" + callback(null) removeAccessToDoc: (client, doc_id, callback = (error) ->) -> - client.del("doc:#{doc_id}", callback) + delete client.ol_context["doc:#{doc_id}"] + callback(null) diff --git a/services/real-time/app/coffee/HttpController.coffee b/services/real-time/app/coffee/HttpController.coffee index ae92a9e299..1fc74e8c16 100644 --- a/services/real-time/app/coffee/HttpController.coffee +++ b/services/real-time/app/coffee/HttpController.coffee @@ -1,4 +1,3 @@ -Utils = require "./Utils" async = require "async" module.exports = HttpController = @@ -8,11 +7,8 @@ module.exports = HttpController = # and for checking internal state in acceptance tests. The acceptances tests # should provide appropriate coverage. _getConnectedClientView: (ioClient, callback = (error, client) ->) -> - client_id = ioClient.id - Utils.getClientAttributes ioClient, [ - "project_id", "user_id", "first_name", "last_name", "email", "connected_time" - ], (error, {project_id, user_id, first_name, last_name, email, connected_time}) -> - return callback(error) if error? + client_id = ioClient.id + {project_id, user_id, first_name, last_name, email, connected_time} = ioClient.ol_context client = {client_id, project_id, user_id, first_name, last_name, email, connected_time} client.rooms = [] for name, joined of ioClient.manager.roomClients[client_id] diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 49166b8de1..f89e1d9eaf 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -4,7 +4,6 @@ settings = require "settings-sharelatex" WebsocketController = require "./WebsocketController" HttpController = require "./HttpController" HttpApiController = require "./HttpApiController" -Utils = require "./Utils" bodyParser = require "body-parser" base64id = require("base64id") @@ -16,10 +15,9 @@ httpAuth = basicAuth (user, pass)-> return isValid module.exports = Router = - _handleError: (callback = ((error) ->), error, client, method, extraAttrs = {}) -> - Utils.getClientAttributes client, ["project_id", "doc_id", "user_id"], (_, attrs) -> - for key, value of extraAttrs - attrs[key] = value + _handleError: (callback = ((error) ->), error, client, method, attrs = {}) -> + for key in ["project_id", "doc_id", "user_id"] + attrs[key] = client.ol_context[key] attrs.client_id = client.id attrs.err = error if error.name == "CodedError" @@ -57,6 +55,8 @@ module.exports = Router = app.post "/client/:client_id/disconnect", httpAuth, HttpApiController.disconnectClient session.on 'connection', (error, client, session) -> + client.ol_context = {} + client?.on "error", (err) -> logger.err { clientErr: err }, "socket.io client error" if client.connected @@ -112,9 +112,14 @@ module.exports = Router = client.on "disconnect", () -> metrics.inc('socket-io.disconnect') metrics.gauge('socket-io.clients', io.sockets.clients()?.length - 1) + + cleanup = () -> + delete client.ol_context WebsocketController.leaveProject io, client, (err) -> if err? - Router._handleError null, err, client, "leaveProject" + Router._handleError cleanup, err, client, "leaveProject" + else + cleanup() # Variadic. The possible arguments: # doc_id, callback diff --git a/services/real-time/app/coffee/Utils.coffee b/services/real-time/app/coffee/Utils.coffee deleted file mode 100644 index 72dada30a3..0000000000 --- a/services/real-time/app/coffee/Utils.coffee +++ /dev/null @@ -1,14 +0,0 @@ -async = require "async" - -module.exports = Utils = - getClientAttributes: (client, keys, callback = (error, attributes) ->) -> - attributes = {} - jobs = keys.map (key) -> - (callback) -> - client.get key, (error, value) -> - return callback(error) if error? - attributes[key] = value - callback() - async.series jobs, (error) -> - return callback(error) if error? - callback null, attributes \ No newline at end of file diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index cdcf42a147..3ca030ca0c 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -7,7 +7,6 @@ DocumentUpdaterManager = require "./DocumentUpdaterManager" ConnectedUsersManager = require "./ConnectedUsersManager" WebsocketLoadBalancer = require "./WebsocketLoadBalancer" RoomManager = require "./RoomManager" -Utils = require "./Utils" module.exports = WebsocketController = # If the protocol version changes when the client reconnects, @@ -34,17 +33,17 @@ module.exports = WebsocketController = logger.warn {err, project_id, user_id, client_id: client.id}, "user is not authorized to join project" return callback(err) - client.set("privilege_level", privilegeLevel) - client.set("user_id", user_id) - client.set("project_id", project_id) - client.set("owner_id", project?.owner?._id) - client.set("first_name", user?.first_name) - client.set("last_name", user?.last_name) - client.set("email", user?.email) - client.set("connected_time", new Date()) - client.set("signup_date", user?.signUpDate) - client.set("login_count", user?.loginCount) - client.set("is_restricted_user", !!(isRestrictedUser)) + client.ol_context["privilege_level"] = privilegeLevel + client.ol_context["user_id"] = user_id + client.ol_context["project_id"] = project_id + client.ol_context["owner_id"] = project?.owner?._id + client.ol_context["first_name"] = user?.first_name + client.ol_context["last_name"] = user?.last_name + client.ol_context["email"] = user?.email + client.ol_context["connected_time"] = new Date() + client.ol_context["signup_date"] = user?.signUpDate + client.ol_context["login_count"] = user?.loginCount + client.ol_context["is_restricted_user"] = !!(isRestrictedUser) RoomManager.joinProject client, project_id, (err) -> return callback(err) if err @@ -59,8 +58,7 @@ module.exports = WebsocketController = # is determined by FLUSH_IF_EMPTY_DELAY. FLUSH_IF_EMPTY_DELAY: 500 #ms leaveProject: (io, client, callback = (error) ->) -> - Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> - return callback(error) if error? + {project_id, user_id} = client.ol_context return callback() unless project_id # client did not join project metrics.inc "editor.leave-project" @@ -84,13 +82,12 @@ module.exports = WebsocketController = , WebsocketController.FLUSH_IF_EMPTY_DELAY joinDoc: (client, doc_id, fromVersion = -1, options, callback = (error, doclines, version, ops, ranges) ->) -> - if client.disconnected + if client.disconnected metrics.inc('editor.join-doc.disconnected', 1, {status: 'immediately'}) return callback() - metrics.inc "editor.join-doc" - Utils.getClientAttributes client, ["project_id", "user_id", "is_restricted_user"], (error, {project_id, user_id, is_restricted_user}) -> - return callback(error) if error? + metrics.inc "editor.join-doc" + {project_id, user_id, is_restricted_user} = client.ol_context return callback(new Error("no project_id found on client")) if !project_id? logger.log {user_id, project_id, doc_id, fromVersion, client_id: client.id}, "client joining doc" @@ -142,9 +139,9 @@ module.exports = WebsocketController = callback null, escapedLines, version, ops, ranges leaveDoc: (client, doc_id, callback = (error) ->) -> - # client may have disconnected, but we have to cleanup internal state. - metrics.inc "editor.leave-doc" - Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> + # client may have disconnected, but we have to cleanup internal state. + metrics.inc "editor.leave-doc" + {project_id, user_id} = client.ol_context logger.log {user_id, project_id, doc_id, client_id: client.id}, "client leaving doc" RoomManager.leaveDoc(client, doc_id) # we could remove permission when user leaves a doc, but because @@ -153,15 +150,12 @@ module.exports = WebsocketController = ## AuthorizationManager.removeAccessToDoc client, doc_id callback() updateClientPosition: (client, cursorData, callback = (error) ->) -> - if client.disconnected + if client.disconnected # do not create a ghost entry in redis return callback() - metrics.inc "editor.update-client-position", 0.1 - Utils.getClientAttributes client, [ - "project_id", "first_name", "last_name", "email", "user_id" - ], (error, {project_id, first_name, last_name, email, user_id}) -> - return callback(error) if error? + metrics.inc "editor.update-client-position", 0.1 + {project_id, first_name, last_name, email, user_id} = client.ol_context logger.log {user_id, project_id, client_id: client.id, cursorData: cursorData}, "updating client position" AuthorizationManager.assertClientCanViewProjectAndDoc client, cursorData.doc_id, (error) -> @@ -198,14 +192,12 @@ module.exports = WebsocketController = CLIENT_REFRESH_DELAY: 1000 getConnectedUsers: (client, callback = (error, users) ->) -> - if client.disconnected + if client.disconnected # they are not interested anymore, skip the redis lookups return callback() - metrics.inc "editor.get-connected-users" - Utils.getClientAttributes client, ["project_id", "user_id", "is_restricted_user"], (error, clientAttributes) -> - return callback(error) if error? - {project_id, user_id, is_restricted_user} = clientAttributes + metrics.inc "editor.get-connected-users" + {project_id, user_id, is_restricted_user} = client.ol_context if is_restricted_user return callback(null, []) return callback(new Error("no project_id found on client")) if !project_id? @@ -221,9 +213,8 @@ module.exports = WebsocketController = , WebsocketController.CLIENT_REFRESH_DELAY applyOtUpdate: (client, doc_id, update, callback = (error) ->) -> - # client may have disconnected, but we can submit their update to doc-updater anyways. - Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) -> - return callback(error) if error? + # client may have disconnected, but we can submit their update to doc-updater anyways. + {user_id, project_id} = client.ol_context return callback(new Error("no project_id found on client")) if !project_id? WebsocketController._assertClientCanApplyUpdate client, doc_id, update, (error) -> diff --git a/services/real-time/app/coffee/WebsocketLoadBalancer.coffee b/services/real-time/app/coffee/WebsocketLoadBalancer.coffee index 9508ac8714..5a59ea504f 100644 --- a/services/real-time/app/coffee/WebsocketLoadBalancer.coffee +++ b/services/real-time/app/coffee/WebsocketLoadBalancer.coffee @@ -7,7 +7,6 @@ HealthCheckManager = require "./HealthCheckManager" RoomManager = require "./RoomManager" ChannelManager = require "./ChannelManager" ConnectedUsersManager = require "./ConnectedUsersManager" -Utils = require './Utils' Async = require 'async' RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST = [ @@ -94,8 +93,7 @@ module.exports = WebsocketLoadBalancer = Async.eachLimit clientList , 2 , (client, cb) -> - Utils.getClientAttributes client, ['is_restricted_user'], (err, {is_restricted_user}) -> - return cb(err) if err? + is_restricted_user = client.ol_context['is_restricted_user'] if !seen[client.id] seen[client.id] = true if !(is_restricted_user && message.message not in RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST) diff --git a/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee b/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee index 46b9e8be9a..143218d8b2 100644 --- a/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee +++ b/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee @@ -9,64 +9,56 @@ modulePath = '../../../app/js/AuthorizationManager' describe 'AuthorizationManager', -> beforeEach -> @client = - params: {} - get: (param, cb) -> - cb null, @params[param] - set: (param, value, cb) -> - @params[param] = value - cb() - del: (param, cb) -> - delete @params[param] - cb() + ol_context: {} @AuthorizationManager = SandboxedModule.require modulePath, requires: {} describe "assertClientCanViewProject", -> it "should allow the readOnly privilegeLevel", (done) -> - @client.params.privilege_level = "readOnly" + @client.ol_context.privilege_level = "readOnly" @AuthorizationManager.assertClientCanViewProject @client, (error) -> expect(error).to.be.null done() it "should allow the readAndWrite privilegeLevel", (done) -> - @client.params.privilege_level = "readAndWrite" + @client.ol_context.privilege_level = "readAndWrite" @AuthorizationManager.assertClientCanViewProject @client, (error) -> expect(error).to.be.null done() it "should allow the owner privilegeLevel", (done) -> - @client.params.privilege_level = "owner" + @client.ol_context.privilege_level = "owner" @AuthorizationManager.assertClientCanViewProject @client, (error) -> expect(error).to.be.null done() it "should return an error with any other privilegeLevel", (done) -> - @client.params.privilege_level = "unknown" + @client.ol_context.privilege_level = "unknown" @AuthorizationManager.assertClientCanViewProject @client, (error) -> error.message.should.equal "not authorized" done() describe "assertClientCanEditProject", -> it "should not allow the readOnly privilegeLevel", (done) -> - @client.params.privilege_level = "readOnly" + @client.ol_context.privilege_level = "readOnly" @AuthorizationManager.assertClientCanEditProject @client, (error) -> error.message.should.equal "not authorized" done() it "should allow the readAndWrite privilegeLevel", (done) -> - @client.params.privilege_level = "readAndWrite" + @client.ol_context.privilege_level = "readAndWrite" @AuthorizationManager.assertClientCanEditProject @client, (error) -> expect(error).to.be.null done() it "should allow the owner privilegeLevel", (done) -> - @client.params.privilege_level = "owner" + @client.ol_context.privilege_level = "owner" @AuthorizationManager.assertClientCanEditProject @client, (error) -> expect(error).to.be.null done() it "should return an error with any other privilegeLevel", (done) -> - @client.params.privilege_level = "unknown" + @client.ol_context.privilege_level = "unknown" @AuthorizationManager.assertClientCanEditProject @client, (error) -> error.message.should.equal "not authorized" done() @@ -77,11 +69,11 @@ describe 'AuthorizationManager', -> beforeEach () -> @doc_id = "12345" @callback = sinon.stub() - @client.params = {} + @client.ol_context = {} describe "when not authorised at the project level", -> beforeEach () -> - @client.params.privilege_level = "unknown" + @client.ol_context.privilege_level = "unknown" it "should not allow access", () -> @AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, (err) -> @@ -97,7 +89,7 @@ describe 'AuthorizationManager', -> describe "when authorised at the project level", -> beforeEach () -> - @client.params.privilege_level = "readOnly" + @client.ol_context.privilege_level = "readOnly" describe "and not authorised at the document level", -> it "should not allow access", () -> @@ -127,11 +119,11 @@ describe 'AuthorizationManager', -> beforeEach () -> @doc_id = "12345" @callback = sinon.stub() - @client.params = {} + @client.ol_context = {} describe "when not authorised at the project level", -> beforeEach () -> - @client.params.privilege_level = "readOnly" + @client.ol_context.privilege_level = "readOnly" it "should not allow access", () -> @AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, (err) -> @@ -147,7 +139,7 @@ describe 'AuthorizationManager', -> describe "when authorised at the project level", -> beforeEach () -> - @client.params.privilege_level = "readAndWrite" + @client.ol_context.privilege_level = "readAndWrite" describe "and not authorised at the document level", -> it "should not allow access", () -> diff --git a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee index 719a736f4a..c0047c49b7 100644 --- a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee +++ b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee @@ -23,9 +23,7 @@ describe 'WebsocketController', -> disconnected: false id: @client_id = "mock-client-id-123" publicId: "other-id-#{Math.random()}" - params: {} - set: sinon.stub() - get: (param, cb) -> cb null, @params[param] + ol_context: {} join: sinon.stub() leave: sinon.stub() @WebsocketController = SandboxedModule.require modulePath, requires: @@ -69,38 +67,27 @@ describe 'WebsocketController', -> @RoomManager.joinProject.calledWith(@client, @project_id).should.equal true it "should set the privilege level on the client", -> - @client.set.calledWith("privilege_level", @privilegeLevel).should.equal true - + @client.ol_context["privilege_level"].should.equal @privilegeLevel it "should set the user's id on the client", -> - @client.set.calledWith("user_id", @user._id).should.equal true - + @client.ol_context["user_id"].should.equal @user._id it "should set the user's email on the client", -> - @client.set.calledWith("email", @user.email).should.equal true - + @client.ol_context["email"].should.equal @user.email it "should set the user's first_name on the client", -> - @client.set.calledWith("first_name", @user.first_name).should.equal true - + @client.ol_context["first_name"].should.equal @user.first_name it "should set the user's last_name on the client", -> - @client.set.calledWith("last_name", @user.last_name).should.equal true - + @client.ol_context["last_name"].should.equal @user.last_name it "should set the user's sign up date on the client", -> - @client.set.calledWith("signup_date", @user.signUpDate).should.equal true - + @client.ol_context["signup_date"].should.equal @user.signUpDate it "should set the user's login_count on the client", -> - @client.set.calledWith("login_count", @user.loginCount).should.equal true - + @client.ol_context["login_count"].should.equal @user.loginCount it "should set the connected time on the client", -> - @client.set.calledWith("connected_time", new Date()).should.equal true - + @client.ol_context["connected_time"].should.equal new Date() it "should set the project_id on the client", -> - @client.set.calledWith("project_id", @project_id).should.equal true - + @client.ol_context["project_id"].should.equal @project_id it "should set the project owner id on the client", -> - @client.set.calledWith("owner_id", @owner_id).should.equal true - + @client.ol_context["owner_id"].should.equal @owner_id it "should set the is_restricted_user flag on the client", -> - @client.set.calledWith("is_restricted_user", @isRestrictedUser).should.equal true - + @client.ol_context["is_restricted_user"].should.equal @isRestrictedUser it "should call the callback with the project, privilegeLevel and protocolVersion", -> @callback .calledWith(null, @project, @privilegeLevel, @WebsocketController.PROTOCOL_VERSION) @@ -191,14 +178,14 @@ describe 'WebsocketController', -> if room_id != @project_id throw "expected room_id to be project_id" return @clientsInRoom - @client.params.project_id = @project_id - @client.params.user_id = @user_id + @client.ol_context.project_id = @project_id + @client.ol_context.user_id = @user_id @WebsocketController.FLUSH_IF_EMPTY_DELAY = 0 tk.reset() # Allow setTimeout to work. describe "when the client did not joined a project yet", -> beforeEach (done) -> - @client.params = {} + @client.ol_context = {} @WebsocketController.leaveProject @io, @client, done it "should bail out when calling leaveProject", () -> @@ -248,8 +235,8 @@ describe 'WebsocketController', -> describe "when client has not authenticated", -> beforeEach (done) -> - @client.params.user_id = null - @client.params.project_id = null + @client.ol_context.user_id = null + @client.ol_context.project_id = null @WebsocketController.leaveProject @io, @client, done it "should not end clientTracking.clientDisconnected to the project room", -> @@ -272,8 +259,8 @@ describe 'WebsocketController', -> describe "when client has not joined a project", -> beforeEach (done) -> - @client.params.user_id = @user_id - @client.params.project_id = null + @client.ol_context.user_id = @user_id + @client.ol_context.project_id = null @WebsocketController.leaveProject @io, @client, done it "should not end clientTracking.clientDisconnected to the project room", -> @@ -303,8 +290,8 @@ describe 'WebsocketController', -> @ranges = { "mock": "ranges" } @options = {} - @client.params.project_id = @project_id - @client.params.is_restricted_user = false + @client.ol_context.project_id = @project_id + @client.ol_context.is_restricted_user = false @AuthorizationManager.addAccessToDoc = sinon.stub() @AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null) @DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(3, null, @doc_lines, @version, @ranges, @ops) @@ -408,7 +395,7 @@ describe 'WebsocketController', -> describe "with a restricted client", -> beforeEach -> @ranges.comments = [{op: {a: 1}}, {op: {a: 2}}] - @client.params.is_restricted_user = true + @client.ol_context.is_restricted_user = true @WebsocketController.joinDoc @client, @doc_id, -1, @options, @callback it "should overwrite ranges.comments with an empty list", -> @@ -463,7 +450,7 @@ describe 'WebsocketController', -> describe "leaveDoc", -> beforeEach -> @doc_id = "doc-id-123" - @client.params.project_id = @project_id + @client.ol_context.project_id = @project_id @RoomManager.leaveDoc = sinon.stub() @WebsocketController.leaveDoc @client, @doc_id, @callback @@ -479,7 +466,7 @@ describe 'WebsocketController', -> describe "getConnectedUsers", -> beforeEach -> - @client.params.project_id = @project_id + @client.ol_context.project_id = @project_id @users = ["mock", "users"] @WebsocketLoadBalancer.emitToRoom = sinon.stub() @ConnectedUsersManager.getConnectedUsers = sinon.stub().callsArgWith(1, null, @users) @@ -527,7 +514,7 @@ describe 'WebsocketController', -> describe "when restricted user", -> beforeEach -> - @client.params.is_restricted_user = true + @client.ol_context.is_restricted_user = true @AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null) @WebsocketController.getConnectedUsers @client, @callback @@ -564,14 +551,13 @@ describe 'WebsocketController', -> describe "with a logged in user", -> beforeEach -> - @clientParams = { + @client.ol_context = { project_id: @project_id first_name: @first_name = "Douglas" last_name: @last_name = "Adams" email: @email = "joe@example.com" user_id: @user_id = "user-id-123" } - @client.get = (param, callback) => callback null, @clientParams[param] @WebsocketController.updateClientPosition @client, @update @populatedCursorData = @@ -604,14 +590,13 @@ describe 'WebsocketController', -> describe "with a logged in user who has no last_name set", -> beforeEach -> - @clientParams = { + @client.ol_context = { project_id: @project_id first_name: @first_name = "Douglas" last_name: undefined email: @email = "joe@example.com" user_id: @user_id = "user-id-123" } - @client.get = (param, callback) => callback null, @clientParams[param] @WebsocketController.updateClientPosition @client, @update @populatedCursorData = @@ -644,14 +629,13 @@ describe 'WebsocketController', -> describe "with a logged in user who has no first_name set", -> beforeEach -> - @clientParams = { + @client.ol_context = { project_id: @project_id first_name: undefined last_name: @last_name = "Adams" email: @email = "joe@example.com" user_id: @user_id = "user-id-123" } - @client.get = (param, callback) => callback null, @clientParams[param] @WebsocketController.updateClientPosition @client, @update @populatedCursorData = @@ -683,14 +667,13 @@ describe 'WebsocketController', -> @metrics.inc.calledWith("editor.update-client-position", 0.1).should.equal true describe "with a logged in user who has no names set", -> beforeEach -> - @clientParams = { + @client.ol_context = { project_id: @project_id first_name: undefined last_name: undefined email: @email = "joe@example.com" user_id: @user_id = "user-id-123" } - @client.get = (param, callback) => callback null, @clientParams[param] @WebsocketController.updateClientPosition @client, @update it "should send the update to the project name with no name", -> @@ -709,10 +692,9 @@ describe 'WebsocketController', -> describe "with an anonymous user", -> beforeEach -> - @clientParams = { + @client.ol_context = { project_id: @project_id } - @client.get = (param, callback) => callback null, @clientParams[param] @WebsocketController.updateClientPosition @client, @update it "should send the update to the project room with no name", -> @@ -745,8 +727,8 @@ describe 'WebsocketController', -> describe "applyOtUpdate", -> beforeEach -> @update = {op: {p: 12, t: "foo"}} - @client.params.user_id = @user_id - @client.params.project_id = @project_id + @client.ol_context.user_id = @user_id + @client.ol_context.project_id = @project_id @WebsocketController._assertClientCanApplyUpdate = sinon.stub().yields() @DocumentUpdaterManager.queueChange = sinon.stub().callsArg(3) @@ -807,8 +789,8 @@ describe 'WebsocketController', -> beforeEach (done) -> @client.disconnect = sinon.stub() @client.emit = sinon.stub() - @client.params.user_id = @user_id - @client.params.project_id = @project_id + @client.ol_context.user_id = @user_id + @client.ol_context.project_id = @project_id error = new Error("update is too large") error.updateSize = 7372835 @DocumentUpdaterManager.queueChange = sinon.stub().callsArgWith(3, error) diff --git a/services/real-time/test/unit/coffee/WebsocketLoadBalancerTests.coffee b/services/real-time/test/unit/coffee/WebsocketLoadBalancerTests.coffee index e6fe1df8c9..b2441cd6d0 100644 --- a/services/real-time/test/unit/coffee/WebsocketLoadBalancerTests.coffee +++ b/services/real-time/test/unit/coffee/WebsocketLoadBalancerTests.coffee @@ -18,12 +18,6 @@ describe "WebsocketLoadBalancer", -> "./RoomManager" : @RoomManager = {eventSource: sinon.stub().returns @RoomEvents} "./ChannelManager": @ChannelManager = {publish: sinon.stub()} "./ConnectedUsersManager": @ConnectedUsersManager = {refreshClient: sinon.stub()} - "./Utils": @Utils = { - getClientAttributes: sinon.spy( - (client, _attrs, callback) -> - callback(null, {is_restricted_user: !!client.__isRestricted}) - ) - } @io = {} @WebsocketLoadBalancer.rclientPubList = [{publish: sinon.stub()}] @WebsocketLoadBalancer.rclientSubList = [{ @@ -87,9 +81,9 @@ describe "WebsocketLoadBalancer", -> beforeEach -> @io.sockets = clients: sinon.stub().returns([ - {id: 'client-id-1', emit: @emit1 = sinon.stub()} - {id: 'client-id-2', emit: @emit2 = sinon.stub()} - {id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client + {id: 'client-id-1', emit: @emit1 = sinon.stub(), ol_context: {}} + {id: 'client-id-2', emit: @emit2 = sinon.stub(), ol_context: {}} + {id: 'client-id-1', emit: @emit3 = sinon.stub(), ol_context: {}} # duplicate client ]) data = JSON.stringify room_id: @room_id @@ -109,10 +103,10 @@ describe "WebsocketLoadBalancer", -> beforeEach -> @io.sockets = clients: sinon.stub().returns([ - {id: 'client-id-1', emit: @emit1 = sinon.stub()} - {id: 'client-id-2', emit: @emit2 = sinon.stub()} - {id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client - {id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true} + {id: 'client-id-1', emit: @emit1 = sinon.stub(), ol_context: {}} + {id: 'client-id-2', emit: @emit2 = sinon.stub(), ol_context: {}} + {id: 'client-id-1', emit: @emit3 = sinon.stub(), ol_context: {}} # duplicate client + {id: 'client-id-4', emit: @emit4 = sinon.stub(), ol_context: {is_restricted_user: true}} ]) data = JSON.stringify room_id: @room_id @@ -133,10 +127,10 @@ describe "WebsocketLoadBalancer", -> beforeEach -> @io.sockets = clients: sinon.stub().returns([ - {id: 'client-id-1', emit: @emit1 = sinon.stub()} - {id: 'client-id-2', emit: @emit2 = sinon.stub()} - {id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client - {id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true} + {id: 'client-id-1', emit: @emit1 = sinon.stub(), ol_context: {}} + {id: 'client-id-2', emit: @emit2 = sinon.stub(), ol_context: {}} + {id: 'client-id-1', emit: @emit3 = sinon.stub(), ol_context: {}} # duplicate client + {id: 'client-id-4', emit: @emit4 = sinon.stub(), ol_context: {is_restricted_user: true}} ]) data = JSON.stringify room_id: @room_id diff --git a/services/real-time/test/unit/coffee/helpers/MockClient.coffee b/services/real-time/test/unit/coffee/helpers/MockClient.coffee index 4dcddeb01d..497928132a 100644 --- a/services/real-time/test/unit/coffee/helpers/MockClient.coffee +++ b/services/real-time/test/unit/coffee/helpers/MockClient.coffee @@ -4,15 +4,10 @@ idCounter = 0 module.exports = class MockClient constructor: () -> - @attributes = {} + @ol_context = {} @join = sinon.stub() @emit = sinon.stub() @disconnect = sinon.stub() @id = idCounter++ @publicId = idCounter++ - set : (key, value, callback) -> - @attributes[key] = value - callback() if callback? - get : (key, callback) -> - callback null, @attributes[key] disconnect: () ->