From 1b63141e4915ae971085d5b1323d01e1b29b71d2 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 23 Apr 2018 12:08:04 +0100 Subject: [PATCH 1/2] Revert "Revert "Add projectHistoryId to updates"" --- .../app/coffee/DocumentManager.coffee | 44 +++++++++--------- .../app/coffee/HistoryManager.coffee | 4 +- .../app/coffee/HttpController.coffee | 8 ++-- .../app/coffee/PersistenceManager.coffee | 4 +- .../coffee/ProjectHistoryRedisManager.coffee | 12 +++-- .../app/coffee/ProjectManager.coffee | 10 ++--- .../app/coffee/RedisManager.coffee | 21 +++++---- .../app/coffee/UpdateManager.coffee | 7 +-- .../config/settings.defaults.coffee | 1 + .../DocumentManagerTests.coffee | 35 ++++++++------- .../HistoryManager/HistoryManagerTests.coffee | 5 ++- .../HttpController/HttpControllerTests.coffee | 18 ++++---- .../PersistenceManagerTests.coffee | 6 ++- .../ProjectHistoryRedisManagerTests.coffee | 11 ++++- .../ProjectManager/updateProjectTests.coffee | 32 ++++++------- .../RedisManager/RedisManagerTests.coffee | 45 +++++++++++++------ .../UpdateManager/UpdateManagerTests.coffee | 12 +++-- 17 files changed, 160 insertions(+), 115 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 640ebf63cb..0c50d9b1f3 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -13,39 +13,39 @@ async = require "async" MAX_UNFLUSHED_AGE = 300 * 1000 # 5 mins, document should be flushed to mongo this time after a change module.exports = DocumentManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) ->) -> timer = new Metrics.Timer("docManager.getDoc") callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime) -> return callback(error) if error? if !lines? or !version? logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API" - PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? - logger.log {project_id, doc_id, lines, version, pathname}, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, (error) -> + logger.log {project_id, doc_id, lines, version, pathname, projectHistoryId}, "got doc from persistence API" + RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, projectHistoryId, (error) -> return callback(error) if error? - callback null, lines, version, ranges, pathname, null, false + callback null, lines, version, ranges, pathname, projectHistoryId, null, false else - callback null, lines, version, ranges, pathname, unflushedTime, true + callback null, lines, version, ranges, pathname, projectHistoryId, unflushedTime, true - getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, ops, ranges, pathname) ->) -> + getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, ops, ranges, pathname, projectHistoryId) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") callback = (args...) -> timer.done() _callback(args...) - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? if fromVersion == -1 - callback null, lines, version, [], ranges, pathname + callback null, lines, version, [], ranges, pathname, projectHistoryId else RedisManager.getPreviousDocOps doc_id, fromVersion, version, (error, ops) -> return callback(error) if error? - callback null, lines, version, ops, ranges, pathname + callback null, lines, version, ops, ranges, pathname, projectHistoryId setDoc: (project_id, doc_id, newLines, source, user_id, undoing, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") @@ -57,7 +57,7 @@ module.exports = DocumentManager = return callback(new Error("No lines were provided to setDoc")) UpdateManager = require "./UpdateManager" - DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) -> return callback(error) if error? if oldLines? and oldLines.length > 0 and oldLines[0].text? @@ -161,16 +161,16 @@ module.exports = DocumentManager = return callback(error) if error? callback() - renameDoc: (project_id, doc_id, user_id, update, _callback = (error) ->) -> + renameDoc: (project_id, doc_id, user_id, update, projectHistoryId, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.updateProject") callback = (args...) -> timer.done() _callback(args...) - RedisManager.renameDoc project_id, doc_id, user_id, update, callback + RedisManager.renameDoc project_id, doc_id, user_id, update, projectHistoryId, callback getDocAndFlushIfOld: (project_id, doc_id, callback = (error, doc) ->) -> - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) -> return callback(error) if error? # if doc was already loaded see if it needs to be flushed if alreadyLoaded and unflushedTime? and (Date.now() - unflushedTime) > MAX_UNFLUSHED_AGE @@ -181,21 +181,21 @@ module.exports = DocumentManager = callback(null, lines, version) resyncDocContents: (project_id, doc_id, callback) -> - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? if !lines? or !version? - PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? - ProjectHistoryRedisManager.queueResyncDocContent project_id, doc_id, lines, version, pathname, callback + ProjectHistoryRedisManager.queueResyncDocContent project_id, projectHistoryId, doc_id, lines, version, pathname, callback else - ProjectHistoryRedisManager.queueResyncDocContent project_id, doc_id, lines, version, pathname, callback + ProjectHistoryRedisManager.queueResyncDocContent project_id, projectHistoryId, doc_id, lines, version, pathname, callback getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDoc, project_id, doc_id, callback - getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version, ops, ranges, pathname) ->) -> + getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version, ops, ranges, pathname, projectHistoryId) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback @@ -223,9 +223,9 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.deleteComment, project_id, doc_id, thread_id, callback - renameDocWithLock: (project_id, doc_id, user_id, update, callback = (error) ->) -> + renameDocWithLock: (project_id, doc_id, user_id, update, projectHistoryId, callback = (error) ->) -> UpdateManager = require "./UpdateManager" - UpdateManager.lockUpdatesAndDo DocumentManager.renameDoc, project_id, doc_id, user_id, update, callback + UpdateManager.lockUpdatesAndDo DocumentManager.renameDoc, project_id, doc_id, user_id, update, projectHistoryId, callback resyncDocContentsWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" diff --git a/services/document-updater/app/coffee/HistoryManager.coffee b/services/document-updater/app/coffee/HistoryManager.coffee index c1371615f7..9d39166681 100644 --- a/services/document-updater/app/coffee/HistoryManager.coffee +++ b/services/document-updater/app/coffee/HistoryManager.coffee @@ -65,8 +65,8 @@ module.exports = HistoryManager = newBlock = Math.floor(length / threshold) return newBlock != prevBlock - resyncProjectHistory: (project_id, docs, files, callback) -> - ProjectHistoryRedisManager.queueResyncProjectStructure project_id, docs, files, (error) -> + resyncProjectHistory: (project_id, projectHistoryId, docs, files, callback) -> + ProjectHistoryRedisManager.queueResyncProjectStructure project_id, projectHistoryId, docs, files, (error) -> return callback(error) if error? DocumentManager = require "./DocumentManager" resyncDoc = (doc, cb) -> diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index ce4d8bf637..93f915d662 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, version} = req.body + {projectHistoryId, userId, docUpdates, fileUpdates, version} = req.body logger.log {project_id, docUpdates, fileUpdates, version}, "updating project via http" - ProjectManager.updateProjectWithLocks project_id, userId, docUpdates, fileUpdates, version, (error) -> + ProjectManager.updateProjectWithLocks project_id, projectHistoryId, userId, docUpdates, fileUpdates, version, (error) -> timer.done() return next(error) if error? logger.log project_id: project_id, "updated project via http" @@ -172,10 +172,10 @@ module.exports = HttpController = resyncProjectHistory: (req, res, next = (error) ->) -> project_id = req.params.project_id - {docs, files} = req.body + {projectHistoryId, docs, files} = req.body logger.log {project_id, docs, files}, "queuing project history resync via http" - HistoryManager.resyncProjectHistory project_id, docs, files, (error) -> + HistoryManager.resyncProjectHistory project_id, projectHistoryId, docs, files, (error) -> return next(error) if error? logger.log {project_id}, "queued project history resync via http" res.send 204 diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index bd5ce5239c..8a43d989a8 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -13,7 +13,7 @@ request = (require("requestretry")).defaults({ MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds module.exports = PersistenceManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -44,7 +44,7 @@ module.exports = PersistenceManager = return callback(new Error("web API response had no valid doc version")) if !body.pathname? return callback(new Error("web API response had no valid doc pathname")) - return callback null, body.lines, body.version, body.ranges, body.pathname + return callback null, body.lines, body.version, body.ranges, body.pathname, body.projectHistoryId else if res.statusCode == 404 return callback(new Errors.NotFoundError("doc not not found: #{url}")) else diff --git a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index c92b7277f4..42b9f16df2 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -7,7 +7,7 @@ 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, projectUpdate, callback) -> + queueRenameEntity: (project_id, projectHistoryId, entity_type, entity_id, user_id, projectUpdate, callback) -> projectUpdate = pathname: projectUpdate.pathname new_pathname: projectUpdate.newPathname @@ -15,6 +15,7 @@ module.exports = ProjectHistoryRedisManager = user_id: user_id ts: new Date() version: projectUpdate.version + projectHistoryId: projectHistoryId projectUpdate[entity_type] = entity_id logger.log {project_id, projectUpdate}, "queue rename operation to project-history" @@ -22,7 +23,7 @@ module.exports = ProjectHistoryRedisManager = ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback - queueAddEntity: (project_id, entity_type, entitiy_id, user_id, projectUpdate, callback = (error) ->) -> + queueAddEntity: (project_id, projectHistoryId, entity_type, entitiy_id, user_id, projectUpdate, callback = (error) ->) -> projectUpdate = pathname: projectUpdate.pathname docLines: projectUpdate.docLines @@ -31,6 +32,7 @@ module.exports = ProjectHistoryRedisManager = user_id: user_id ts: new Date() version: projectUpdate.version + projectHistoryId: projectHistoryId projectUpdate[entity_type] = entitiy_id logger.log {project_id, projectUpdate}, "queue add operation to project-history" @@ -38,21 +40,23 @@ module.exports = ProjectHistoryRedisManager = ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback - queueResyncProjectStructure: (project_id, docs, files, callback) -> + queueResyncProjectStructure: (project_id, projectHistoryId, docs, files, callback) -> logger.log {project_id, docs, files}, "queue project structure resync" projectUpdate = resyncProjectStructure: { docs, files } + projectHistoryId: projectHistoryId meta: ts: new Date() jsonUpdate = JSON.stringify projectUpdate ProjectHistoryRedisManager.queueOps project_id, jsonUpdate, callback - queueResyncDocContent: (project_id, doc_id, lines, version, pathname, callback) -> + queueResyncDocContent: (project_id, projectHistoryId, doc_id, lines, version, pathname, callback) -> logger.log {project_id, doc_id, lines, version, pathname}, "queue doc content resync" projectUpdate = resyncDocContent: content: lines.join("\n"), version: version + projectHistoryId: projectHistoryId path: pathname doc: doc_id meta: diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index eb7acaede1..cbf7bb661b 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -105,7 +105,7 @@ module.exports = ProjectManager = clearProjectState: (project_id, callback = (error) ->) -> RedisManager.clearProjectState project_id, callback - updateProjectWithLocks: (project_id, user_id, docUpdates, fileUpdates, version, _callback = (error) ->) -> + updateProjectWithLocks: (project_id, projectHistoryId, user_id, docUpdates, fileUpdates, version, _callback = (error) ->) -> timer = new Metrics.Timer("projectManager.updateProject") callback = (args...) -> timer.done() @@ -120,11 +120,11 @@ module.exports = ProjectManager = 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) -> + ProjectHistoryRedisManager.queueAddEntity project_id, projectHistoryId, 'doc', doc_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) else - DocumentManager.renameDocWithLock project_id, doc_id, user_id, projectUpdate, (error, count) -> + DocumentManager.renameDocWithLock project_id, doc_id, user_id, projectUpdate, projectHistoryId, (error, count) -> project_ops_length = count cb(error) @@ -132,11 +132,11 @@ module.exports = ProjectManager = 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) -> + ProjectHistoryRedisManager.queueAddEntity project_id, projectHistoryId, 'file', file_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) else - ProjectHistoryRedisManager.queueRenameEntity project_id, 'file', file_id, user_id, projectUpdate, (error, count) -> + ProjectHistoryRedisManager.queueRenameEntity project_id, projectHistoryId, 'file', file_id, user_id, projectUpdate, (error, count) -> project_ops_length = count cb(error) diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 382ff2b502..a940970176 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -36,7 +36,7 @@ historyKeys = Settings.redis.history.key_schema module.exports = RedisManager = rclient: rclient - putDocInMemory : (project_id, doc_id, docLines, version, ranges, pathname, _callback)-> + putDocInMemory : (project_id, doc_id, docLines, version, ranges, pathname, projectHistoryId, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> timer.done() @@ -47,7 +47,7 @@ module.exports = RedisManager = logger.error {err: error, doc_id: doc_id, docLines: docLines}, error.message return callback(error) docHash = RedisManager._computeHash(docLines) - logger.log {project_id, doc_id, version, docHash, pathname}, "putting doc in redis" + logger.log {project_id, doc_id, version, docHash, pathname, projectHistoryId}, "putting doc in redis" RedisManager._serializeRanges ranges, (error, ranges) -> if error? logger.error {err: error, doc_id, project_id}, error.message @@ -62,6 +62,7 @@ module.exports = RedisManager = else multi.del keys.ranges(doc_id:doc_id) multi.set keys.pathname(doc_id:doc_id), pathname + multi.set keys.projectHistoryId(doc_id:doc_id), projectHistoryId multi.exec (error, result) -> return callback(error) if error? # check the hash computed on the redis server @@ -88,6 +89,7 @@ module.exports = RedisManager = multi.del keys.docHash(doc_id:doc_id) multi.del keys.ranges(doc_id:doc_id) multi.del keys.pathname(doc_id:doc_id) + multi.del keys.projectHistoryId(doc_id:doc_id) multi.del keys.unflushedTime(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? @@ -108,7 +110,7 @@ module.exports = RedisManager = clearProjectState: (project_id, callback = (error) ->) -> rclient.del keys.projectState(project_id:project_id), callback - getDoc : (project_id, doc_id, callback = (error, lines, version, ranges, pathname, unflushedTime) ->)-> + getDoc : (project_id, doc_id, callback = (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi() multi.get keys.docLines(doc_id:doc_id) @@ -117,8 +119,9 @@ module.exports = RedisManager = multi.get keys.projectKey(doc_id:doc_id) multi.get keys.ranges(doc_id:doc_id) multi.get keys.pathname(doc_id:doc_id) + multi.get keys.projectHistoryId(doc_id:doc_id) multi.get keys.unflushedTime(doc_id:doc_id) - multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, unflushedTime])-> + multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, projectHistoryId, unflushedTime])-> timeSpan = timer.done() return callback(error) if error? # check if request took too long and bail out. only do this for @@ -147,14 +150,14 @@ module.exports = RedisManager = # doc is not in redis, bail out if !docLines? - return callback null, docLines, version, ranges, pathname, unflushedTime + return callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime # doc should be in project set, check if missing (workaround for missing docs from putDoc) rclient.sadd keys.docsInProject(project_id:project_id), doc_id, (error, result) -> return callback(error) if error? if result isnt 0 # doc should already be in set logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc missing from docsInProject set" - callback null, docLines, version, ranges, pathname, unflushedTime + callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -272,16 +275,16 @@ module.exports = RedisManager = else callback null, docUpdateCount - renameDoc: (project_id, doc_id, user_id, update, callback = (error) ->) -> + renameDoc: (project_id, doc_id, user_id, update, projectHistoryId, 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? - ProjectHistoryRedisManager.queueRenameEntity project_id, 'doc', doc_id, user_id, update, callback + ProjectHistoryRedisManager.queueRenameEntity project_id, projectHistoryId, 'doc', doc_id, user_id, update, callback else - ProjectHistoryRedisManager.queueRenameEntity project_id, 'doc', doc_id, user_id, update, callback + ProjectHistoryRedisManager.queueRenameEntity project_id, projectHistoryId, 'doc', doc_id, user_id, update, callback clearUnflushedTime: (doc_id, callback = (error) ->) -> rclient.del keys.unflushedTime(doc_id:doc_id), callback diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 02f0a1b8d1..bfcfb806ca 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -71,7 +71,7 @@ module.exports = UpdateManager = profile = new Profiler("applyUpdate", {project_id, doc_id}) UpdateManager._sanitizeUpdate update profile.log("sanitizeUpdate") - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> profile.log("getDoc") return callback(error) if error? if !lines? or !version? @@ -80,7 +80,7 @@ module.exports = UpdateManager = profile.log("sharejs.applyUpdate") return callback(error) if error? RangesManager.applyUpdate project_id, doc_id, ranges, appliedOps, updatedDocLines, (error, new_ranges) -> - UpdateManager._addProjectHistoryMetadataToOps(appliedOps, pathname, lines) + UpdateManager._addProjectHistoryMetadataToOps(appliedOps, pathname, projectHistoryId, lines) profile.log("RangesManager.applyUpdate") return callback(error) if error? RedisManager.updateDocument project_id, doc_id, updatedDocLines, version, appliedOps, new_ranges, (error, doc_ops_length, project_ops_length) -> @@ -130,12 +130,13 @@ module.exports = UpdateManager = op.i = op.i.replace(/[\uD800-\uDFFF]/g, "\uFFFD") return update - _addProjectHistoryMetadataToOps: (updates, pathname, lines) -> + _addProjectHistoryMetadataToOps: (updates, pathname, projectHistoryId, lines) -> doc_length = _.reduce lines, (chars, line) -> chars + line.length, 0 doc_length += lines.length - 1 # count newline characters updates.forEach (update) -> + update.projectHistoryId = projectHistoryId update.meta ||= {} update.meta.pathname = pathname update.meta.doc_length = doc_length diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index 1c7ebf283e..18f2b1570b 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -46,6 +46,7 @@ module.exports = docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" pathname: ({doc_id}) -> "Pathname:#{doc_id}" + projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" # cluster: [{ diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 16e58a81a7..d7cd18630a 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -25,6 +25,7 @@ describe "DocumentManager", -> "./UpdateManager": @UpdateManager = {} "./RangesManager": @RangesManager = {} @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @doc_id = "doc-id-123" @user_id = 1234 @callback = sinon.stub() @@ -111,7 +112,7 @@ describe "DocumentManager", -> describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback @@ -126,14 +127,14 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ops, @ranges, @pathname).should.equal true + @callback.calledWith(null, @lines, @version, @ops, @ranges, @pathname, @projectHistoryId).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, @pathname) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback @@ -146,7 +147,7 @@ describe "DocumentManager", -> @RedisManager.getPreviousDocOps.called.should.equal false it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, [], @ranges, @pathname).should.equal true + @callback.calledWith(null, @lines, @version, [], @ranges, @pathname, @projectHistoryId).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -154,7 +155,7 @@ describe "DocumentManager", -> describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @unflushedTime) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @@ -163,7 +164,7 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname, @unflushedTime, true).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime, true).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -171,7 +172,7 @@ describe "DocumentManager", -> describe "when the doc does not exist in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -187,11 +188,11 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId) .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname, null, false).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId, null, false).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -202,7 +203,7 @@ describe "DocumentManager", -> @beforeLines = ["before", "lines"] @afterLines = ["after", "lines"] @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, @pathname, @unflushedTime, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) @@ -402,7 +403,7 @@ describe "DocumentManager", -> describe "when the doc is in Redis", -> describe "and has changes to be flushed", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, Date.now() - 1e9, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @projectHistoryId, @pathname, Date.now() - 1e9, true) @DocumentManager.getDocAndFlushIfOld @project_id, @doc_id, @callback it "should get the doc", -> @@ -459,11 +460,11 @@ describe "DocumentManager", -> describe "successfully", -> beforeEach -> - @DocumentManager.renameDoc @project_id, @doc_id, @user_id, @update, @callback + @DocumentManager.renameDoc @project_id, @doc_id, @user_id, @update, @projectHistoryId, @callback it "should rename the document", -> @RedisManager.renameDoc - .calledWith(@project_id, @doc_id, @user_id, @update) + .calledWith(@project_id, @doc_id, @user_id, @update, @projectHistoryId) .should.equal true it "should call the callback", -> @@ -472,7 +473,7 @@ describe "DocumentManager", -> describe "resyncDocContents", -> describe "when doc is loaded in redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub() @DocumentManager.resyncDocContents @project_id, @doc_id, @callback @@ -483,13 +484,13 @@ describe "DocumentManager", -> it "queues a resync doc content update", -> @ProjectHistoryRedisManager.queueResyncDocContent - .calledWith(@project_id, @doc_id, @lines, @version, @pathname, @callback) + .calledWith(@project_id, @projectHistoryId, @doc_id, @lines, @version, @pathname, @callback) .should.equal true describe "when doc is not loaded in redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(2, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub() @DocumentManager.resyncDocContents @project_id, @doc_id, @callback @@ -505,5 +506,5 @@ describe "DocumentManager", -> it "queues a resync doc content update", -> @ProjectHistoryRedisManager.queueResyncDocContent - .calledWith(@project_id, @doc_id, @lines, @version, @pathname, @callback) + .calledWith(@project_id, @projectHistoryId, @doc_id, @lines, @version, @pathname, @callback) .should.equal true diff --git a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee index e207cde99d..2233610d28 100644 --- a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee @@ -164,6 +164,7 @@ describe "HistoryManager", -> describe "resyncProjectHistory", -> beforeEach -> + @projectHistoryId = 'history-id-1234' @docs = [ doc: @doc_id path: 'main.tex' @@ -175,11 +176,11 @@ describe "HistoryManager", -> ] @ProjectHistoryRedisManager.queueResyncProjectStructure = sinon.stub().yields() @DocumentManager.resyncDocContentsWithLock = sinon.stub().yields() - @HistoryManager.resyncProjectHistory @project_id, @docs, @files, @callback + @HistoryManager.resyncProjectHistory @project_id, @projectHistoryId, @docs, @files, @callback it "should queue a project structure reync", -> @ProjectHistoryRedisManager.queueResyncProjectStructure - .calledWith(@project_id, @docs, @files) + .calledWith(@project_id, @projectHistoryId, @docs, @files) .should.equal true it "should queue doc content reyncs", -> diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index fca1614c2d..ab6718c12a 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -509,23 +509,24 @@ describe "HttpController", -> describe "updateProject", -> beforeEach -> + @projectHistoryId = "history-id-123" @userId = "user-id-123" @docUpdates = sinon.stub() @fileUpdates = sinon.stub() @version = 1234567 @req = - body: {@userId, @docUpdates, @fileUpdates, @version} + body: {@projectHistoryId, @userId, @docUpdates, @fileUpdates, @version} params: project_id: @project_id describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(5) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(6) @HttpController.updateProject(@req, @res, @next) it "should accept the change", -> @ProjectManager.updateProjectWithLocks - .calledWith(@project_id, @userId, @docUpdates, @fileUpdates, @version) + .calledWith(@project_id, @projectHistoryId, @userId, @docUpdates, @fileUpdates, @version) .should.equal true it "should return a successful No Content response", -> @@ -538,7 +539,7 @@ describe "HttpController", -> describe "when an errors occurs", -> beforeEach -> - @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(5, new Error("oops")) + @ProjectManager.updateProjectWithLocks = sinon.stub().callsArgWith(6, new Error("oops")) @HttpController.updateProject(@req, @res, @next) it "should call next with the error", -> @@ -548,23 +549,24 @@ describe "HttpController", -> describe "resyncProjectHistory", -> beforeEach -> + @projectHistoryId = "history-id-123" @docs = sinon.stub() @files = sinon.stub() @fileUpdates = sinon.stub() @req = body: - {@docs, @files} + {@projectHistoryId, @docs, @files} params: project_id: @project_id describe "successfully", -> beforeEach -> - @HistoryManager.resyncProjectHistory = sinon.stub().callsArg(3) + @HistoryManager.resyncProjectHistory = sinon.stub().callsArgWith(4) @HttpController.resyncProjectHistory(@req, @res, @next) it "should accept the change", -> @HistoryManager.resyncProjectHistory - .calledWith(@project_id, @docs, @files) + .calledWith(@project_id, @projectHistoryId, @docs, @files) .should.equal true it "should return a successful No Content response", -> @@ -574,7 +576,7 @@ describe "HttpController", -> describe "when an errors occurs", -> beforeEach -> - @HistoryManager.resyncProjectHistory = sinon.stub().callsArgWith(3, new Error("oops")) + @HistoryManager.resyncProjectHistory = sinon.stub().callsArgWith(4, new Error("oops")) @HttpController.resyncProjectHistory(@req, @res, @next) it "should call next with the error", -> diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee index 937dcf3a77..0f8ad59167 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -17,6 +17,7 @@ describe "PersistenceManager", -> done: sinon.stub() "logger-sharelatex": @logger = {log: sinon.stub(), err: sinon.stub()} @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] @version = 42 @@ -36,6 +37,7 @@ describe "PersistenceManager", -> version: @version, ranges: @ranges pathname: @pathname, + projectHistoryId: @projectHistoryId } describe "with a successful response from the web api", -> @@ -60,7 +62,9 @@ describe "PersistenceManager", -> .should.equal true it "should call the callback with the doc lines, version and ranges", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname).should.equal true + @callback + .calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId) + .should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee index 8ad0f53b5b..349d3623e6 100644 --- a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee @@ -8,6 +8,7 @@ tk = require "timekeeper" describe "ProjectHistoryRedisManager", -> beforeEach -> @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @user_id = "user-id-123" @callback = sinon.stub() @rclient = {} @@ -50,9 +51,10 @@ describe "ProjectHistoryRedisManager", -> @rawUpdate = pathname: @pathname = '/old' newPathname: @newPathname = '/new' + version: @version = 2 @ProjectHistoryRedisManager.queueOps = sinon.stub() - @ProjectHistoryRedisManager.queueRenameEntity @project_id, 'file', @file_id, @user_id, @rawUpdate, @callback + @ProjectHistoryRedisManager.queueRenameEntity @project_id, @projectHistoryId, 'file', @file_id, @user_id, @rawUpdate, @callback it "should queue an update", -> update = @@ -61,6 +63,8 @@ describe "ProjectHistoryRedisManager", -> meta: user_id: @user_id ts: new Date() + version: @version + projectHistoryId: @projectHistoryId file: @file_id @ProjectHistoryRedisManager.queueOps @@ -75,10 +79,11 @@ describe "ProjectHistoryRedisManager", -> @rawUpdate = pathname: @pathname = '/old' docLines: @docLines = 'a\nb' + version: @version = 2 url: @url = 'filestore.example.com' @ProjectHistoryRedisManager.queueOps = sinon.stub() - @ProjectHistoryRedisManager.queueAddEntity @project_id, 'doc', @doc_id, @user_id, @rawUpdate, @callback + @ProjectHistoryRedisManager.queueAddEntity @project_id, @projectHistoryId, 'doc', @doc_id, @user_id, @rawUpdate, @callback it "should queue an update", -> update = @@ -88,6 +93,8 @@ describe "ProjectHistoryRedisManager", -> meta: user_id: @user_id ts: new Date() + version: @version + projectHistoryId: @projectHistoryId doc: @doc_id @ProjectHistoryRedisManager.queueOps diff --git a/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/updateProjectTests.coffee index 96d2ccc07b..3ed0109be7 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", -> done: sinon.stub() @project_id = "project-id-123" + @projectHistoryId = 'history-id-123' @user_id = "user-id-123" @version = 1234567 @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(false) @@ -27,7 +28,6 @@ describe "ProjectManager", -> describe "updateProjectWithLocks", -> describe "rename operations", -> beforeEach -> - @firstDocUpdate = id: 1 pathname: 'foo' @@ -47,22 +47,22 @@ describe "ProjectManager", -> describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @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, firstDocUpdateWithVersion) + .calledWith(@project_id, @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion, @projectHistoryId) .should.equal true @DocumentManager.renameDocWithLock - .calledWith(@project_id, @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) + .calledWith(@project_id, @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion, @projectHistoryId) .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, firstFileUpdateWithVersion) + .calledWith(@project_id, @projectHistoryId, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) .should.equal true it "should not flush the history", -> @@ -77,7 +77,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @DocumentManager.renameDocWithLock = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -86,7 +86,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueRenameEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -94,7 +94,7 @@ describe "ProjectManager", -> describe "with enough ops to flush", -> beforeEach -> @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should flush the history", -> @HistoryManager.flushProjectChangesAsync @@ -121,26 +121,26 @@ describe "ProjectManager", -> describe "successfully", -> beforeEach -> - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @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.getCall(0) - .calledWith(@project_id, 'doc', @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion) + .calledWith(@project_id, @projectHistoryId, 'doc', @firstDocUpdate.id, @user_id, firstDocUpdateWithVersion) .should.equal true @ProjectHistoryRedisManager.queueAddEntity.getCall(1) - .calledWith(@project_id, 'doc', @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) + .calledWith(@project_id, @projectHistoryId, 'doc', @secondDocUpdate.id, @user_id, secondDocUpdateWithVersion) .should.equal true it "should add the files in the updates", -> firstFileUpdateWithVersion = _.extend({}, @firstFileUpdate, {version: "#{@version}.2"}) secondFileUpdateWithVersion = _.extend({}, @secondFileUpdate, {version: "#{@version}.3"}) @ProjectHistoryRedisManager.queueAddEntity.getCall(2) - .calledWith(@project_id, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) + .calledWith(@project_id, @projectHistoryId, 'file', @firstFileUpdate.id, @user_id, firstFileUpdateWithVersion) .should.equal true @ProjectHistoryRedisManager.queueAddEntity.getCall(3) - .calledWith(@project_id, 'file', @secondFileUpdate.id, @user_id, secondFileUpdateWithVersion) + .calledWith(@project_id, @projectHistoryId, 'file', @secondFileUpdate.id, @user_id, secondFileUpdateWithVersion) .should.equal true it "should not flush the history", -> @@ -155,7 +155,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueAddEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -164,7 +164,7 @@ describe "ProjectManager", -> beforeEach -> @error = new Error('error') @ProjectHistoryRedisManager.queueAddEntity = sinon.stub().yields(@error) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true @@ -172,7 +172,7 @@ describe "ProjectManager", -> describe "with enough ops to flush", -> beforeEach -> @HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true) - @ProjectManager.updateProjectWithLocks @project_id, @user_id, @docUpdates, @fileUpdates, @version, @callback + @ProjectManager.updateProjectWithLocks @project_id, @projectHistoryId, @user_id, @docUpdates, @fileUpdates, @version, @callback it "should flush the history", -> @HistoryManager.flushProjectChangesAsync diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index a3b28d00cb..e2263c21f4 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -33,6 +33,7 @@ describe "RedisManager", -> docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" pathname: ({doc_id}) -> "Pathname:#{doc_id}" + projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" history: @@ -59,6 +60,7 @@ describe "RedisManager", -> @doc_id = "doc-id-123" @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @callback = sinon.stub() describe "getDoc", -> @@ -72,7 +74,7 @@ describe "RedisManager", -> @unflushed_time = 12345 @pathname = '/a/b/c.tex' @multi.get = sinon.stub() - @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @unflushed_time]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @projectHistoryId, @unflushed_time]) @rclient.sadd = sinon.stub().yields(null, 0) describe "successfully", -> @@ -109,6 +111,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}") .should.equal true + it "should get the projectHistoryId", -> + @multi.get + .calledWith("ProjectHistoryId:#{@doc_id}") + .should.equal true + it "should check if the document is in the DocsIn set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}") @@ -116,7 +123,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) .should.equal true it 'should not log any errors', -> @@ -125,7 +132,7 @@ describe "RedisManager", -> describe "when the document is not present", -> beforeEach -> - @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null]) + @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null, null]) @rclient.sadd = sinon.stub().yields() @RedisManager.getDoc @project_id, @doc_id, @callback @@ -136,7 +143,7 @@ describe "RedisManager", -> it 'should return an empty result', -> @callback - .calledWithExactly(null, null, 0, {}, null, null) + .calledWithExactly(null, null, 0, {}, null, null, null) .should.equal true it 'should not log any errors', -> @@ -154,7 +161,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) .should.equal true describe "with a corrupted document", -> @@ -532,7 +539,7 @@ describe "RedisManager", -> describe "with non-empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, done it "should set the lines", -> @multi.eval @@ -564,6 +571,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}", @pathname) .should.equal true + it "should set the projectHistoryId for the doc", -> + @multi.set + .calledWith("ProjectHistoryId:#{@doc_id}", @projectHistoryId) + .should.equal true + it "should add the doc_id to the project set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}", @doc_id) @@ -575,7 +587,7 @@ describe "RedisManager", -> describe "with empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, @pathname, @projectHistoryId, done it "should delete the ranges key", -> @multi.del @@ -590,7 +602,7 @@ describe "RedisManager", -> describe "with a corrupted write", -> beforeEach (done) -> @multi.exec = sinon.stub().callsArgWith(0, null, ["INVALID-HASH-VALUE"]) - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, done it 'should log a hash error', -> @logger.error.calledWith() @@ -600,7 +612,7 @@ describe "RedisManager", -> beforeEach -> @_stringify = JSON.stringify @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, @callback afterEach -> @JSON.stringify = @_stringify @@ -614,7 +626,7 @@ describe "RedisManager", -> describe "with ranges that are too big", -> beforeEach -> @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, @callback it 'should log an error', -> @logger.error.called.should.equal true @@ -664,6 +676,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}") .should.equal true + it "should delete the pathname for the doc", -> + @multi.del + .calledWith("ProjectHistoryId:#{@doc_id}") + .should.equal true + describe "clearProjectState", -> beforeEach (done) -> @rclient.del = sinon.stub().callsArg(1) @@ -687,7 +704,7 @@ describe "RedisManager", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, 'lines', 'version') @ProjectHistoryRedisManager.queueRenameEntity = sinon.stub().yields() - @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @callback + @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @projectHistoryId, @callback it "update the cached pathname", -> @rclient.set @@ -696,19 +713,19 @@ describe "RedisManager", -> it "should queue an update", -> @ProjectHistoryRedisManager.queueRenameEntity - .calledWithExactly(@project_id, 'doc', @doc_id, @userId, @update, @callback) + .calledWithExactly(@project_id, @projectHistoryId, 'doc', @doc_id, @userId, @update, @callback) .should.equal true describe "the document is not cached in redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null) @ProjectHistoryRedisManager.queueRenameEntity = sinon.stub().yields() - @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @callback + @RedisManager.renameDoc @project_id, @doc_id, @userId, @update, @projectHistoryId, @callback it "does not update the cached pathname", -> @rclient.set.called.should.equal false it "should queue an update", -> @ProjectHistoryRedisManager.queueRenameEntity - .calledWithExactly(@project_id, 'doc', @doc_id, @userId, @update, @callback) + .calledWithExactly(@project_id, @projectHistoryId, 'doc', @doc_id, @userId, @update, @callback) .should.equal true diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index e91c35f7e6..383bd1848e 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -7,6 +7,7 @@ SandboxedModule = require('sandboxed-module') describe "UpdateManager", -> beforeEach -> @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @doc_id = "document-id-123" @callback = sinon.stub() @UpdateManager = SandboxedModule.require modulePath, requires: @@ -167,7 +168,7 @@ describe "UpdateManager", -> @doc_ops_length = sinon.stub() @project_ops_length = sinon.stub() @pathname = '/a/b/c.tex' - @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges, @pathname) + @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) @RedisManager.updateDocument = sinon.stub().yields(null, @doc_ops_length, @project_ops_length) @@ -196,7 +197,7 @@ describe "UpdateManager", -> it "should add metadata to the ops" , -> @UpdateManager._addProjectHistoryMetadataToOps - .calledWith(@appliedOps, @pathname, @lines) + .calledWith(@appliedOps, @pathname, @projectHistoryId, @lines) .should.equal true it "should push the applied ops into the history queue", -> @@ -239,7 +240,7 @@ describe "UpdateManager", -> @callback.calledWith(@error).should.equal true describe "_addProjectHistoryMetadataToOps", -> - it "should add pathname and doc_length metadata to the ops", -> + it "should add projectHistoryId, pathname and doc_length metadata to the ops", -> lines = [ 'some' 'test' @@ -250,20 +251,23 @@ describe "UpdateManager", -> { v: 45, op: [{d: "qux", p: 4}, { i: "bazbaz", p: 14 }] }, { v: 49, op: [{i: "penguin", p: 18}] } ] - @UpdateManager._addProjectHistoryMetadataToOps(appliedOps, @pathname, lines) + @UpdateManager._addProjectHistoryMetadataToOps(appliedOps, @pathname, @projectHistoryId, lines) appliedOps.should.deep.equal [{ + projectHistoryId: @projectHistoryId v: 42 op: [{i: "foo", p: 4}, { i: "bar", p: 6 }] meta: pathname: @pathname doc_length: 14 }, { + projectHistoryId: @projectHistoryId v: 45 op: [{d: "qux", p: 4}, { i: "bazbaz", p: 14 }] meta: pathname: @pathname doc_length: 20 # 14 + 'foo' + 'bar' }, { + projectHistoryId: @projectHistoryId v: 49 op: [{i: "penguin", p: 18}] meta: From af92ca70a17de24e3ff4c82720ae93ca8e698d0e Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 23 Apr 2018 15:19:06 +0100 Subject: [PATCH 2/2] coerce projectHistoryId to integer after reading from Redis --- services/document-updater/app/coffee/RedisManager.coffee | 3 +++ .../test/unit/coffee/RedisManager/RedisManagerTests.coffee | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index a940970176..25dbafc6e7 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -148,6 +148,9 @@ module.exports = RedisManager = logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc not in project" return callback(new Errors.NotFoundError("document not found")) + if projectHistoryId? + projectHistoryId = parseInt(projectHistoryId) + # doc is not in redis, bail out if !docLines? return callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index e2263c21f4..42d06d743a 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -60,7 +60,7 @@ describe "RedisManager", -> @doc_id = "doc-id-123" @project_id = "project-id-123" - @projectHistoryId = "history-id-123" + @projectHistoryId = 123 @callback = sinon.stub() describe "getDoc", -> @@ -74,7 +74,7 @@ describe "RedisManager", -> @unflushed_time = 12345 @pathname = '/a/b/c.tex' @multi.get = sinon.stub() - @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @projectHistoryId, @unflushed_time]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @projectHistoryId.toString(), @unflushed_time]) @rclient.sadd = sinon.stub().yields(null, 0) describe "successfully", -> @@ -111,7 +111,7 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}") .should.equal true - it "should get the projectHistoryId", -> + it "should get the projectHistoryId as an integer", -> @multi.get .calledWith("ProjectHistoryId:#{@doc_id}") .should.equal true