From 6d571e6d2367bbd68ecf788a2d9a2c34198cd99c Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 1 Nov 2017 19:16:49 +0000 Subject: [PATCH 1/3] version document renames --- services/document-updater/app.coffee | 1 + .../app/coffee/DocumentManager.coffee | 13 +++++ .../app/coffee/HttpController.coffee | 14 ++++- .../app/coffee/ProjectManager.coffee | 12 +++++ .../app/coffee/RedisManager.coffee | 16 ++++++ .../DocumentManagerTests.coffee | 18 +++++++ .../HttpController/HttpControllerTests.coffee | 37 +++++++++++++ .../ProjectManager/updateProjectTests.coffee | 54 +++++++++++++++++++ .../RedisManager/RedisManagerTests.coffee | 42 +++++++++++++++ 9 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 41cab59680..b4188292da 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -47,6 +47,7 @@ app.post '/project/:project_id/doc/:doc_id', HttpCont app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLoaded app.delete '/project/:project_id/doc/:doc_id', HttpController.flushAndDeleteDoc app.delete '/project/:project_id', HttpController.deleteProject +app.post '/project/:project_id', HttpController.updateProject app.post '/project/:project_id/flush', HttpController.flushProject app.post '/project/:project_id/doc/:doc_id/change/:change_id/accept', HttpController.acceptChanges app.post '/project/:project_id/doc/:doc_id/change/accept', HttpController.acceptChanges diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 5ddca2e6a8..c557db3f54 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -7,6 +7,7 @@ HistoryManager = require "./HistoryManager" RealTimeRedisManager = require "./RealTimeRedisManager" Errors = require "./Errors" RangesManager = require "./RangesManager" +async = require "async" MAX_UNFLUSHED_AGE = 300 * 1000 # 5 mins, document should be flushed to mongo this time after a change @@ -155,6 +156,14 @@ module.exports = DocumentManager = return callback(error) if error? callback() + renameDoc: (project_id, doc_id, user_id, update, _callback = (error) ->) -> + timer = new Metrics.Timer("docManager.updateProject") + callback = (args...) -> + timer.done() + _callback(args...) + + RedisManager.renameDoc project_id, doc_id, user_id, update, callback + getDocAndFlushIfOld: (project_id, doc_id, callback = (error, doc) ->) -> DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> return callback(error) if error? @@ -197,3 +206,7 @@ module.exports = DocumentManager = deleteCommentWithLock: (project_id, doc_id, thread_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.deleteComment, project_id, doc_id, thread_id, callback + + renameDocWithLock: (project_id, doc_id, user_id, update, callback = (error) ->) -> + UpdateManager = require "./UpdateManager" + UpdateManager.lockUpdatesAndDo DocumentManager.renameDoc, project_id, doc_id, user_id, update, callback diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 0c03a4f7bd..78de5fb765 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -141,7 +141,7 @@ module.exports = HttpController = return next(error) if error? logger.log {project_id, doc_id}, "accepted #{ change_ids.length } changes via http" res.send 204 # No Content - + deleteComment: (req, res, next = (error) ->) -> {project_id, doc_id, comment_id} = req.params logger.log {project_id, doc_id, comment_id}, "deleting comment via http" @@ -151,5 +151,15 @@ module.exports = HttpController = return next(error) if error? logger.log {project_id, doc_id, comment_id}, "deleted comment via http" res.send 204 # No Content - + updateProject: (req, res, next = (error) ->) -> + timer = new Metrics.Timer("http.updateProject") + project_id = req.params.project_id + {userId, docUpdates} = req.body + logger.log {project_id, docUpdates}, "updating project via http" + + ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, (error) -> + timer.done() + return next(error) if error? + logger.log project_id: project_id, "updated project via http" + res.send 204 # No Content diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index 26b6e79b0d..6b320e6e28 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -93,3 +93,15 @@ module.exports = ProjectManager = clearProjectState: (project_id, callback = (error) ->) -> RedisManager.clearProjectState project_id, callback + + updateProjectWithLocks: (project_id, user_id, updates, _callback = (error) ->) -> + timer = new Metrics.Timer("projectManager.updateProject") + callback = (args...) -> + timer.done() + _callback(args...) + + handleUpdate = (update, cb) -> + doc_id = update.id + DocumentManager.renameDocWithLock project_id, doc_id, user_id, update, cb + + async.each updates, handleUpdate, callback diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index d2ab35a3ee..cde2ccddc9 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -272,6 +272,22 @@ module.exports = RedisManager = else callback null, docUpdateCount + renameDoc: (project_id, doc_id, user_id, update, callback = (error) ->) -> + update = + doc: doc_id + pathname: update.pathname + new_pathname: update.newPathname + meta: + user_id: user_id + ts: new Date() + jsonUpdate = JSON.stringify(update) + + RedisManager.getDoc project_id, doc_id, (error, lines, version) -> + return callback(error) if error? + if lines? and version? + rclient.set keys.pathname(doc_id:doc_id), update.new_pathname + + rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), jsonUpdate, callback clearUnflushedTime: (doc_id, callback = (error) ->) -> rclient.del keys.unflushedTime(doc_id:doc_id), callback diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index ac0601b34b..6c7b051f7e 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -23,6 +23,7 @@ describe "DocumentManager", -> "./RangesManager": @RangesManager = {} @project_id = "project-id-123" @doc_id = "doc-id-123" + @user_id = 1234 @callback = sinon.stub() @lines = ["one", "two", "three"] @version = 42 @@ -439,3 +440,20 @@ describe "DocumentManager", -> it "should call the callback with the lines and versions", -> @callback.calledWith(null, @lines, @version).should.equal true + + describe "renameDoc", -> + beforeEach -> + @update = 'some-update' + @RedisManager.renameDoc = sinon.stub().yields() + + describe "successfully", -> + beforeEach -> + @DocumentManager.renameDoc @project_id, @doc_id, @user_id, @update, @callback + + it "should rename the document", -> + @RedisManager.renameDoc + .calledWith(@project_id, @doc_id, @user_id, @update) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index 17b5d10304..5ddefefaa3 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -492,3 +492,40 @@ describe "HttpController", -> @next .calledWith(new Error("oops")) .should.equal true + + describe "updateProject", -> + beforeEach -> + @userId = "user-id-123" + @docUpdates = sinon.stub() + @req = + body: {@userId, @docUpdates} + params: + project_id: @project_id + + describe "successfully", -> + beforeEach -> + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(3) + @HttpController.updateProject(@req, @res, @next) + + it "should accept the change", -> + @ProjectManager.updateProjectWithLocks + .calledWith(@project_id, @userId, @docUpdates) + .should.equal true + + it "should return a successful No Content response", -> + @res.send + .calledWith(204) + .should.equal true + + it "should time the request", -> + @Metrics.Timer::done.called.should.equal true + + describe "when an errors occurs", -> + beforeEach -> + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(3, new Error("oops")) + @HttpController.updateProject(@req, @res, @next) + + it "should call next with the error", -> + @next + .calledWith(new Error("oops")) + .should.equal true diff --git a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee new file mode 100644 index 0000000000..fc81834782 --- /dev/null +++ b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee @@ -0,0 +1,54 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +modulePath = "../../../../app/js/ProjectManager.js" +SandboxedModule = require('sandboxed-module') + +describe "ProjectManager", -> + beforeEach -> + @ProjectManager = SandboxedModule.require modulePath, requires: + "./RedisManager": @RedisManager = {} + "./DocumentManager": @DocumentManager = {} + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } + "./Metrics": @Metrics = + Timer: class Timer + done: sinon.stub() + + @project_id = "project-id-123" + @user_id = "user-id-123" + @callback = sinon.stub() + + describe "updateProjectWithLocks", -> + beforeEach -> + @firstUpdate = + id: 1 + update: 'foo' + @secondUpdate = + id: 2 + update: 'bar' + @updates = [ @firstUpdate, @secondUpdate ] + + describe "successfully", -> + beforeEach -> + @DocumentManager.renameDocWithLock = sinon.stub().yields() + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @updates, @callback + + it "should rename the documents in the updates", -> + @DocumentManager.renameDocWithLock + .calledWith(@project_id, @firstUpdate.id, @user_id, @firstUpdate) + .should.equal true + @DocumentManager.renameDocWithLock + .calledWith(@project_id, @secondUpdate.id, @user_id, @secondUpdate) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when renaming a doc fails", -> + beforeEach -> + @error = new Error('error') + @DocumentManager.renameDocWithLock = sinon.stub().yields(@error) + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @updates, @callback + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index f5bf3843fa..2b81c18a18 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -673,3 +673,45 @@ describe "RedisManager", -> @rclient.del .calledWith("ProjectState:#{@project_id}") .should.equal true + + describe "renameDoc", -> + beforeEach () -> + @rclient.rpush = sinon.stub().callsArg(2) + @rclient.set = sinon.stub() + @update = + id: @doc_id + pathname: @pathname = 'pathname' + newPathname: @newPathname = 'new-pathname' + + describe "the document is cached in redis", -> + beforeEach -> + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, 'lines', 'version') + @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @callback + + it "update the cached pathname", -> + @rclient.set + .calledWith("Pathname:#{@doc_id}", @newPathname) + .should.equal true + + it "should queue an update", -> + update = + doc: @doc_id + pathname: @pathname + new_pathname: @newPathname + meta: + user_id: @userId + ts: new Date() + @rclient.rpush + .calledWith("ProjectHistory:Ops:#{@project_id}", JSON.stringify(update)) + .should.equal true + + it "should call the callback", -> + @callback.calledWith().should.equal true + + describe "the document is not cached in redis", -> + beforeEach -> + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null) + @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @callback + + it "does not update the cached pathname", -> + @rclient.set.called.should.equal false From 7e86afe55e1bbd24d33035092f3519522159e794 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 6 Nov 2017 16:14:27 +0000 Subject: [PATCH 2/3] version file renames --- .../app/coffee/HttpController.coffee | 6 +-- .../app/coffee/ProjectManager.coffee | 12 ++++-- .../app/coffee/RedisManager.coffee | 25 +++++++++---- .../HttpController/HttpControllerTests.coffee | 9 +++-- .../ProjectManager/updateProjectTests.coffee | 37 ++++++++++++++----- .../RedisManager/RedisManagerTests.coffee | 30 +++++++++++++-- 6 files changed, 90 insertions(+), 29 deletions(-) diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 78de5fb765..38e82fb04e 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -155,10 +155,10 @@ module.exports = HttpController = updateProject: (req, res, next = (error) ->) -> timer = new Metrics.Timer("http.updateProject") project_id = req.params.project_id - {userId, docUpdates} = req.body - logger.log {project_id, docUpdates}, "updating project via http" + {userId, docUpdates, fileUpdates} = req.body + logger.log {project_id, docUpdates, fileUpdates}, "updating project via http" - ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, (error) -> + ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, fileUpdates, (error) -> timer.done() return next(error) if error? logger.log project_id: project_id, "updated project via http" diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index 6b320e6e28..2f85173b4c 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -94,14 +94,20 @@ module.exports = ProjectManager = clearProjectState: (project_id, callback = (error) ->) -> RedisManager.clearProjectState project_id, callback - updateProjectWithLocks: (project_id, user_id, updates, _callback = (error) ->) -> + updateProjectWithLocks: (project_id, user_id, docUpdates, fileUpdates, _callback = (error) ->) -> timer = new Metrics.Timer("projectManager.updateProject") callback = (args...) -> timer.done() _callback(args...) - handleUpdate = (update, cb) -> + handleDocUpdate = (update, cb) -> doc_id = update.id DocumentManager.renameDocWithLock project_id, doc_id, user_id, update, cb - async.each updates, handleUpdate, callback + handleFileUpdate = (update, cb) -> + file_id = update.id + RedisManager.renameFile project_id, file_id, user_id, update, cb + + async.each docUpdates, handleDocUpdate, (error) -> + return callback(error) if error? + async.each fileUpdates, handleFileUpdate, callback diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index cde2ccddc9..56ce0fb9f9 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -273,21 +273,32 @@ module.exports = RedisManager = callback null, docUpdateCount renameDoc: (project_id, doc_id, user_id, update, callback = (error) ->) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version) -> + return callback(error) if error? + + if lines? and version? + rclient.set keys.pathname(doc_id:doc_id), update.newPathname, (error) -> + return callback(error) if error? + RedisManager._renameEntity project_id, 'doc', doc_id, user_id, update, callback + else + RedisManager._renameEntity project_id, 'doc', doc_id, user_id, update, callback + + renameFile: (project_id, file_id, user_id, update, callback = (error) ->) -> + RedisManager._renameEntity project_id, 'file', file_id, user_id, update, callback + + _renameEntity: (project_id, entity_type, entity_id, user_id, update, callback = (error) ->) -> update = - doc: doc_id pathname: update.pathname new_pathname: update.newPathname meta: user_id: user_id ts: new Date() + update[entity_type] = entity_id + + logger.log {project_id, update}, "queue rename operation to project-history" jsonUpdate = JSON.stringify(update) - RedisManager.getDoc project_id, doc_id, (error, lines, version) -> - return callback(error) if error? - if lines? and version? - rclient.set keys.pathname(doc_id:doc_id), update.new_pathname - - rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), jsonUpdate, callback + rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), jsonUpdate, callback clearUnflushedTime: (doc_id, callback = (error) ->) -> rclient.del keys.unflushedTime(doc_id:doc_id), callback diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index 5ddefefaa3..b5ebe02339 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -497,19 +497,20 @@ describe "HttpController", -> beforeEach -> @userId = "user-id-123" @docUpdates = sinon.stub() + @fileUpdates = sinon.stub() @req = - body: {@userId, @docUpdates} + body: {@userId, @docUpdates, @fileUpdates} params: project_id: @project_id describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(3) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(4) @HttpController.updateProject(@req, @res, @next) it "should accept the change", -> @ProjectManager.updateProjectWithLocks - .calledWith(@project_id, @userId, @docUpdates) + .calledWith(@project_id, @userId, @docUpdates, @fileUpdates) .should.equal true it "should return a successful No Content response", -> @@ -522,7 +523,7 @@ describe "HttpController", -> describe "when an errors occurs", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(3, new Error("oops")) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(4, new Error("oops")) @HttpController.updateProject(@req, @res, @next) it "should call next with the error", -> diff --git a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee index fc81834782..7009405842 100644 --- a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee @@ -20,25 +20,35 @@ describe "ProjectManager", -> describe "updateProjectWithLocks", -> beforeEach -> - @firstUpdate = + @firstDocUpdate = id: 1 update: 'foo' - @secondUpdate = + @secondDocUpdate = id: 2 update: 'bar' - @updates = [ @firstUpdate, @secondUpdate ] + @docUpdates = [ @firstDocUpdate, @secondDocUpdate ] + @firstFileUpdate = + id: 2 + update: 'bar' + @fileUpdates = [ @firstFileUpdate ] + @DocumentManager.renameDocWithLock = sinon.stub().yields() + @RedisManager.renameFile = sinon.stub().yields() describe "successfully", -> beforeEach -> - @DocumentManager.renameDocWithLock = sinon.stub().yields() - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @updates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback - it "should rename the documents in the updates", -> + it "should rename the docs in the updates", -> @DocumentManager.renameDocWithLock - .calledWith(@project_id, @firstUpdate.id, @user_id, @firstUpdate) + .calledWith(@project_id, @firstDocUpdate.id, @user_id, @firstDocUpdate) .should.equal true @DocumentManager.renameDocWithLock - .calledWith(@project_id, @secondUpdate.id, @user_id, @secondUpdate) + .calledWith(@project_id, @secondDocUpdate.id, @user_id, @secondDocUpdate) + .should.equal true + + it "should rename the files in the updates", -> + @RedisManager.renameFile + .calledWith(@project_id, @firstFileUpdate.id, @user_id, @firstFileUpdate) .should.equal true it "should call the callback", -> @@ -48,7 +58,16 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @DocumentManager.renameDocWithLock = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @updates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true + + describe "when renaming a file fails", -> + beforeEach -> + @error = new Error('error') + @RedisManager.renameFile = sinon.stub().yields(@error) + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index 2b81c18a18..157c315b63 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -676,8 +676,8 @@ describe "RedisManager", -> describe "renameDoc", -> beforeEach () -> - @rclient.rpush = sinon.stub().callsArg(2) - @rclient.set = sinon.stub() + @rclient.rpush = sinon.stub().yields() + @rclient.set = sinon.stub().yields() @update = id: @doc_id pathname: @pathname = 'pathname' @@ -695,12 +695,12 @@ describe "RedisManager", -> it "should queue an update", -> update = - doc: @doc_id pathname: @pathname new_pathname: @newPathname meta: user_id: @userId ts: new Date() + doc: @doc_id @rclient.rpush .calledWith("ProjectHistory:Ops:#{@project_id}", JSON.stringify(update)) .should.equal true @@ -715,3 +715,27 @@ describe "RedisManager", -> it "does not update the cached pathname", -> @rclient.set.called.should.equal false + + describe "renameFile", -> + beforeEach () -> + @rclient.rpush = sinon.stub().yields() + @file_id = 1234 + + @update = + pathname: @pathname = '/old' + newPathname: @newPathname = '/new' + + @RedisManager.renameFile @project_id, @file_id, @userId, @update + + it "should queue an update", -> + update = + pathname: @pathname + new_pathname: @newPathname + meta: + user_id: @userId + ts: new Date() + file: @file_id + + @rclient.rpush + .calledWith("ProjectHistory:Ops:#{@project_id}", JSON.stringify(update)) + .should.equal true From 944e633bac4301f116014c4dafbeed22da607abd Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 6 Nov 2017 17:18:28 +0000 Subject: [PATCH 3/3] add acceptance test for entity renaming --- .../app/coffee/DocumentManager.coffee | 10 +- .../app/coffee/HttpController.coffee | 3 +- ...lyingUpdatesToProjectStructureTests.coffee | 98 +++++++++++++++++++ .../coffee/helpers/DocUpdaterClient.coffee | 7 ++ .../DocumentManagerTests.coffee | 8 +- .../HttpController/HttpControllerTests.coffee | 4 +- 6 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index c557db3f54..8e69989d09 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -31,20 +31,20 @@ module.exports = DocumentManager = else callback null, lines, version, ranges, pathname, unflushedTime, true - getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps, ranges) ->) -> + getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, ops, ranges, pathname) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") callback = (args...) -> timer.done() _callback(args...) - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> return callback(error) if error? if fromVersion == -1 - callback null, lines, version, [], ranges + callback null, lines, version, [], ranges, pathname else RedisManager.getPreviousDocOps doc_id, fromVersion, version, (error, ops) -> return callback(error) if error? - callback null, lines, version, ops, ranges + callback null, lines, version, ops, ranges, pathname setDoc: (project_id, doc_id, newLines, source, user_id, undoing, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") @@ -179,7 +179,7 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDoc, project_id, doc_id, callback - getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version) ->) -> + getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version, ops, ranges, pathname) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 38e82fb04e..ef9f860552 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -18,7 +18,7 @@ module.exports = HttpController = else fromVersion = -1 - DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops, ranges) -> + DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops, ranges, pathname) -> timer.done() return next(error) if error? logger.log project_id: project_id, doc_id: doc_id, "got doc via http" @@ -30,6 +30,7 @@ module.exports = HttpController = version: version ops: ops ranges: ranges + pathname: pathname _getTotalSizeOfLines: (lines) -> size = 0 diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee new file mode 100644 index 0000000000..21657793a8 --- /dev/null +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee @@ -0,0 +1,98 @@ +sinon = require "sinon" +chai = require("chai") +chai.should() +Settings = require('settings-sharelatex') +rclient_history = require("redis-sharelatex").createClient(Settings.redis.history) +ProjectHistoryKeys = Settings.redis.project_history.key_schema + +MockWebApi = require "./helpers/MockWebApi" +DocUpdaterClient = require "./helpers/DocUpdaterClient" + +describe "Applying updates to a project's structure", -> + before -> + @user_id = 'user-id-123' + + describe "renaming a file", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + @fileUpdate = + id: DocUpdaterClient.randomId() + pathname: '/file-path' + newPathname: '/new-file-path' + @fileUpdates = [ @fileUpdate ] + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, [], @fileUpdates, (error) -> + throw error if error? + setTimeout done, 200 + + it "should push the applied file renames to the project history changes api", (done) -> + rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) => + throw error if error? + + update = JSON.parse(updates[0]) + update.file.should.equal @fileUpdate.id + update.pathname.should.equal '/file-path' + update.new_pathname.should.equal '/new-file-path' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + + done() + + describe "renaming a document", -> + before -> + @docUpdate = + id: DocUpdaterClient.randomId() + pathname: '/doc-path' + newPathname: '/new-doc-path' + @docUpdates = [ @docUpdate ] + + describe "when the document is not loaded", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], (error) -> + throw error if error? + setTimeout done, 200 + + it "should push the applied doc renames to the project history changes api", (done) -> + rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) => + throw error if error? + + update = JSON.parse(updates[0]) + update.doc.should.equal @docUpdate.id + update.pathname.should.equal '/doc-path' + update.new_pathname.should.equal '/new-doc-path' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + + done() + + describe "when the document is loaded", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + MockWebApi.insertDoc @project_id, @docUpdate.id, {} + DocUpdaterClient.preloadDoc @project_id, @docUpdate.id, (error) => + throw error if error? + sinon.spy MockWebApi, "getDocument" + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], (error) -> + throw error if error? + setTimeout done, 200 + + after -> + MockWebApi.getDocument.restore() + + it "should update the doc", (done) -> + DocUpdaterClient.getDoc @project_id, @docUpdate.id, (error, res, doc) => + doc.pathname.should.equal @docUpdate.newPathname + done() + + it "should push the applied doc renames to the project history changes api", (done) -> + rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) => + throw error if error? + + update = JSON.parse(updates[0]) + update.doc.should.equal @docUpdate.id + update.pathname.should.equal '/doc-path' + update.new_pathname.should.equal '/new-doc-path' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + + done() diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index 6b2a5ac2fb..f70271021b 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -86,3 +86,10 @@ module.exports = DocUpdaterClient = if body? and res.statusCode >= 200 and res.statusCode < 300 body = JSON.parse(body) callback error, res, body + + sendProjectUpdate: (project_id, userId, docUpdates, fileUpdates, callback = (error) ->) -> + request.post { + url: "http://localhost:3003/project/#{project_id}" + json: { userId, docUpdates, fileUpdates } + }, (error, res, body) -> + callback error, res, body diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 6c7b051f7e..702617f7ae 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -109,7 +109,7 @@ describe "DocumentManager", -> describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback @@ -124,14 +124,14 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ops, @ranges).should.equal true + @callback.calledWith(null, @lines, @version, @ops, @ranges, @pathname).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true describe "with no previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback @@ -144,7 +144,7 @@ describe "DocumentManager", -> @RedisManager.getPreviousDocOps.called.should.equal false it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, [], @ranges).should.equal true + @callback.calledWith(null, @lines, @version, [], @ranges, @pathname).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index b5ebe02339..d52956635d 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -28,6 +28,7 @@ describe "HttpController", -> @version = 42 @fromVersion = 42 @ranges = { changes: "mock", comments: "mock" } + @pathname = '/a/b/c' @req = params: project_id: @project_id @@ -35,7 +36,7 @@ describe "HttpController", -> describe "when the document exists and no recent ops are requested", -> beforeEach -> - @DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [], @ranges) + @DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [], @ranges, @pathname) @HttpController.getDoc(@req, @res, @next) it "should get the doc", -> @@ -51,6 +52,7 @@ describe "HttpController", -> version: @version ops: [] ranges: @ranges + pathname: @pathname })) .should.equal true