From 0642e3c8c9df79d3247f77b2555c5aff87e751c5 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 5 Mar 2018 12:14:47 +0000 Subject: [PATCH 1/6] support project version on incoming requests --- .../app/coffee/HttpController.coffee | 6 +++--- .../coffee/ProjectHistoryRedisManager.coffee | 2 ++ .../app/coffee/ProjectManager.coffee | 7 ++++++- .../HttpController/HttpControllerTests.coffee | 9 +++++---- .../ProjectManager/updateProjectTests.coffee | 17 +++++++++-------- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 9918339d6e..ce4d8bf637 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -161,10 +161,10 @@ module.exports = HttpController = updateProject: (req, res, next = (error) ->) -> timer = new Metrics.Timer("http.updateProject") project_id = req.params.project_id - {userId, docUpdates, fileUpdates} = req.body - logger.log {project_id, docUpdates, fileUpdates}, "updating project via http" + {userId, docUpdates, fileUpdates, version} = req.body + logger.log {project_id, docUpdates, fileUpdates, version}, "updating project via http" - ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, fileUpdates, (error) -> + ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, fileUpdates, version, (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/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index 6de1d8efc9..ebf5568317 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -14,6 +14,7 @@ module.exports = ProjectHistoryRedisManager = meta: user_id: user_id ts: new Date() + version: update.version update[entity_type] = entity_id logger.log {project_id, update}, "queue rename operation to project-history" @@ -29,6 +30,7 @@ module.exports = ProjectHistoryRedisManager = meta: user_id: user_id ts: new Date() + version: update.version update[entity_type] = entitiy_id logger.log {project_id, update}, "queue add operation to project-history" diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index 36ae86363d..729f1743e8 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -105,16 +105,20 @@ module.exports = ProjectManager = clearProjectState: (project_id, callback = (error) ->) -> RedisManager.clearProjectState project_id, callback - updateProjectWithLocks: (project_id, user_id, docUpdates, fileUpdates, _callback = (error) ->) -> + updateProjectWithLocks: (project_id, user_id, docUpdates, fileUpdates, version, _callback = (error) ->) -> timer = new Metrics.Timer("projectManager.updateProject") callback = (args...) -> timer.done() _callback(args...) + project_version = version + project_subversion = 0 # project versions can have multiple operations + project_ops_length = 0 handleDocUpdate = (update, cb) -> doc_id = update.id + update.version = "#{project_version}.#{project_subversion++}" if update.docLines? ProjectHistoryRedisManager.queueAddEntity project_id, 'doc', doc_id, user_id, update, (error, count) -> project_ops_length = count @@ -126,6 +130,7 @@ module.exports = ProjectManager = handleFileUpdate = (update, cb) -> file_id = update.id + update.version = "#{project_version}.#{project_subversion++}" if update.url? ProjectHistoryRedisManager.queueAddEntity project_id, 'file', file_id, user_id, update, (error, count) -> project_ops_length = count diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index b79659cada..fca1614c2d 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -512,19 +512,20 @@ describe "HttpController", -> @userId = "user-id-123" @docUpdates = sinon.stub() @fileUpdates = sinon.stub() + @version = 1234567 @req = - body: {@userId, @docUpdates, @fileUpdates} + body: {@userId, @docUpdates, @fileUpdates, @version} params: project_id: @project_id describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(4) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(5) @HttpController.updateProject(@req, @res, @next) it "should accept the change", -> @ProjectManager.updateProjectWithLocks - .calledWith(@project_id, @userId, @docUpdates, @fileUpdates) + .calledWith(@project_id, @userId, @docUpdates, @fileUpdates, @version) .should.equal true it "should return a successful No Content response", -> @@ -537,7 +538,7 @@ describe "HttpController", -> describe "when an errors occurs", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(4, new Error("oops")) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(5, 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 7bd5c19848..b04a8c7a50 100644 --- a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee @@ -18,6 +18,7 @@ describe "ProjectManager", -> @project_id = "project-id-123" @user_id = "user-id-123" + @version = 1234567 @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(false) @HistoryManager.flushProjectChangesAsync = sinon.stub() @callback = sinon.stub() @@ -45,7 +46,7 @@ describe "ProjectManager", -> describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should rename the docs in the updates", -> @DocumentManager.renameDocWithLock @@ -72,7 +73,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @DocumentManager.renameDocWithLock = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -81,7 +82,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueRenameEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -89,7 +90,7 @@ describe "ProjectManager", -> describe "with enough ops to flush", -> beforeEach -> @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should flush the history", -> @HistoryManager.flushProjectChangesAsync @@ -113,7 +114,7 @@ describe "ProjectManager", -> describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should add the docs in the updates", -> @ProjectHistoryRedisManager.queueAddEntity @@ -140,7 +141,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueAddEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -149,7 +150,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueAddEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -157,7 +158,7 @@ describe "ProjectManager", -> describe "with enough ops to flush", -> beforeEach -> @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @callback + @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should flush the history", -> @HistoryManager.flushProjectChangesAsync From b3887fd9844483c850cb8b73eff0f814086a7079 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 6 Mar 2018 09:49:54 +0000 Subject: [PATCH 2/6] update unit tests for incoming project versions --- .../ProjectManager/updateProjectTests.coffee | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee index b04a8c7a50..a5d3e881c6 100644 --- a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee @@ -3,6 +3,7 @@ chai = require('chai') should = chai.should() modulePath = "../../../../app/js/ProjectManager.js" SandboxedModule = require('sandboxed-module') +_ = require('underscore') describe "ProjectManager", -> beforeEach -> @@ -49,16 +50,19 @@ describe "ProjectManager", -> @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should rename the docs in the updates", -> + firstDocUpdateWithVersion = _.extend({}, @firstDocUpdate, {version: "#{@version}.0"}) + secondDocUpdateWithVersion = _.extend({}, @secondDocUpdate, {version: "#{@version}.1"}) @DocumentManager.renameDocWithLock - .calledWith(@project_id, @firstDocUpdate.id, @user_id, @firstDocUpdate) + .calledWith(@project_id, @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion) .should.equal true @DocumentManager.renameDocWithLock - .calledWith(@project_id, @secondDocUpdate.id, @user_id, @secondDocUpdate) + .calledWith(@project_id, @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) .should.equal true it "should rename the files in the updates", -> + firstFileUpdateWithVersion = _.extend({}, @firstFileUpdate, {version: "#{@version}.2"}) @ProjectHistoryRedisManager.queueRenameEntity - .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, @firstFileUpdate) + .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) .should.equal true it "should not flush the history", -> @@ -117,16 +121,19 @@ describe "ProjectManager", -> @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should add the docs in the updates", -> + firstDocUpdateWithVersion = _.extend({}, @firstDocUpdate, {version: "#{@version}.0"}) + secondDocUpdateWithVersion = _.extend({}, @secondDocUpdate, {version: "#{@version}.1"}) @ProjectHistoryRedisManager.queueAddEntity - .calledWith(@project_id, 'doc', @firstDocUpdate.id, @user_id, @firstDocUpdate) + .calledWith(@project_id, 'doc', @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion) .should.equal true @ProjectHistoryRedisManager.queueAddEntity - .calledWith(@project_id, 'doc', @secondDocUpdate.id, @user_id, @secondDocUpdate) + .calledWith(@project_id, 'doc', @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) .should.equal true it "should add the files in the updates", -> + firstFileUpdateWithVersion = _.extend({}, @firstFileUpdate, {version: "#{@version}.2"}) @ProjectHistoryRedisManager.queueAddEntity - .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, @firstFileUpdate) + .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) .should.equal true it "should not flush the history", -> From 75a5428cbffb97fea0f1c8ea47b0afd71f418468 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 6 Mar 2018 10:36:38 +0000 Subject: [PATCH 3/6] update acceptance tests --- ...lyingUpdatesToProjectStructureTests.coffee | 29 ++++++++++++------- .../coffee/helpers/DocUpdaterClient.coffee | 4 +-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee index d060cd918c..b617afc7db 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee @@ -13,6 +13,7 @@ DocUpdaterApp = require "./helpers/DocUpdaterApp" describe "Applying updates to a project's structure", -> before -> @user_id = 'user-id-123' + @version = 1234 describe "renaming a file", -> before (done) -> @@ -24,7 +25,7 @@ describe "Applying updates to a project's structure", -> @fileUpdates = [ @fileUpdate ] DocUpdaterApp.ensureRunning (error) => throw error if error? - DocUpdaterClient.sendProjectUpdate @project_id, @user_id, [], @fileUpdates, (error) -> + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, [], @fileUpdates, @version, (error) -> throw error if error? setTimeout done, 200 @@ -38,6 +39,7 @@ describe "Applying updates to a project's structure", -> update.new_pathname.should.equal '/new-file-path' update.meta.user_id.should.equal @user_id update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" done() @@ -52,7 +54,7 @@ describe "Applying updates to a project's structure", -> describe "when the document is not loaded", -> before (done) -> @project_id = DocUpdaterClient.randomId() - DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], (error) -> + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], @version, (error) -> throw error if error? setTimeout done, 200 @@ -66,6 +68,7 @@ describe "Applying updates to a project's structure", -> update.new_pathname.should.equal '/new-doc-path' update.meta.user_id.should.equal @user_id update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" done() @@ -76,7 +79,7 @@ describe "Applying updates to a project's structure", -> DocUpdaterClient.preloadDoc @project_id, @docUpdate.id, (error) => throw error if error? sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], (error) -> + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], @version, (error) -> throw error if error? setTimeout done, 200 @@ -98,6 +101,7 @@ describe "Applying updates to a project's structure", -> update.new_pathname.should.equal '/new-doc-path' update.meta.user_id.should.equal @user_id update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" done() @@ -109,7 +113,7 @@ describe "Applying updates to a project's structure", -> pathname: '/file-path' url: 'filestore.example.com' @fileUpdates = [ @fileUpdate ] - DocUpdaterClient.sendProjectUpdate @project_id, @user_id, [], @fileUpdates, (error) -> + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, [], @fileUpdates, @version, (error) -> throw error if error? setTimeout done, 200 @@ -123,6 +127,7 @@ describe "Applying updates to a project's structure", -> update.url.should.equal 'filestore.example.com' update.meta.user_id.should.equal @user_id update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" done() @@ -134,7 +139,7 @@ describe "Applying updates to a project's structure", -> pathname: '/file-path' docLines: 'a\nb' @docUpdates = [ @docUpdate ] - DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], (error) -> + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, [], @version, (error) -> throw error if error? setTimeout done, 200 @@ -148,6 +153,7 @@ describe "Applying updates to a project's structure", -> update.docLines.should.equal 'a\nb' update.meta.user_id.should.equal @user_id update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" done() @@ -155,7 +161,8 @@ describe "Applying updates to a project's structure", -> before (done) -> @project_id = DocUpdaterClient.randomId() @user_id = DocUpdaterClient.randomId() - + @version0 = 12345 + @version1 = @version0 + 1 updates = [] for v in [0..599] # Should flush after 500 ops updates.push @@ -168,9 +175,9 @@ describe "Applying updates to a project's structure", -> # Send updates in chunks to causes multiple flushes projectId = @project_id userId = @project_id - DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(0, 250), [], (error) -> + DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(0, 250), [], @version0, (error) -> throw error if error? - DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(250), [], (error) -> + DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(250), [], @version1, (error) -> throw error if error? setTimeout done, 2000 @@ -184,6 +191,8 @@ describe "Applying updates to a project's structure", -> before (done) -> @project_id = DocUpdaterClient.randomId() @user_id = DocUpdaterClient.randomId() + @version0 = 12345 + @version1 = @version0 + 1 updates = [] for v in [0..42] # Should flush after 500 ops @@ -197,9 +206,9 @@ describe "Applying updates to a project's structure", -> # Send updates in chunks projectId = @project_id userId = @project_id - DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(0, 10), [], (error) -> + DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(0, 10), [], @version0, (error) -> throw error if error? - DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(10), [], (error) -> + DocUpdaterClient.sendProjectUpdate projectId, userId, updates.slice(10), [], @version1, (error) -> throw error if error? setTimeout done, 2000 diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index f70271021b..7f50d64372 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -87,9 +87,9 @@ module.exports = DocUpdaterClient = body = JSON.parse(body) callback error, res, body - sendProjectUpdate: (project_id, userId, docUpdates, fileUpdates, callback = (error) ->) -> + sendProjectUpdate: (project_id, userId, docUpdates, fileUpdates, version, callback = (error) ->) -> request.post { url: "http://localhost:3003/project/#{project_id}" - json: { userId, docUpdates, fileUpdates } + json: { userId, docUpdates, fileUpdates, version } }, (error, res, body) -> callback error, res, body From 3385d2640a5a8a8e140bf8b4887ab8d5bdcce9fb Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 7 Mar 2018 17:06:42 +0000 Subject: [PATCH 4/6] fix structure ordering bug --- .../app/coffee/ProjectManager.coffee | 4 ++-- .../ProjectManager/updateProjectTests.coffee | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index 729f1743e8..8ae14cd66e 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -140,9 +140,9 @@ module.exports = ProjectManager = project_ops_length = count cb(error) - async.each docUpdates, handleDocUpdate, (error) -> + async.eachSeries docUpdates, handleDocUpdate, (error) -> return callback(error) if error? - async.each fileUpdates, handleFileUpdate, (error) -> + async.eachSeries fileUpdates, handleFileUpdate, (error) -> return callback(error) if error? if HistoryManager.shouldFlushHistoryOps(project_ops_length, docUpdates.length + fileUpdates.length, HistoryManager.FLUSH_PROJECT_EVERY_N_OPS) HistoryManager.flushProjectChangesAsync project_id diff --git a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee index a5d3e881c6..96d2ccc07b 100644 --- a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee @@ -111,9 +111,12 @@ describe "ProjectManager", -> docLines: "a\nb" @docUpdates = [ @firstDocUpdate, @secondDocUpdate ] @firstFileUpdate = - id: 2 + id: 3 url: 'filestore.example.com/2' - @fileUpdates = [ @firstFileUpdate ] + @secondFileUpdate = + id: 4 + url: 'filestore.example.com/3' + @fileUpdates = [ @firstFileUpdate, @secondFileUpdate ] @ProjectHistoryRedisManager.queueAddEntity = sinon.stub().yields() describe "successfully", -> @@ -123,18 +126,22 @@ describe "ProjectManager", -> it "should add the docs in the updates", -> firstDocUpdateWithVersion = _.extend({}, @firstDocUpdate, {version: "#{@version}.0"}) secondDocUpdateWithVersion = _.extend({}, @secondDocUpdate, {version: "#{@version}.1"}) - @ProjectHistoryRedisManager.queueAddEntity + @ProjectHistoryRedisManager.queueAddEntity.getCall(0) .calledWith(@project_id, 'doc', @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion) .should.equal true - @ProjectHistoryRedisManager.queueAddEntity + @ProjectHistoryRedisManager.queueAddEntity.getCall(1) .calledWith(@project_id, 'doc', @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) .should.equal true it "should add the files in the updates", -> firstFileUpdateWithVersion = _.extend({}, @firstFileUpdate, {version: "#{@version}.2"}) - @ProjectHistoryRedisManager.queueAddEntity + secondFileUpdateWithVersion = _.extend({}, @secondFileUpdate, {version: "#{@version}.3"}) + @ProjectHistoryRedisManager.queueAddEntity.getCall(2) .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) .should.equal true + @ProjectHistoryRedisManager.queueAddEntity.getCall(3) + .calledWith(@project_id, 'file', @secondFileUpdate.id, @user_id, secondFileUpdateWithVersion) + .should.equal true it "should not flush the history", -> @HistoryManager.flushProjectChangesAsync From 779f00f9125a013f1a28a8c7e9d6561bb7abf199 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 8 Mar 2018 09:38:24 +0000 Subject: [PATCH 5/6] add acceptance test for ordering of project structure changes --- ...lyingUpdatesToProjectStructureTests.coffee | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee index b617afc7db..5fdf2a9b4d 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToProjectStructureTests.coffee @@ -105,6 +105,73 @@ describe "Applying updates to a project's structure", -> done() + describe "renaming multiple documents and files", -> + before -> + @docUpdate0 = + id: DocUpdaterClient.randomId() + pathname: '/doc-path0' + newPathname: '/new-doc-path0' + @docUpdate1 = + id: DocUpdaterClient.randomId() + pathname: '/doc-path1' + newPathname: '/new-doc-path1' + @docUpdates = [ @docUpdate0, @docUpdate1 ] + @fileUpdate0 = + id: DocUpdaterClient.randomId() + pathname: '/file-path0' + newPathname: '/new-file-path0' + @fileUpdate1 = + id: DocUpdaterClient.randomId() + pathname: '/file-path1' + newPathname: '/new-file-path1' + @fileUpdates = [ @fileUpdate0, @fileUpdate1 ] + + describe "when the documents are not loaded", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + DocUpdaterClient.sendProjectUpdate @project_id, @user_id, @docUpdates, @fileUpdates, @version, (error) -> + throw error if error? + setTimeout done, 200 + + it "should push the applied doc renames to the project history 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 @docUpdate0.id + update.pathname.should.equal '/doc-path0' + update.new_pathname.should.equal '/new-doc-path0' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.0" + + update = JSON.parse(updates[1]) + update.doc.should.equal @docUpdate1.id + update.pathname.should.equal '/doc-path1' + update.new_pathname.should.equal '/new-doc-path1' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.1" + + update = JSON.parse(updates[2]) + update.file.should.equal @fileUpdate0.id + update.pathname.should.equal '/file-path0' + update.new_pathname.should.equal '/new-file-path0' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.2" + + update = JSON.parse(updates[3]) + update.file.should.equal @fileUpdate1.id + update.pathname.should.equal '/file-path1' + update.new_pathname.should.equal '/new-file-path1' + update.meta.user_id.should.equal @user_id + update.meta.ts.should.be.a('string') + update.version.should.equal "#{@version}.3" + + done() + + describe "adding a file", -> before (done) -> @project_id = DocUpdaterClient.randomId() From dd0f8b880aeb71a6a82db5fe688e7ab826587825 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 16 Mar 2018 10:54:12 +0000 Subject: [PATCH 6/6] change update to projectUpdate in project related methods --- .../coffee/ProjectHistoryRedisManager.coffee | 42 +++++++++---------- .../app/coffee/ProjectManager.coffee | 24 +++++------ 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index ebf5568317..c92b7277f4 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -7,49 +7,49 @@ module.exports = ProjectHistoryRedisManager = queueOps: (project_id, ops..., callback) -> rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops..., callback - queueRenameEntity: (project_id, entity_type, entity_id, user_id, update, callback) -> - update = - pathname: update.pathname - new_pathname: update.newPathname + queueRenameEntity: (project_id, entity_type, entity_id, user_id, projectUpdate, callback) -> + projectUpdate = + pathname: projectUpdate.pathname + new_pathname: projectUpdate.newPathname meta: user_id: user_id ts: new Date() - version: update.version - update[entity_type] = entity_id + version: projectUpdate.version + projectUpdate[entity_type] = entity_id - logger.log {project_id, update}, "queue rename operation to project-history" - jsonUpdate = JSON.stringify(update) + logger.log {project_id, projectUpdate}, "queue rename operation to project-history" + jsonUpdate = JSON.stringify(projectUpdate) ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback - queueAddEntity: (project_id, entity_type, entitiy_id, user_id, update, callback = (error) ->) -> - update = - pathname: update.pathname - docLines: update.docLines - url: update.url + queueAddEntity: (project_id, entity_type, entitiy_id, user_id, projectUpdate, callback = (error) ->) -> + projectUpdate = + pathname: projectUpdate.pathname + docLines: projectUpdate.docLines + url: projectUpdate.url meta: user_id: user_id ts: new Date() - version: update.version - update[entity_type] = entitiy_id + version: projectUpdate.version + projectUpdate[entity_type] = entitiy_id - logger.log {project_id, update}, "queue add operation to project-history" - jsonUpdate = JSON.stringify(update) + logger.log {project_id, projectUpdate}, "queue add operation to project-history" + jsonUpdate = JSON.stringify(projectUpdate) ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback queueResyncProjectStructure: (project_id, docs, files, callback) -> logger.log {project_id, docs, files}, "queue project structure resync" - update = + projectUpdate = resyncProjectStructure: { docs, files } meta: ts: new Date() - jsonUpdate = JSON.stringify update + jsonUpdate = JSON.stringify projectUpdate ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback queueResyncDocContent: (project_id, doc_id, lines, version, pathname, callback) -> logger.log {project_id, doc_id, lines, version, pathname}, "queue doc content resync" - update = + projectUpdate = resyncDocContent: content: lines.join("\n"), version: version @@ -57,5 +57,5 @@ module.exports = ProjectHistoryRedisManager = doc: doc_id meta: ts: new Date() - jsonUpdate = JSON.stringify update + jsonUpdate = JSON.stringify projectUpdate ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index 8ae14cd66e..eb7acaede1 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -116,27 +116,27 @@ module.exports = ProjectManager = project_ops_length = 0 - handleDocUpdate = (update, cb) -> - doc_id = update.id - update.version = "#{project_version}.#{project_subversion++}" - if update.docLines? - ProjectHistoryRedisManager.queueAddEntity project_id, 'doc', doc_id, user_id, update, (error, count) -> + handleDocUpdate = (projectUpdate, cb) -> + doc_id = projectUpdate.id + projectUpdate.version = "#{project_version}.#{project_subversion++}" + if projectUpdate.docLines? + ProjectHistoryRedisManager.queueAddEntity project_id, 'doc', doc_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) else - DocumentManager.renameDocWithLock project_id, doc_id, user_id, update, (error, count) -> + DocumentManager.renameDocWithLock project_id, doc_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) - handleFileUpdate = (update, cb) -> - file_id = update.id - update.version = "#{project_version}.#{project_subversion++}" - if update.url? - ProjectHistoryRedisManager.queueAddEntity project_id, 'file', file_id, user_id, update, (error, count) -> + handleFileUpdate = (projectUpdate, cb) -> + file_id = projectUpdate.id + projectUpdate.version = "#{project_version}.#{project_subversion++}" + if projectUpdate.url? + ProjectHistoryRedisManager.queueAddEntity project_id, 'file', file_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) else - ProjectHistoryRedisManager.queueRenameEntity project_id, 'file', file_id, user_id, update, (error, count) -> + ProjectHistoryRedisManager.queueRenameEntity project_id, 'file', file_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error)