From f0e69bfe2d4c3877d27048f458906026677aece1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 13 Nov 2014 17:07:05 +0000 Subject: [PATCH] Add appltOtUpdate end point (sans acceptance tests for now) --- .../app/coffee/AuthorizationManager.coffee | 5 +- .../app/coffee/DocumentUpdaterManager.coffee | 16 ++++- .../app/coffee/WebsocketController.coffee | 24 +++++++ .../acceptance/coffee/ApplyUpdateTests.coffee | 12 ++++ .../coffee/AuthorizationManagerTests.coffee | 25 +++++++ .../coffee/DocumentUpdaterManagerTests.coffee | 43 ++++++++++++ .../coffee/WebsocketControllerTests.coffee | 70 ++++++++++++++++++- 7 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee diff --git a/services/real-time/app/coffee/AuthorizationManager.coffee b/services/real-time/app/coffee/AuthorizationManager.coffee index 345bfa4804..273e9697cf 100644 --- a/services/real-time/app/coffee/AuthorizationManager.coffee +++ b/services/real-time/app/coffee/AuthorizationManager.coffee @@ -1,7 +1,10 @@ module.exports = AuthorizationManager = assertClientCanViewProject: (client, callback = (error) ->) -> AuthorizationManager._assertClientHasPrivilegeLevel client, ["readOnly", "readAndWrite", "owner"], callback - + + assertClientCanEditProject: (client, callback = (error) ->) -> + AuthorizationManager._assertClientHasPrivilegeLevel client, ["readAndWrite", "owner"], callback + _assertClientHasPrivilegeLevel: (client, allowedLevels, callback = (error) ->) -> client.get "privilege_level", (error, privilegeLevel) -> return callback(error) if error? diff --git a/services/real-time/app/coffee/DocumentUpdaterManager.coffee b/services/real-time/app/coffee/DocumentUpdaterManager.coffee index 54ba6f9072..2af68eef0c 100644 --- a/services/real-time/app/coffee/DocumentUpdaterManager.coffee +++ b/services/real-time/app/coffee/DocumentUpdaterManager.coffee @@ -2,6 +2,9 @@ request = require "request" logger = require "logger-sharelatex" settings = require "settings-sharelatex" +redis = require("redis-sharelatex") +rclient = redis.createClient(settings.redis.web) + module.exports = DocumentUpdaterManager = getDocument: (project_id, doc_id, fromVersion, callback = (error, exists, doclines, version) ->) -> #timer = new metrics.Timer("get-document") @@ -23,4 +26,15 @@ module.exports = DocumentUpdaterManager = err = new Error("doc updater returned a non-success status code: #{res.statusCode}") err.statusCode = res.statusCode logger.error {err, project_id, doc_id, url}, "doc updater returned a non-success status code: #{res.statusCode}" - callback err \ No newline at end of file + callback err + + queueChange: (project_id, doc_id, change, callback = ()->)-> + jsonChange = JSON.stringify change + doc_key = "#{project_id}:#{doc_id}" + multi = rclient.multi() + multi.rpush "PendingUpdates:#{doc_id}", jsonChange + multi.sadd "DocsWithPendingUpdates", doc_key + multi.rpush "pending-updates-list", doc_key + multi.exec (error) -> + return callback(error) if error? + callback() \ 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 a115023ea5..0bdc0fa019 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -109,3 +109,27 @@ module.exports = WebsocketController = ConnectedUsersManager.getConnectedUsers project_id, (error, users) -> return callback(error) if error? callback null, users + + applyOtUpdate: (client, project_id, doc_id, update, callback = (error) ->) -> + AuthorizationManager.assertClientCanEditProject client, (error) -> + if error? + logger.error {err: error, project_id, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update" + client.disconnect() + return callback(error) + + Utils.getClientAttributes client, ["user_id"], (error, {user_id}) -> + return callback(error) if error? + update.meta ||= {} + update.meta.source = client.id + update.meta.user_id = user_id + # metrics.inc "editor.doc-update", 0.3 + # metrics.set "editor.active-projects", project_id, 0.3 + # metrics.set "editor.active-users", user_id, 0.3 + + logger.log {user_id, doc_id, project_id, client_id: client.id, version: update.v}, "sending update to doc updater" + + DocumentUpdaterManager.queueChange project_id, doc_id, update, (error) -> + if error? + logger.error {err: error, project_id, doc_id, client_id: client.id, version: update.v}, "document was not available for update" + client.disconnect() + callback(error) diff --git a/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee new file mode 100644 index 0000000000..284117b4d1 --- /dev/null +++ b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee @@ -0,0 +1,12 @@ +describe "applyOtUpdate", -> + describe "when authorized", -> + it "should publish a message on redis" + + it "should add the doc to the pending updates set in redis" + + it "should push the update into redis" + + describe "when not authorized", -> + it "should return an error" + + it "should disconnect the client" \ No newline at end of file diff --git a/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee b/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee index cd3822538b..a4741ab4d8 100644 --- a/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee +++ b/services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee @@ -35,5 +35,30 @@ describe 'AuthorizationManager', -> it "should return an error with any other privilegeLevel", (done) -> @client.params.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" + @AuthorizationManager.assertClientCanEditProject @client, (error) -> + error.message.should.equal "not authorized" + done() + + it "should allow the readAndWrite privilegeLevel", (done) -> + @client.params.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" + @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" + @AuthorizationManager.assertClientCanEditProject @client, (error) -> error.message.should.equal "not authorized" done() \ No newline at end of file diff --git a/services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee b/services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee index 9f80981374..80caa45802 100644 --- a/services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee +++ b/services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee @@ -12,11 +12,14 @@ describe 'DocumentUpdaterManager', -> @version = 42 @settings = apis: documentupdater: url: "http://doc-updater.example.com" + redis: web: {} + @rclient = {auth:->} @DocumentUpdaterManager = SandboxedModule.require modulePath, requires: 'settings-sharelatex':@settings 'logger-sharelatex': @logger = {log: sinon.stub(), error: sinon.stub()} 'request': @request = {} + 'redis-sharelatex' : createClient: () => @rclient describe "getDocument", -> beforeEach -> @@ -58,3 +61,43 @@ describe 'DocumentUpdaterManager', -> @callback .calledWith(err) .should.equal true + + describe 'queueChange', -> + beforeEach -> + @change = { + "action":"removeText", + "range":{"start":{"row":2,"column":2},"end":{"row":2,"column":3}}, + "text":"e" + } + @rclient.multi = sinon.stub().returns @rclient + @rclient.exec = sinon.stub().callsArg(0) + @rclient.rpush = sinon.stub() + @rclient.sadd = sinon.stub() + @callback = sinon.stub() + + describe "successfully", -> + beforeEach -> + @DocumentUpdaterManager.queueChange(@project_id, @doc_id, @change, @callback) + + it "should push the change", -> + @rclient.rpush + .calledWith("PendingUpdates:#{@doc_id}", JSON.stringify(@change)) + .should.equal true + + it "should notify the doc updater of the change via the pending-updates-list queue", -> + @rclient.rpush + .calledWith("pending-updates-list", "#{@project_id}:#{@doc_id}") + .should.equal true + + it "should push the doc id into the pending updates set", -> + @rclient.sadd + .calledWith("DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}") + .should.equal true + + describe "with error connecting to redis during exec", -> + beforeEach -> + @rclient.exec = sinon.stub().callsArgWith(0, new Error("something went wrong")) + @DocumentUpdaterManager.queueChange(@project_id, @doc_id, @change, @callback) + + it "should return an error", -> + @callback.calledWithExactly(sinon.match(Error)).should.equal true diff --git a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee index e0be68fc50..3e2a73b6c3 100644 --- a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee +++ b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee @@ -10,7 +10,7 @@ describe 'WebsocketController', -> tk.freeze(new Date()) @project_id = "project-id-123" @user = { - _id: "user-id-123" + _id: @user_id = "user-id-123" first_name: "James" last_name: "Allen" email: "james@example.com" @@ -19,6 +19,7 @@ describe 'WebsocketController', -> } @callback = sinon.stub() @client = + id: @client_id = "mock-client-id-123" params: {} set: sinon.stub() get: (param, cb) -> cb null, @params[param] @@ -286,4 +287,69 @@ describe 'WebsocketController', -> it "should not send cursor data to the connected user manager", (done)-> @ConnectedUsersManager.updateUserPosition.called.should.equal false - done() \ No newline at end of file + done() + + describe "applyOtUpdate", -> + beforeEach -> + @update = {op: {p: 12, t: "foo"}} + @client.params.user_id = @user_id + @AuthorizationManager.assertClientCanEditProject = sinon.stub().callsArg(1) + @DocumentUpdaterManager.queueChange = sinon.stub().callsArg(3) + + describe "succesfully", -> + beforeEach -> + @WebsocketController.applyOtUpdate @client, @project_id, @doc_id, @update, @callback + + it "should set the source of the update to the client id", -> + @update.meta.source.should.equal @client.id + + it "should set the user_id of the update to the user id", -> + @update.meta.user_id.should.equal @user_id + + it "should queue the update", -> + @DocumentUpdaterManager.queueChange + .calledWith(@project_id, @doc_id, @update) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + # it "should update the active users metric", -> + # @metrics.set.calledWith("editor.active-users", @user_id).should.equal true + # + # it "should update the active projects metric", -> + # @metrics.set.calledWith("editor.active-projects", @project_id).should.equal true + # + # it "should increment the doc updates", -> + # @metrics.inc.calledWith("editor.doc-update").should.equal true + + describe "unsuccessfully", -> + beforeEach -> + @client.disconnect = sinon.stub() + @DocumentUpdaterManager.queueChange = sinon.stub().callsArgWith(3, @error = new Error("Something went wrong")) + @WebsocketController.applyOtUpdate @client, @project_id, @doc_id, @update, @callback + + it "should disconnect the client", -> + @client.disconnect.called.should.equal true + + it "should log an error", -> + @logger.error.called.should.equal true + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true + + describe "when not authorized", -> + beforeEach -> + @client.disconnect = sinon.stub() + @AuthorizationManager.assertClientCanEditProject = sinon.stub().callsArgWith(1, @error = new Error("not authorized")) + @WebsocketController.applyOtUpdate @client, @project_id, @doc_id, @update, @callback + + it "should disconnect the client", -> + @client.disconnect.called.should.equal true + + it "should log an error", -> + @logger.error.called.should.equal true + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true + \ No newline at end of file