From fef5f6b775de4f68bed3e4e8bc3698967bfb4e84 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 14 Nov 2014 10:12:35 +0000 Subject: [PATCH] Add acceptance tests for applyOtUpdate --- services/real-time/app/coffee/Router.coffee | 8 ++ .../app/coffee/WebsocketController.coffee | 11 +- .../acceptance/coffee/ApplyUpdateTests.coffee | 104 +++++++++++++++++- .../coffee/WebsocketControllerTests.coffee | 7 +- 4 files changed, 118 insertions(+), 12 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 43ec7976f0..7872727377 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -13,6 +13,7 @@ module.exports = Router = attrs.err = error logger.error attrs, "server side error in #{method}" # Don't return raw error to prevent leaking server side info + console.log "CALLING CALLBACK", callback return callback {message: "Something went wrong"} configure: (app, io, session) -> @@ -74,5 +75,12 @@ module.exports = Router = WebsocketController.updateClientPosition client, cursorData, (err) -> if err? Router._handleError callback, err, client, "clientTracking.updatePosition" + else + callback() + + client.on "applyOtUpdate", (doc_id, update, callback = (error) ->) -> + WebsocketController.applyOtUpdate client, doc_id, update, (err) -> + if err? + Router._handleError callback, err, client, "applyOtUpdate", {doc_id, update} else 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 0bdc0fa019..6f1b3b5119 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -110,14 +110,17 @@ module.exports = WebsocketController = return callback(error) if error? callback null, users - applyOtUpdate: (client, project_id, doc_id, update, callback = (error) ->) -> + applyOtUpdate: (client, 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() + logger.error {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update" + setTimeout () -> + # Disconnect, but give the client the chance to receive the error + client.disconnect() + , 100 return callback(error) - Utils.getClientAttributes client, ["user_id"], (error, {user_id}) -> + Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) -> return callback(error) if error? update.meta ||= {} update.meta.source = client.id diff --git a/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee index 284117b4d1..b5676909cb 100644 --- a/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee +++ b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee @@ -1,12 +1,106 @@ +async = require "async" +chai = require("chai") +expect = chai.expect +chai.should() + +RealTimeClient = require "./helpers/RealTimeClient" +FixturesManager = require "./helpers/FixturesManager" + +settings = require "settings-sharelatex" +redis = require "redis-sharelatex" +rclient = redis.createClient(settings.redis.web) + describe "applyOtUpdate", -> + before -> + @update = { + op: [{i: "foo", p: 42}] + } describe "when authorized", -> - it "should publish a message on redis" + before (done) -> + async.series [ + (cb) => + FixturesManager.setUpProject { + privilegeLevel: "readAndWrite" + }, (e, {@project_id, @user_id}) => + cb(e) + + (cb) => + FixturesManager.setUpDoc @project_id, {@lines, @version, @ops}, (e, {@doc_id}) => + cb(e) + + (cb) => + @client = RealTimeClient.connect() + @client.emit "joinProject", project_id: @project_id, cb + + (cb) => + @client.emit "joinDoc", @doc_id, cb + + (cb) => + @client.emit "applyOtUpdate", @doc_id, @update, cb + ], done - it "should add the doc to the pending updates set in redis" + it "should push the doc into the pending updates list", (done) -> + rclient.lrange "pending-updates-list", 0, -1, (error, [doc_id]) => + doc_id.should.equal "#{@project_id}:#{@doc_id}" + done() - it "should push the update into redis" + it "should add the doc to the pending updates set in redis", (done) -> + rclient.sismember "DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}", (error, isMember) => + isMember.should.equal 1 + done() + + it "should push the update into redis", (done) -> + rclient.lrange "PendingUpdates:#{@doc_id}", 0, -1, (error, [update]) => + update = JSON.parse(update) + update.op.should.deep.equal @update.op + update.meta.should.deep.equal { + source: @client.socket.sessionid + user_id: @user_id + } + done() + + after (done) -> + async.series [ + (cb) => rclient.del "pending-updates-list", cb + (cb) => rclient.del "DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}", cb + (cb) => rclient.del "PendingUpdates:#{@doc_id}", cb + ], done describe "when not authorized", -> - it "should return an error" + before (done) -> + async.series [ + (cb) => + FixturesManager.setUpProject { + privilegeLevel: "readOnly" + }, (e, {@project_id, @user_id}) => + cb(e) + + (cb) => + FixturesManager.setUpDoc @project_id, {@lines, @version, @ops}, (e, {@doc_id}) => + cb(e) + + (cb) => + @client = RealTimeClient.connect() + @client.emit "joinProject", project_id: @project_id, cb + + (cb) => + @client.emit "joinDoc", @doc_id, cb + + (cb) => + @client.emit "applyOtUpdate", @doc_id, @update, (@error) => + cb() + ], done - it "should disconnect the client" \ No newline at end of file + it "should return an error", -> + expect(@error).to.exist + + it "should disconnect the client", (done) -> + setTimeout () => + @client.socket.connected.should.equal false + done() + , 300 + + it "should not put the update in redis", (done) -> + rclient.llen "PendingUpdates:#{@doc_id}", (error, len) => + len.should.equal 0 + done() \ No newline at end of file diff --git a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee index 3e2a73b6c3..8ea62f466b 100644 --- a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee +++ b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee @@ -293,12 +293,13 @@ describe 'WebsocketController', -> beforeEach -> @update = {op: {p: 12, t: "foo"}} @client.params.user_id = @user_id + @client.params.project_id = @project_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 + @WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback it "should set the source of the update to the client id", -> @update.meta.source.should.equal @client.id @@ -327,7 +328,7 @@ describe 'WebsocketController', -> 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 + @WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback it "should disconnect the client", -> @client.disconnect.called.should.equal true @@ -342,7 +343,7 @@ describe 'WebsocketController', -> 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 + @WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback it "should disconnect the client", -> @client.disconnect.called.should.equal true