diff --git a/services/document-updater/Jenkinsfile b/services/document-updater/Jenkinsfile index 80f552dbea..3d51dfcb43 100644 --- a/services/document-updater/Jenkinsfile +++ b/services/document-updater/Jenkinsfile @@ -41,7 +41,7 @@ pipeline { stage('Acceptance Tests') { steps { sh 'docker pull sharelatex/acceptance-test-runner' - sh 'docker run --rm -v $(pwd):/app sharelatex/acceptance-test-runner' + sh 'docker run --rm -e SHARELATEX_ENABLE_PROJECT_HISTORY=true -v $(pwd):/app sharelatex/acceptance-test-runner' } } stage('Package') { diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 21251933d2..5ddca2e6a8 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -11,31 +11,31 @@ RangesManager = require "./RangesManager" 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, unflushedTime, alreadyLoaded) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) ->) -> timer = new Metrics.Timer("docManager.getDoc") callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, unflushedTime) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, 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) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> return callback(error) if error? logger.log {project_id, doc_id, lines, version}, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, (error) -> + RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, (error) -> return callback(error) if error? - callback null, lines, version, ranges, null, false + callback null, lines, version, ranges, pathname, null, false else - callback null, lines, version, ranges, unflushedTime, true + callback null, lines, version, ranges, pathname, unflushedTime, true getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps, ranges) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") callback = (args...) -> timer.done() _callback(args...) - + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? if fromVersion == -1 @@ -55,9 +55,9 @@ 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, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> return callback(error) if error? - + if oldLines? and oldLines.length > 0 and oldLines[0].text? logger.log doc_id: doc_id, project_id: project_id, oldLines: oldLines, newLines: newLines, "document is JSON so not updating" return callback(null) @@ -115,12 +115,9 @@ module.exports = DocumentManager = DocumentManager.flushDocIfLoaded project_id, doc_id, (error) -> return callback(error) if error? - - # Flush in the background since it requires and http request - # to track changes - HistoryManager.flushDocChanges project_id, doc_id, (err) -> - if err? - logger.err {err, project_id, doc_id}, "error flushing to track changes" + + # Flush in the background since it requires a http request + HistoryManager.flushChangesAsync project_id, doc_id RedisManager.removeDocFromMemory project_id, doc_id, (error) -> return callback(error) if error? @@ -138,10 +135,10 @@ module.exports = DocumentManager = return callback(new Errors.NotFoundError("document not found: #{doc_id}")) RangesManager.acceptChanges change_ids, ranges, (error, new_ranges) -> return callback(error) if error? - RedisManager.updateDocument doc_id, lines, version, [], new_ranges, (error) -> + RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, (error) -> return callback(error) if error? callback() - + deleteComment: (project_id, doc_id, comment_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.deleteComment") callback = (args...) -> @@ -154,12 +151,12 @@ module.exports = DocumentManager = return callback(new Errors.NotFoundError("document not found: #{doc_id}")) RangesManager.deleteComment comment_id, ranges, (error, new_ranges) -> return callback(error) if error? - RedisManager.updateDocument doc_id, lines, version, [], new_ranges, (error) -> + RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, (error) -> return callback(error) if error? callback() getDocAndFlushIfOld: (project_id, doc_id, callback = (error, doc) ->) -> - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, 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 @@ -172,7 +169,7 @@ module.exports = DocumentManager = 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) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback @@ -184,7 +181,7 @@ module.exports = DocumentManager = setDocWithLock: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, undoing, callback - + flushDocIfLoadedWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.flushDocIfLoaded, project_id, doc_id, callback diff --git a/services/document-updater/app/coffee/HistoryManager.coffee b/services/document-updater/app/coffee/HistoryManager.coffee index c3675f4018..daf558c2e2 100644 --- a/services/document-updater/app/coffee/HistoryManager.coffee +++ b/services/document-updater/app/coffee/HistoryManager.coffee @@ -1,44 +1,72 @@ -settings = require "settings-sharelatex" +Settings = require "settings-sharelatex" request = require "request" logger = require "logger-sharelatex" HistoryRedisManager = require "./HistoryRedisManager" module.exports = HistoryManager = - flushDocChanges: (project_id, doc_id, callback = (error) ->) -> - if !settings.apis?.trackchanges? - logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing" - return callback() + flushChangesAsync: (project_id, doc_id) -> + HistoryManager._flushDocChangesAsync project_id, doc_id + if Settings.apis?.project_history?.enabled + HistoryManager._flushProjectChangesAsync project_id - url = "#{settings.apis.trackchanges.url}/project/#{project_id}/doc/#{doc_id}/flush" - logger.log project_id: project_id, doc_id: doc_id, url: url, "flushing doc in track changes api" + _flushDocChangesAsync: (project_id, doc_id) -> + if !Settings.apis?.trackchanges? + logger.warn { doc_id }, "track changes API is not configured, so not flushing" + return + + url = "#{Settings.apis.trackchanges.url}/project/#{project_id}/doc/#{doc_id}/flush" + logger.log { project_id, doc_id, url }, "flushing doc in track changes api" request.post url, (error, res, body)-> if error? - return callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - return callback(null) - else - error = new Error("track changes api returned a failure status code: #{res.statusCode}") - return callback(error) + logger.error( + { error, doc_id, project_id}, + "track changes doc to track changes api" + ) + else if res.statusCode < 200 and res.statusCode >= 300 + logger.error( + { doc_id, project_id }, + "track changes api returned a failure status code: #{res.statusCode}" + ) + + _flushProjectChangesAsync: (project_id) -> + return if !Settings.apis?.project_history? + + url = "#{Settings.apis.project_history.url}/project/#{project_id}/flush" + logger.log { project_id, url }, "flushing doc in project history api" + request.post url, (error, res, body)-> + if error? + logger.error { error, project_id}, "project history doc to track changes api" + else if res.statusCode < 200 and res.statusCode >= 300 + logger.error { project_id }, "project history api returned a failure status code: #{res.statusCode}" FLUSH_EVERY_N_OPS: 100 - recordAndFlushHistoryOps: (project_id, doc_id, ops = [], length, callback = (error) ->) -> + recordAndFlushHistoryOps: (project_id, doc_id, ops = [], doc_ops_length, project_ops_length, callback = (error) ->) -> if ops.length == 0 return callback() - HistoryRedisManager.recordDocHasHistoryOps project_id, doc_id, ops, (error) -> - return callback(error) if error? - return callback() if not length? # don't flush unless we know the length - # We want to flush every 100 ops, i.e. 100, 200, 300, etc - # Find out which 'block' (i.e. 0-99, 100-199) we were in before and after pushing these - # ops. If we've changed, then we've gone over a multiple of 100 and should flush. - # (Most of the time, we will only hit 100 and then flushing will put us back to 0) - previousLength = length - ops.length - prevBlock = Math.floor(previousLength / HistoryManager.FLUSH_EVERY_N_OPS) - newBlock = Math.floor(length / HistoryManager.FLUSH_EVERY_N_OPS) - if newBlock != prevBlock + + if Settings.apis?.project_history?.enabled + if HistoryManager._shouldFlushHistoryOps(project_ops_length, ops, HistoryManager.FLUSH_EVERY_N_OPS) # Do this in the background since it uses HTTP and so may be too # slow to wait for when processing a doc update. - logger.log length: length, doc_id: doc_id, project_id: project_id, "flushing track changes api" - HistoryManager.flushDocChanges project_id, doc_id, (error) -> - if error? - logger.error err: error, doc_id: doc_id, project_id: project_id, "error flushing doc to track changes api" + logger.log { project_ops_length, project_id }, "flushing project history api" + HistoryManager._flushProjectChangesAsync project_id + + HistoryRedisManager.recordDocHasHistoryOps project_id, doc_id, ops, (error) -> + return callback(error) if error? + if HistoryManager._shouldFlushHistoryOps(doc_ops_length, ops, HistoryManager.FLUSH_EVERY_N_OPS) + # Do this in the background since it uses HTTP and so may be too + # slow to wait for when processing a doc update. + logger.log { doc_ops_length, doc_id, project_id }, "flushing track changes api" + HistoryManager._flushDocChangesAsync project_id, doc_id callback() + + _shouldFlushHistoryOps: (length, ops, threshold) -> + return false if !length # don't flush unless we know the length + # We want to flush every 100 ops, i.e. 100, 200, 300, etc + # Find out which 'block' (i.e. 0-99, 100-199) we were in before and after pushing these + # ops. If we've changed, then we've gone over a multiple of 100 and should flush. + # (Most of the time, we will only hit 100 and then flushing will put us back to 0) + previousLength = length - ops.length + prevBlock = Math.floor(previousLength / threshold) + newBlock = Math.floor(length / threshold) + return newBlock != prevBlock diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index 1e168f5495..974dec5a2c 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) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -42,7 +42,7 @@ module.exports = PersistenceManager = return callback(new Error("web API response had no doc lines")) if !body.version? or not body.version instanceof Number return callback(new Error("web API response had no valid doc version")) - return callback null, body.lines, body.version, body.ranges + return callback null, body.lines, body.version, body.ranges, body.pathname else if res.statusCode == 404 return callback(new Errors.NotFoundError("doc not not found: #{url}")) else diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index ccb82a1ab6..82125310b4 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -31,11 +31,12 @@ MAX_RANGES_SIZE = 3 * MEGABYTES keys = Settings.redis.documentupdater.key_schema historyKeys = Settings.redis.history.key_schema +projectHistoryKeys = Settings.redis.project_history.key_schema module.exports = RedisManager = rclient: rclient - putDocInMemory : (project_id, doc_id, docLines, version, ranges, _callback)-> + putDocInMemory : (project_id, doc_id, docLines, version, ranges, pathname, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> timer.done() @@ -60,6 +61,7 @@ module.exports = RedisManager = multi.set keys.ranges(doc_id:doc_id), ranges else multi.del keys.ranges(doc_id:doc_id) + multi.set keys.pathname(doc_id:doc_id), pathname multi.exec (error, result) -> return callback(error) if error? # check the hash computed on the redis server @@ -85,6 +87,7 @@ module.exports = RedisManager = multi.del keys.docVersion(doc_id:doc_id) 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.unflushedTime(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? @@ -105,7 +108,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, unflushedTime) ->)-> + getDoc : (project_id, doc_id, callback = (error, lines, version, ranges, pathname, unflushedTime) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi() multi.get keys.docLines(doc_id:doc_id) @@ -113,8 +116,9 @@ module.exports = RedisManager = multi.get keys.docHash(doc_id:doc_id) 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.unflushedTime(doc_id:doc_id) - multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, unflushedTime])-> + multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, unflushedTime])-> timeSpan = timer.done() return callback(error) if error? # check if request took too long and bail out. only do this for @@ -143,14 +147,14 @@ module.exports = RedisManager = # doc is not in redis, bail out if !docLines? - return callback null, docLines, version, ranges + return callback null, docLines, version, ranges, pathname, 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, unflushedTime + callback null, docLines, version, ranges, pathname, unflushedTime getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -200,7 +204,7 @@ module.exports = RedisManager = DOC_OPS_TTL: 60 * minutes DOC_OPS_MAX_LENGTH: 100 - updateDocument : (doc_id, docLines, newVersion, appliedOps = [], ranges, callback = (error) ->)-> + updateDocument : (project_id, doc_id, docLines, newVersion, appliedOps = [], ranges, callback = (error) ->)-> RedisManager.getDocVersion doc_id, (error, currentVersion) -> return callback(error) if error? if currentVersion + appliedOps.length != newVersion @@ -258,9 +262,16 @@ module.exports = RedisManager = writeHash = result?[0] if logHashWriteErrors and writeHash? and writeHash isnt newHash logger.error doc_id: doc_id, writeHash: writeHash, origHash: newHash, docLines:newDocLines, "hash mismatch on updateDocument" - # return length of uncompressedHistoryOps queue (index 7) - uncompressedHistoryOpsLength = result?[7] - return callback(null, uncompressedHistoryOpsLength) + + # length of uncompressedHistoryOps queue (index 7) + docUpdateCount = result[7] + + if jsonOps.length > 0 && Settings.apis?.project_history?.enabled + rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), jsonOps..., (error, projectUpdateCount) -> + callback null, docUpdateCount, projectUpdateCount + else + callback null, docUpdateCount + clearUnflushedTime: (doc_id, callback = (error) ->) -> rclient.del keys.unflushedTime(doc_id:doc_id), callback @@ -276,7 +287,7 @@ module.exports = RedisManager = # Most doc will have empty ranges so don't fill redis with lots of '{}' keys jsonRanges = null return callback null, jsonRanges - + _deserializeRanges: (ranges) -> if !ranges? or ranges == "" return {} diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 3a69761977..5064725aa0 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -4,6 +4,7 @@ RealTimeRedisManager = require "./RealTimeRedisManager" ShareJsUpdateManager = require "./ShareJsUpdateManager" HistoryManager = require "./HistoryManager" Settings = require('settings-sharelatex') +_ = require("underscore") async = require("async") logger = require('logger-sharelatex') Metrics = require "./Metrics" @@ -69,7 +70,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) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> profile.log("getDoc") return callback(error) if error? if !lines? or !version? @@ -78,15 +79,15 @@ 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, updatedDocLines) profile.log("RangesManager.applyUpdate") return callback(error) if error? - RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_ranges, (error, historyOpsLength) -> + RedisManager.updateDocument project_id, doc_id, updatedDocLines, version, appliedOps, new_ranges, (error, doc_ops_length, project_ops_length) -> profile.log("RedisManager.updateDocument") return callback(error) if error? - HistoryManager.recordAndFlushHistoryOps project_id, doc_id, appliedOps, historyOpsLength, (error) -> + HistoryManager.recordAndFlushHistoryOps project_id, doc_id, appliedOps, doc_ops_length, project_ops_length, (error) -> profile.log("recordAndFlushHistoryOps") - return callback(error) if error? - callback() + callback(error) lockUpdatesAndDo: (method, project_id, doc_id, args..., callback) -> profile = new Profiler("lockUpdatesAndDo", {project_id, doc_id}) @@ -109,15 +110,15 @@ module.exports = UpdateManager = _handleErrorInsideLock: (doc_id, lockValue, original_error, callback = (error) ->) -> LockManager.releaseLock doc_id, lockValue, (lock_error) -> callback(original_error) - + _sanitizeUpdate: (update) -> # In Javascript, characters are 16-bits wide. It does not understand surrogates as characters. - # + # # From Wikipedia (http://en.wikipedia.org/wiki/Plane_(Unicode)#Basic_Multilingual_Plane): # "The High Surrogates (U+D800–U+DBFF) and Low Surrogate (U+DC00–U+DFFF) codes are reserved # for encoding non-BMP characters in UTF-16 by using a pair of 16-bit codes: one High Surrogate # and one Low Surrogate. A single surrogate code point will never be assigned a character."" - # + # # The main offender seems to be \uD835 as a stand alone character, which would be the first # 16-bit character of a blackboard bold character (http://www.fileformat.info/info/unicode/char/1d400/index.htm). # Something must be going on client side that is screwing up the encoding and splitting the @@ -128,3 +129,12 @@ module.exports = UpdateManager = op.i = op.i.replace(/[\uD800-\uDFFF]/g, "\uFFFD") return update + _addProjectHistoryMetadataToOps: (ops, pathname, lines) -> + doc_length = _.reduce lines, + (chars, line) -> chars + line.length, + 0 + doc_length += lines.length - 1 # count newline characters + ops.forEach (op) -> + op.meta ||= {} + op.meta.pathname = pathname + op.meta.doc_length = doc_length diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index 9b800f2729..fee3d614c2 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -14,6 +14,9 @@ module.exports = pass: "password" trackchanges: url: "http://localhost:3015" + project_history: + enabled: process.env.SHARELATEX_ENABLE_PROJECT_HISTORY == 'true' + url: "http://localhost:3054" redis: realtime: @@ -41,6 +44,7 @@ module.exports = projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" + pathname: ({doc_id}) -> "Pathname:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" # cluster: [{ @@ -64,6 +68,10 @@ module.exports = key_schema: uncompressedHistoryOps: ({doc_id}) -> "UncompressedHistoryOps:#{doc_id}" docsWithHistoryOps: ({project_id}) -> "DocsWithHistoryOps:#{project_id}" + + project_history: + key_schema: + projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" # cluster: [{ # port: "7000" # host: "localhost" diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index a2eba4c063..29a17c04ea 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -8,6 +8,7 @@ rclient_history = require("redis-sharelatex").createClient(Settings.redis.histor rclient_du = require("redis-sharelatex").createClient(Settings.redis.documentupdater) Keys = Settings.redis.documentupdater.key_schema HistoryKeys = Settings.redis.history.key_schema +ProjectHistoryKeys = Settings.redis.project_history.key_schema MockTrackChangesApi = require "./helpers/MockTrackChangesApi" MockWebApi = require "./helpers/MockWebApi" @@ -58,6 +59,11 @@ describe "Applying updates to a doc", -> result.should.equal 1 done() + it "should push the applied updates to the project history changes api", (done) -> + rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) => + throw error if error? + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() describe "when the document is loaded", -> before (done) -> @@ -89,6 +95,12 @@ describe "Applying updates to a doc", -> result.should.equal 1 done() + it "should push the applied updates to the project history changes api", (done) -> + rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) => + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() + + describe "when the document has been deleted", -> describe "when the ops come in a single linear order", -> before (done) -> @@ -136,7 +148,7 @@ describe "Applying updates to a doc", -> rclient_history.sismember HistoryKeys.docsWithHistoryOps({@project_id}), @doc_id, (error, result) => result.should.equal 1 done() - + it "should store the doc ops in the correct order", (done) -> rclient_du.lrange Keys.docOps({doc_id: @doc_id}), 0, -1, (error, updates) => updates = (JSON.parse(u) for u in updates) @@ -181,7 +193,7 @@ describe "Applying updates to a doc", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @broken_update = { doc_id: @doc_id, v: @version, op: [d: "not the correct content", p: 0 ] } MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} - + DocUpdaterClient.subscribeToAppliedOps @messageCallback = sinon.stub() DocUpdaterClient.sendUpdate @project_id, @doc_id, @broken_update, (error) -> @@ -192,14 +204,14 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @lines done() - + it "should send a message with an error", -> @messageCallback.called.should.equal true [channel, message] = @messageCallback.args[0] channel.should.equal "applied-ops" JSON.parse(message).should.deep.equal { project_id: @project_id, - doc_id: @doc_id, + doc_id: @doc_id, error:'Delete component \'not the correct content\' does not match deleted text \'one\ntwo\nthree\'' } @@ -240,7 +252,7 @@ describe "Applying updates to a doc", -> lines: @lines } - update = + update = doc: @doc_id op: @update.op v: 0 @@ -252,12 +264,12 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @result done() - + describe "when the sending duplicate ops", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} - + DocUpdaterClient.subscribeToAppliedOps @messageCallback = sinon.stub() # One user delete 'one', the next turns it into 'once'. The second becomes a NOP. @@ -292,7 +304,7 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @result done() - + it "should return a message about duplicate ops", -> @messageCallback.calledTwice.should.equal true @messageCallback.args[0][0].should.equal "applied-ops" diff --git a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee index 291b627a3e..2be5b01245 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee @@ -3,6 +3,7 @@ chai = require("chai") chai.should() MockTrackChangesApi = require "./helpers/MockTrackChangesApi" +MockProjectHistoryApi = require "./helpers/MockProjectHistoryApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -18,11 +19,13 @@ describe "Deleting a document", -> }] v: @version @result = ["one", "one and a half", "two", "three"] - + sinon.spy MockTrackChangesApi, "flushDoc" - + sinon.spy MockProjectHistoryApi, "flushProject" + after -> MockTrackChangesApi.flushDoc.restore() + MockProjectHistoryApi.flushProject.restore() describe "when the updated doc exists in the doc updater", -> before (done) -> @@ -60,10 +63,13 @@ describe "Deleting a document", -> .calledWith(@project_id, @doc_id) .should.equal true done() - + it "should flush track changes", -> MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true + it "should flush project history", -> + MockProjectHistoryApi.flushProject.calledWith(@project_id).should.equal true + describe "when the doc is not in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -93,9 +99,9 @@ describe "Deleting a document", -> .calledWith(@project_id, @doc_id) .should.equal true done() - + it "should flush track changes", -> MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true - - + it "should flush project history", -> + MockProjectHistoryApi.flushProject.calledWith(@project_id).should.equal true diff --git a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee index 27d241d97d..3b889f79ea 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee @@ -4,6 +4,7 @@ chai.should() async = require "async" MockTrackChangesApi = require "./helpers/MockTrackChangesApi" +MockProjectHistoryApi = require "./helpers/MockProjectHistoryApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -38,11 +39,13 @@ describe "Deleting a project", -> lines: doc.lines version: doc.update.v } - + sinon.spy MockTrackChangesApi, "flushDoc" - + sinon.spy MockProjectHistoryApi, "flushProject" + after -> MockTrackChangesApi.flushDoc.restore() + MockProjectHistoryApi.flushProject.restore() describe "with documents which have been updated", -> before (done) -> @@ -84,9 +87,10 @@ describe "Deleting a project", -> ), () -> MockWebApi.getDocument.restore() done() - + it "should flush each doc in track changes", -> for doc in @docs MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal true - + it "should flush each doc in project history", -> + MockProjectHistoryApi.flushProject.calledWith(@project_id).should.equal true diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index 97fae5cf14..68ed5483c9 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -7,6 +7,7 @@ rclient_du = require("redis-sharelatex").createClient(Settings.redis.documentupd Keys = Settings.redis.documentupdater.key_schema MockTrackChangesApi = require "./helpers/MockTrackChangesApi" +MockProjectHistoryApi = require "./helpers/MockProjectHistoryApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -25,14 +26,16 @@ describe "Setting a document", -> @newLines = ["these", "are", "the", "new", "lines"] @source = "dropbox" @user_id = "user-id-123" - + sinon.spy MockTrackChangesApi, "flushDoc" + sinon.spy MockProjectHistoryApi, "flushProject" sinon.spy MockWebApi, "setDocument" - + after -> - MockWebApi.setDocument.restore() MockTrackChangesApi.flushDoc.restore() - + MockProjectHistoryApi.flushProject.restore() + MockWebApi.setDocument.restore() + describe "when the updated doc exists in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -64,13 +67,13 @@ describe "Setting a document", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.version.should.equal @version + 2 done() - + it "should leave the document in redis", (done) -> rclient_du.get Keys.docLines({doc_id: @doc_id}), (error, lines) => throw error if error? expect(JSON.parse(lines)).to.deep.equal @newLines done() - + describe "when the updated doc does not exist in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -78,7 +81,7 @@ describe "Setting a document", -> DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) => @statusCode = res.statusCode setTimeout done, 200 - + it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -86,16 +89,19 @@ describe "Setting a document", -> MockWebApi.setDocument .calledWith(@project_id, @doc_id, @newLines) .should.equal true - + it "should flush track changes", -> MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true - + + it "should flush project history", -> + MockProjectHistoryApi.flushProject.calledWith(@project_id).should.equal true + it "should remove the document from redis", (done) -> rclient_du.get Keys.docLines({doc_id: @doc_id}), (error, lines) => throw error if error? expect(lines).to.not.exist done() - + describe "with track changes", -> before -> @lines = ["one", "one and a half", "two", "three"] @@ -123,14 +129,14 @@ describe "Setting a document", -> DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, true, (error, res, body) => @statusCode = res.statusCode setTimeout done, 200 - + it "should undo the tracked changes", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => throw error if error? ranges = data.ranges expect(ranges.changes).to.be.undefined done() - + describe "without the undo flag", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -143,7 +149,7 @@ describe "Setting a document", -> DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, false, (error, res, body) => @statusCode = res.statusCode setTimeout done, 200 - + it "should not undo the tracked changes", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => throw error if error? @@ -151,4 +157,4 @@ describe "Setting a document", -> expect(ranges.changes.length).to.equal 1 done() - + diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee new file mode 100644 index 0000000000..2a0c8603a4 --- /dev/null +++ b/services/document-updater/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee @@ -0,0 +1,19 @@ +express = require("express") +app = express() + +module.exports = MockProjectHistoryApi = + flushProject: (doc_id, callback = (error) ->) -> + callback() + + run: () -> + app.post "/project/:project_id/flush", (req, res, next) => + @flushProject req.params.project_id, (error) -> + if error? + res.send 500 + else + res.send 204 + + app.listen 3054, (error) -> + throw error if error? + +MockProjectHistoryApi.run() diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index fabd4587e8..8523f7752e 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -9,6 +9,7 @@ module.exports = MockWebApi = insertDoc: (project_id, doc_id, doc) -> doc.version ?= 0 doc.lines ?= [] + doc.pathname = '/a/b/c.tex' @docs["#{project_id}:#{doc_id}"] = doc setDocument: (project_id, doc_id, lines, version, ranges, callback = (error) ->) -> @@ -16,6 +17,7 @@ module.exports = MockWebApi = doc.lines = lines doc.version = version doc.ranges = ranges + doc.pathname = '/a/b/c.tex' callback null getDocument: (project_id, doc_id, callback = (error, doc) ->) -> diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index aff1cf0bc6..ac0601b34b 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -27,6 +27,7 @@ describe "DocumentManager", -> @lines = ["one", "two", "three"] @version = 42 @ranges = { comments: "mock", entries: "mock" } + @pathname = '/a/b/c.tex' @unflushedTime = Date.now() describe "flushAndDeleteDoc", -> @@ -34,9 +35,9 @@ describe "DocumentManager", -> beforeEach -> @RedisManager.removeDocFromMemory = sinon.stub().callsArg(2) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2) - @HistoryManager.flushDocChanges = sinon.stub().callsArg(2) + @HistoryManager.flushChangesAsync = sinon.stub() @DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback - + it "should flush the doc", -> @DocumentManager.flushDocIfLoaded .calledWith(@project_id, @doc_id) @@ -52,12 +53,12 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + it "should flush to the history api", -> - @HistoryManager.flushDocChanges + @HistoryManager.flushChangesAsync .calledWith(@project_id, @doc_id) .should.equal true - + describe "flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> @@ -75,7 +76,7 @@ describe "DocumentManager", -> @PersistenceManager.setDoc .calledWith(@project_id, @doc_id, @lines, @version, @ranges) .should.equal true - + it "should call the callback without error", -> @callback.calledWith(null).should.equal true @@ -103,7 +104,7 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> @@ -146,28 +147,28 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @unflushedTime) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @unflushedTime) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @RedisManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, @unflushedTime, true).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, @unflushedTime, true).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true describe "when the doc does not exist in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null, null) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -183,22 +184,22 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @ranges) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname) .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, null, false).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, null, false).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "setDoc", -> describe "with plain tex lines", -> beforeEach -> @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, @unflushedTime, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, @pathname, @unflushedTime, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) @@ -240,16 +241,16 @@ describe "DocumentManager", -> @DocumentManager.flushDocIfLoaded .calledWith(@project_id, @doc_id) .should.equal true - + it "should call the callback", -> @callback.calledWith(null).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "when not already loaded", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, null, false) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @pathname, null, false) @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, false, @callback it "should flush and delete the doc from the doc updater", -> @@ -263,21 +264,21 @@ describe "DocumentManager", -> it "should return the callback with an error", -> @callback.calledWith(new Error("No lines were passed to setDoc")) - + it "should not try to get the doc lines", -> @DocumentManager.getDoc.called.should.equal false - + describe "with the undoing flag", -> beforeEach -> # Copy ops so we don't interfere with other tests @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, true, @callback - + it "should set the undo flag on each op", -> for op in @ops op.u.should.equal true - + describe "acceptChanges", -> beforeEach -> @change_id = "mock-change-id" @@ -289,33 +290,33 @@ describe "DocumentManager", -> @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) @RangesManager.acceptChanges = sinon.stub().yields(null, @updated_ranges) @RedisManager.updateDocument = sinon.stub().yields() - + describe "successfully with a single change", -> beforeEach -> @DocumentManager.acceptChanges @project_id, @doc_id, [ @change_id ], @callback - + it "should get the document's current ranges", -> @DocumentManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should apply the accept change to the ranges", -> @RangesManager.acceptChanges .calledWith([ @change_id ], @ranges) .should.equal true - + it "should save the updated ranges", -> @RedisManager.updateDocument - .calledWith(@doc_id, @lines, @version, [], @updated_ranges) + .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges) .should.equal true - + it "should call the callback", -> @callback.called.should.equal true describe "successfully with multiple changes", -> beforeEach -> @DocumentManager.acceptChanges @project_id, @doc_id, @change_ids, @callback - + it "should apply the accept change to the ranges", -> @RangesManager.acceptChanges .calledWith(@change_ids, @ranges) @@ -328,11 +329,11 @@ describe "DocumentManager", -> it "should not save anything", -> @RedisManager.updateDocument.called.should.equal false - + it "should call the callback with a not found error", -> error = new Errors.NotFoundError("document not found: #{@doc_id}") @callback.calledWith(error).should.equal true - + describe "deleteComment", -> beforeEach -> @comment_id = "mock-comment-id" @@ -343,26 +344,26 @@ describe "DocumentManager", -> @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) @RangesManager.deleteComment = sinon.stub().yields(null, @updated_ranges) @RedisManager.updateDocument = sinon.stub().yields() - + describe "successfully", -> beforeEach -> @DocumentManager.deleteComment @project_id, @doc_id, @comment_id, @callback - + it "should get the document's current ranges", -> @DocumentManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should delete the comment from the ranges", -> @RangesManager.deleteComment .calledWith(@comment_id, @ranges) .should.equal true - + it "should save the updated ranges", -> @RedisManager.updateDocument - .calledWith(@doc_id, @lines, @version, [], @updated_ranges) + .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges) .should.equal true - + it "should call the callback", -> @callback.called.should.equal true @@ -373,7 +374,7 @@ describe "DocumentManager", -> it "should not save anything", -> @RedisManager.updateDocument.called.should.equal false - + it "should call the callback with a not found error", -> error = new Errors.NotFoundError("document not found: #{@doc_id}") @callback.calledWith(error).should.equal true @@ -389,7 +390,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, Date.now() - 1e9, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, Date.now() - 1e9, true) @DocumentManager.getDocAndFlushIfOld @project_id, @doc_id, @callback it "should get the doc", -> @@ -407,7 +408,7 @@ describe "DocumentManager", -> describe "and has only changes that don't need to be flushed", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, Date.now() - 100, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, Date.now() - 100, true) @DocumentManager.getDocAndFlushIfOld @project_id, @doc_id, @callback it "should get the doc", -> diff --git a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee index 37e35ca285..4956a410b2 100644 --- a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee @@ -7,106 +7,171 @@ describe "HistoryManager", -> beforeEach -> @HistoryManager = SandboxedModule.require modulePath, requires: "request": @request = {} - "settings-sharelatex": @Settings = {} + "settings-sharelatex": @Settings = { + apis: + project_history: + enabled: true + url: "http://project_history.example.com" + trackchanges: + url: "http://trackchanges.example.com" + } "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "./HistoryRedisManager": @HistoryRedisManager = {} @project_id = "mock-project-id" @doc_id = "mock-doc-id" @callback = sinon.stub() - describe "flushDocChanges", -> + describe "flushChangesAsync", -> beforeEach -> - @Settings.apis = - trackchanges: url: "http://trackchanges.example.com" + @HistoryManager._flushDocChangesAsync = sinon.stub() + @HistoryManager._flushProjectChangesAsync = sinon.stub() - describe "successfully", -> - beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) - @HistoryManager.flushDocChanges @project_id, @doc_id, @callback + @HistoryManager.flushChangesAsync(@project_id, @doc_id) - it "should send a request to the track changes api", -> - @request.post - .calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush") - .should.equal true + it "flushes doc changes", -> + @HistoryManager._flushDocChangesAsync + .calledWith(@project_id, @doc_id) + .should.equal true - it "should return the callback", -> - @callback.calledWith(null).should.equal true + it "flushes project changes", -> + @HistoryManager._flushProjectChangesAsync + .calledWith(@project_id) + .should.equal true - describe "when the track changes api returns an error", -> - beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500) - @HistoryManager.flushDocChanges @project_id, @doc_id, @callback + describe "_flushDocChangesAsync", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) - it "should return the callback with an error", -> - @callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true + @HistoryManager._flushDocChangesAsync @project_id, @doc_id + + it "should send a request to the track changes api", -> + @request.post + .calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush") + .should.equal true + + describe "_flushProjectChangesAsync", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) + + @HistoryManager._flushProjectChangesAsync @project_id + + it "should send a request to the project history api", -> + @request.post + .calledWith("#{@Settings.apis.project_history.url}/project/#{@project_id}/flush") + .should.equal true describe "recordAndFlushHistoryOps", -> beforeEach -> - @ops = ["mock-ops"] - @HistoryManager.flushDocChanges = sinon.stub().callsArg(2) + @ops = [ 'mock-ops' ] + @project_ops_length = 10 + @doc_ops_length = 5 - describe "pushing the op", -> + @HistoryManager._flushProjectChangesAsync = sinon.stub() + @HistoryRedisManager.recordDocHasHistoryOps = sinon.stub().callsArg(3) + @HistoryManager._flushDocChangesAsync = sinon.stub() + + describe "with no ops", -> beforeEach -> - @HistoryRedisManager.recordDocHasHistoryOps = sinon.stub().callsArgWith(3, null) - @HistoryManager.recordAndFlushHistoryOps @project_id, @doc_id, @ops, 1, @callback + @HistoryManager.recordAndFlushHistoryOps( + @project_id, @doc_id, [], @doc_ops_length, @project_ops_length, @callback + ) - it "should push the ops into redis", -> + it "should not flush project changes", -> + @HistoryManager._flushProjectChangesAsync.called.should.equal false + + it "should not record doc has history ops", -> + @HistoryRedisManager.recordDocHasHistoryOps.called.should.equal false + + it "should not flush doc changes", -> + @HistoryManager._flushDocChangesAsync.called.should.equal false + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with enough ops to flush project changes", -> + beforeEach -> + @HistoryManager._shouldFlushHistoryOps = sinon.stub() + @HistoryManager._shouldFlushHistoryOps.withArgs(@project_ops_length).returns(true) + @HistoryManager._shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(false) + + @HistoryManager.recordAndFlushHistoryOps( + @project_id, @doc_id, @ops, @doc_ops_length, @project_ops_length, @callback + ) + + it "should flush project changes", -> + @HistoryManager._flushProjectChangesAsync + .calledWith(@project_id) + .should.equal true + + it "should record doc has history ops", -> @HistoryRedisManager.recordDocHasHistoryOps .calledWith(@project_id, @doc_id, @ops) + + it "should not flush doc changes", -> + @HistoryManager._flushDocChangesAsync.called.should.equal false + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with enough ops to flush doc changes", -> + beforeEach -> + @HistoryManager._shouldFlushHistoryOps = sinon.stub() + @HistoryManager._shouldFlushHistoryOps.withArgs(@project_ops_length).returns(false) + @HistoryManager._shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(true) + + @HistoryManager.recordAndFlushHistoryOps( + @project_id, @doc_id, @ops, @doc_ops_length, @project_ops_length, @callback + ) + + it "should not flush project changes", -> + @HistoryManager._flushProjectChangesAsync.called.should.equal false + + it "should record doc has history ops", -> + @HistoryRedisManager.recordDocHasHistoryOps + .calledWith(@project_id, @doc_id, @ops) + + it "should flush doc changes", -> + @HistoryManager._flushDocChangesAsync + .calledWith(@project_id, @doc_id) .should.equal true it "should call the callback", -> @callback.called.should.equal true - it "should not try to flush the op", -> - @HistoryManager.flushDocChanges.called.should.equal false - - describe "when we hit a multiple of FLUSH_EVERY_N_OPS ops", -> + describe "when recording doc has history ops errors", -> beforeEach -> + @error = new Error("error") @HistoryRedisManager.recordDocHasHistoryOps = - sinon.stub().callsArgWith(3, null) - @HistoryManager.recordAndFlushHistoryOps @project_id, @doc_id, @ops, 2 * @HistoryManager.FLUSH_EVERY_N_OPS,@callback + sinon.stub().callsArgWith(3, @error) - it "should tell the track changes api to flush", -> - @HistoryManager.flushDocChanges - .calledWith(@project_id, @doc_id) - .should.equal true + @HistoryManager.recordAndFlushHistoryOps( + @project_id, @doc_id, @ops, @doc_ops_length, @project_ops_length, @callback + ) - describe "when we go over a multiple of FLUSH_EVERY_N_OPS ops", -> - beforeEach -> - @ops = ["op1", "op2", "op3"] - @HistoryRedisManager.recordDocHasHistoryOps = - sinon.stub().callsArgWith(3, null) - @HistoryManager.recordAndFlushHistoryOps @project_id, @doc_id, @ops, 2 * @HistoryManager.FLUSH_EVERY_N_OPS + 1, @callback + it "should not flush doc changes", -> + @HistoryManager._flushDocChangesAsync.called.should.equal false - it "should tell the track changes api to flush", -> - @HistoryManager.flushDocChanges - .calledWith(@project_id, @doc_id) - .should.equal true + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true - describe "when HistoryManager errors", -> - beforeEach -> - @HistoryRedisManager.recordDocHasHistoryOps = - sinon.stub().callsArgWith(3, null) - @HistoryManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops")) - @HistoryManager.recordAndFlushHistoryOps @project_id, @doc_id, @ops, 2 * @HistoryManager.FLUSH_EVERY_N_OPS, @callback + describe "_shouldFlushHistoryOps", -> + it "should return false if the number of ops is not known", -> + @HistoryManager._shouldFlushHistoryOps(null, ['a', 'b', 'c'], 1).should.equal false - it "should log out the error", -> - @logger.error - .calledWith( - err: @error - doc_id: @doc_id - project_id: @project_id - "error flushing doc to track changes api" - ) - .should.equal true - - describe "with no ops", -> - beforeEach -> - @HistoryRedisManager.recordDocHasHistoryOps = sinon.stub().callsArgWith(3, null) - @HistoryManager.recordAndFlushHistoryOps @project_id, @doc_id, [], 1, @callback - - it "should not call HistoryRedisManager.recordDocHasHistoryOps", -> - @HistoryRedisManager.recordDocHasHistoryOps.called.should.equal false - + it "should return false if the updates didn't take us past the threshold", -> + # Currently there are 14 ops + # Previously we were on 11 ops + # We didn't pass over a multiple of 5 + @HistoryManager._shouldFlushHistoryOps(14, ['a', 'b', 'c'], 5).should.equal false + it "should return true if the updates took to the threshold", -> + # Currently there are 15 ops + # Previously we were on 12 ops + # We've reached a new multiple of 5 + @HistoryManager._shouldFlushHistoryOps(15, ['a', 'b', 'c'], 5).should.equal true + + it "should return true if the updates took past the threshold", -> + # Currently there are 19 ops + # Previously we were on 16 ops + # We didn't pass over a multiple of 5 + @HistoryManager._shouldFlushHistoryOps(17, ['a', 'b', 'c'], 5).should.equal true diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee index d6fa519b4c..925274ac2f 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -22,6 +22,7 @@ describe "PersistenceManager", -> @version = 42 @callback = sinon.stub() @ranges = { comments: "mock", entries: "mock" } + @pathname = '/a/b/c.tex' @Settings.apis = web: url: @url = "www.example.com" @@ -29,13 +30,14 @@ describe "PersistenceManager", -> pass: @pass = "password" describe "getDoc", -> - + describe "with a successful response from the web api", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify({ lines: @lines, version: @version, ranges: @ranges + pathname: @pathname, })) @PersistenceManager.getDoc(@project_id, @doc_id, @callback) @@ -56,7 +58,7 @@ describe "PersistenceManager", -> .should.equal true it "should call the callback with the doc lines, version and ranges", -> - @callback.calledWith(null, @lines, @version, @ranges).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -76,7 +78,7 @@ describe "PersistenceManager", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") @PersistenceManager.getDoc(@project_id, @doc_id, @callback) - + it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -87,7 +89,7 @@ describe "PersistenceManager", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") @PersistenceManager.getDoc(@project_id, @doc_id, @callback) - + it "should return an error", -> @callback.calledWith(new Error("web api error")).should.equal true @@ -155,7 +157,7 @@ describe "PersistenceManager", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) - + it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -166,7 +168,7 @@ describe "PersistenceManager", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) - + it "should return an error", -> @callback.calledWith(new Error("web api error")).should.equal true diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index bc5f382874..f5bf3843fa 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -9,16 +9,16 @@ tk = require "timekeeper" describe "RedisManager", -> beforeEach -> - @rclient = - auth: () -> - exec: sinon.stub() - @rclient.multi = () => @rclient + @multi = exec: sinon.stub() + @rclient = multi: () => @multi tk.freeze(new Date()) @RedisManager = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() } - "settings-sharelatex": { + "settings-sharelatex": @settings = { documentupdater: {logHashErrors: {write:true, read:true}} + apis: + project_history: {enabled: true} redis: documentupdater: key_schema: @@ -31,12 +31,16 @@ describe "RedisManager", -> pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" + pathname: ({doc_id}) -> "Pathname:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" history: key_schema: uncompressedHistoryOps: ({doc_id}) -> "UncompressedHistoryOps:#{doc_id}" docsWithHistoryOps: ({project_id}) -> "DocsWithHistoryOps:#{project_id}" + project_history: + key_schema: + projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" } "redis-sharelatex": createClient: () => @rclient @@ -54,7 +58,7 @@ describe "RedisManager", -> afterEach -> tk.reset() - + @doc_id = "doc-id-123" @project_id = "project-id-123" @callback = sinon.stub() @@ -68,8 +72,9 @@ describe "RedisManager", -> @ranges = { comments: "mock", entries: "mock" } @json_ranges = JSON.stringify @ranges @unflushed_time = 12345 - @rclient.get = sinon.stub() - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @unflushed_time]) + @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]) @rclient.sadd = sinon.stub().yields(null, 0) describe "successfully", -> @@ -77,30 +82,35 @@ describe "RedisManager", -> @RedisManager.getDoc @project_id, @doc_id, @callback it "should get the lines from redis", -> - @rclient.get + @multi.get .calledWith("doclines:#{@doc_id}") .should.equal true - + it "should get the version from", -> - @rclient.get + @multi.get .calledWith("DocVersion:#{@doc_id}") .should.equal true it 'should get the hash', -> - @rclient.get + @multi.get .calledWith("DocHash:#{@doc_id}") .should.equal true it "should get the ranges", -> - @rclient.get + @multi.get .calledWith("Ranges:#{@doc_id}") .should.equal true it "should get the unflushed time", -> - @rclient.get + @multi.get .calledWith("UnflushedTime:#{@doc_id}") .should.equal true + it "should get the pathname", -> + @multi.get + .calledWith("Pathname:#{@doc_id}") + .should.equal true + it "should check if the document is in the DocsIn set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}") @@ -108,7 +118,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) .should.equal true it 'should not log any errors', -> @@ -117,7 +127,7 @@ describe "RedisManager", -> describe "when the document is not present", -> beforeEach -> - @rclient.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null]) + @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null]) @rclient.sadd = sinon.stub().yields() @RedisManager.getDoc @project_id, @doc_id, @callback @@ -128,7 +138,7 @@ describe "RedisManager", -> it 'should return an empty result', -> @callback - .calledWithExactly(null, null, 0, {}) + .calledWithExactly(null, null, 0, {}, null, null) .should.equal true it 'should not log any errors', -> @@ -146,13 +156,13 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) .should.equal true describe "with a corrupted document", -> beforeEach -> @badHash = "INVALID-HASH-VALUE" - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @badHash, @project_id, @json_ranges]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @badHash, @project_id, @json_ranges]) @RedisManager.getDoc @project_id, @doc_id, @callback it 'should log a hash error', -> @@ -167,11 +177,11 @@ describe "RedisManager", -> describe "with a slow request to redis", -> beforeEach -> - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @badHash, @project_id, @json_ranges, @unflushed_time]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @badHash, @project_id, @json_ranges, @pathname, @unflushed_time]) @clock = sinon.useFakeTimers(); - @rclient.exec = (cb) => + @multi.exec = (cb) => @clock.tick(6000); - cb(null, [@jsonlines, @version, @another_project_id, @json_ranges, @unflushed_time]) + cb(null, [@jsonlines, @version, @another_project_id, @json_ranges, @pathname, @unflushed_time]) @RedisManager.getDoc @project_id, @doc_id, @callback @@ -186,7 +196,7 @@ describe "RedisManager", -> describe "getDoc with an invalid project id", -> beforeEach -> @another_project_id = "project-id-456" - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id, @json_ranges, @unflushed_time]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @another_project_id, @json_ranges, @pathname, @unflushed_time]) @RedisManager.getDoc @project_id, @doc_id, @callback it 'should return an error', -> @@ -271,7 +281,7 @@ describe "RedisManager", -> @rclient.get = sinon.stub().callsArgWith(1, null, @version.toString()) @rclient.lrange = sinon.stub().callsArgWith(3, null, @jsonOps) @RedisManager.getPreviousDocOps(@doc_id, @start, @end, @callback) - + it "should return an error", -> @callback.calledWith(new Errors.OpRangeNotAvailableError("doc ops range is not loaded in redis")).should.equal true @@ -309,128 +319,168 @@ describe "RedisManager", -> describe "updateDocument", -> beforeEach -> - @rclient.set = sinon.stub() - @rclient.rpush = sinon.stub() - @rclient.expire = sinon.stub() - @rclient.ltrim = sinon.stub() - @rclient.del = sinon.stub() - @rclient.eval = sinon.stub() - @RedisManager.getDocVersion = sinon.stub() - @lines = ["one", "two", "three", "これは"] @ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 8 }] }] @version = 42 @hash = crypto.createHash('sha1').update(JSON.stringify(@lines),'utf8').digest('hex') @ranges = { comments: "mock", entries: "mock" } + @doc_update_list_length = sinon.stub() + @project_update_list_length = sinon.stub() - @rclient.exec = sinon.stub().callsArg(0, null, [@hash]) + @RedisManager.getDocVersion = sinon.stub() + @multi.set = sinon.stub() + @multi.rpush = sinon.stub() + @multi.expire = sinon.stub() + @multi.ltrim = sinon.stub() + @multi.del = sinon.stub() + @multi.eval = sinon.stub() + @multi.exec = sinon.stub().callsArgWith(0, null, + [@hash, null, null, null, null, null, null, @doc_update_list_length] + ) + @rclient.rpush = sinon.stub().callsArgWith(@ops.length + 1, null, @project_update_list_length) describe "with a consistent version", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback - - it "should get the current doc version to check for consistency", -> - @RedisManager.getDocVersion - .calledWith(@doc_id) - .should.equal true - - it "should set the doclines", -> - @rclient.eval - .calledWith(sinon.match(/redis.call/), 1, "doclines:#{@doc_id}", JSON.stringify(@lines)) - .should.equal true - - it "should set the version", -> - @rclient.set - .calledWith("DocVersion:#{@doc_id}", @version) - .should.equal true - it "should set the hash", -> - @rclient.set - .calledWith("DocHash:#{@doc_id}", @hash) - .should.equal true - - it "should set the ranges", -> - @rclient.set - .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) - .should.equal true + describe "with project history enabled", -> + beforeEach -> + @settings.apis.project_history.enabled = true + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback - it "should set the unflushed time", -> - @rclient.set - .calledWith("UnflushedTime:#{@doc_id}", Date.now(), "NX") - .should.equal true + it "should get the current doc version to check for consistency", -> + @RedisManager.getDocVersion + .calledWith(@doc_id) + .should.equal true - it "should push the doc op into the doc ops list", -> - @rclient.rpush - .calledWith("DocOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) - .should.equal true + it "should set the doclines", -> + @multi.eval + .calledWith(sinon.match(/redis.call/), 1, "doclines:#{@doc_id}", JSON.stringify(@lines)) + .should.equal true - it "should renew the expiry ttl on the doc ops array", -> - @rclient.expire - .calledWith("DocOps:#{@doc_id}", @RedisManager.DOC_OPS_TTL) - .should.equal true + it "should set the version", -> + @multi.set + .calledWith("DocVersion:#{@doc_id}", @version) + .should.equal true - it "should truncate the list to 100 members", -> - @rclient.ltrim - .calledWith("DocOps:#{@doc_id}", -@RedisManager.DOC_OPS_MAX_LENGTH, -1) - .should.equal true + it "should set the hash", -> + @multi.set + .calledWith("DocHash:#{@doc_id}", @hash) + .should.equal true - it "should call the callback", -> - @callback.called.should.equal true + it "should set the ranges", -> + @multi.set + .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) + .should.equal true + + it "should set the unflushed time", -> + @multi.set + .calledWith("UnflushedTime:#{@doc_id}", Date.now(), "NX") + .should.equal true + + it "should push the doc op into the doc ops list", -> + @multi.rpush + .calledWith("DocOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) + .should.equal true + + it "should renew the expiry ttl on the doc ops array", -> + @multi.expire + .calledWith("DocOps:#{@doc_id}", @RedisManager.DOC_OPS_TTL) + .should.equal true + + it "should truncate the list to 100 members", -> + @multi.ltrim + .calledWith("DocOps:#{@doc_id}", -@RedisManager.DOC_OPS_MAX_LENGTH, -1) + .should.equal true + + it "should push the updates into the history ops list", -> + @multi.rpush + .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) + .should.equal true + + it "should push the updates into the project history ops list", -> + @rclient.rpush + .calledWith("ProjectHistory:Ops:#{@project_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) + .should.equal true + + it "should call the callback", -> + @callback + .calledWith(null, @doc_update_list_length, @project_update_list_length) + .should.equal true + + it 'should not log any errors', -> + @logger.error.calledWith() + .should.equal false + + describe "with project history disabled", -> + beforeEach -> + @rclient.rpush = sinon.stub() + @settings.apis.project_history.enabled = false + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + + it "should not push the updates into the project history ops list", -> + @rclient.rpush.called.should.equal false + + it "should call the callback", -> + @callback + .calledWith(null, @doc_update_list_length) + .should.equal true - it 'should not log any errors', -> - @logger.error.calledWith() - .should.equal false - describe "with an inconsistent version", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length - 1) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback - + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + it "should not call multi.exec", -> - @rclient.exec.called.should.equal false - + @multi.exec.called.should.equal false + it "should call the callback with an error", -> @callback .calledWith(new Error("Version mismatch. '#{@doc_id}' is corrupted.")) .should.equal true - + describe "with no updates", -> beforeEach -> + @rclient.rpush = sinon.stub().callsArgWith(1, null, @project_update_list_length) @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version) - @RedisManager.updateDocument @doc_id, @lines, @version, [], @ranges, @callback - - it "should not do an rpush", -> + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, [], @ranges, @callback + + it "should not try to enqueue doc updates", -> + @multi.rpush + .called + .should.equal false + + it "should not try to enqueue project updates", -> @rclient.rpush .called .should.equal false - + it "should still set the doclines", -> - @rclient.eval + @multi.eval .calledWith(sinon.match(/redis.call/), 1, "doclines:#{@doc_id}", JSON.stringify(@lines)) .should.equal true - + describe "with empty ranges", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, {}, @callback - + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, {}, @callback + it "should not set the ranges", -> - @rclient.set + @multi.set .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) .should.equal false - + it "should delete the ranges key", -> - @rclient.del + @multi.del .calledWith("Ranges:#{@doc_id}") .should.equal true describe "with a corrupted write", -> beforeEach -> @badHash = "INVALID-HASH-VALUE" - @rclient.exec = sinon.stub().callsArgWith(0, null, [@badHash]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@badHash]) @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback it 'should log a hash error', -> @logger.error.calledWith() @@ -444,22 +494,22 @@ describe "RedisManager", -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) @_stringify = JSON.stringify @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback afterEach -> @JSON.stringify = @_stringify - + it "should log an error", -> @logger.error.called.should.equal true it "should call the callback with an error", -> @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true - + describe "with ranges that are too big", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback it 'should log an error', -> @logger.error.called.should.equal true @@ -469,45 +519,51 @@ describe "RedisManager", -> describe "putDocInMemory", -> beforeEach -> - @rclient.set = sinon.stub() + @multi.set = sinon.stub() @rclient.sadd = sinon.stub().yields() - @rclient.del = sinon.stub() - @rclient.eval = sinon.stub() + @multi.del = sinon.stub() + @multi.eval = sinon.stub() @lines = ["one", "two", "three", "これは"] @version = 42 @hash = crypto.createHash('sha1').update(JSON.stringify(@lines),'utf8').digest('hex') - @rclient.exec = sinon.stub().callsArgWith(0, null, [@hash]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@hash]) @ranges = { comments: "mock", entries: "mock" } - + @pathname = '/a/b/c.tex' + describe "with non-empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, done - + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done + it "should set the lines", -> - @rclient.eval + @multi.eval .calledWith(sinon.match(/redis.call/), 1, "doclines:#{@doc_id}", JSON.stringify(@lines)) .should.equal true - + it "should set the version", -> - @rclient.set + @multi.set .calledWith("DocVersion:#{@doc_id}", @version) .should.equal true it "should set the hash", -> - @rclient.set + @multi.set .calledWith("DocHash:#{@doc_id}", @hash) .should.equal true - + it "should set the ranges", -> - @rclient.set + @multi.set .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) .should.equal true - + it "should set the project_id for the doc", -> - @rclient.set + @multi.set .calledWith("ProjectId:#{@doc_id}", @project_id) .should.equal true - + + it "should set the pathname for the doc", -> + @multi.set + .calledWith("Pathname:#{@doc_id}", @pathname) + .should.equal true + it "should add the doc_id to the project set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}", @doc_id) @@ -516,25 +572,25 @@ describe "RedisManager", -> it 'should not log any errors', -> @logger.error.calledWith() .should.equal false - + describe "with empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, done - + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, @pathname, done + it "should delete the ranges key", -> - @rclient.del + @multi.del .calledWith("Ranges:#{@doc_id}") .should.equal true - + it "should not set the ranges", -> - @rclient.set + @multi.set .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) .should.equal false describe "with a corrupted write", -> beforeEach (done) -> - @rclient.exec = sinon.stub().callsArgWith(0, null, ["INVALID-HASH-VALUE"]) - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, done + @multi.exec = sinon.stub().callsArgWith(0, null, ["INVALID-HASH-VALUE"]) + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done it 'should log a hash error', -> @logger.error.calledWith() @@ -544,21 +600,21 @@ describe "RedisManager", -> beforeEach -> @_stringify = JSON.stringify @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback afterEach -> @JSON.stringify = @_stringify - + it "should log an error", -> @logger.error.called.should.equal true it "should call the callback with an error", -> @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true - + 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, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback it 'should log an error', -> @logger.error.called.should.equal true @@ -568,41 +624,46 @@ describe "RedisManager", -> describe "removeDocFromMemory", -> beforeEach (done) -> - @rclient.del = sinon.stub() - @rclient.srem = sinon.stub() - @rclient.exec.yields() + @multi.del = sinon.stub() + @multi.srem = sinon.stub() + @multi.exec.yields() @RedisManager.removeDocFromMemory @project_id, @doc_id, done - + it "should delete the lines", -> - @rclient.del + @multi.del .calledWith("doclines:#{@doc_id}") .should.equal true - + it "should delete the version", -> - @rclient.del + @multi.del .calledWith("DocVersion:#{@doc_id}") .should.equal true it "should delete the hash", -> - @rclient.del + @multi.del .calledWith("DocHash:#{@doc_id}") .should.equal true it "should delete the unflushed time", -> - @rclient.del + @multi.del .calledWith("UnflushedTime:#{@doc_id}") .should.equal true - + it "should delete the project_id for the doc", -> - @rclient.del + @multi.del .calledWith("ProjectId:#{@doc_id}") .should.equal true - + it "should remove the doc_id from the project set", -> - @rclient.srem + @multi.srem .calledWith("DocsIn:#{@project_id}", @doc_id) .should.equal true + it "should delete the pathname for the doc", -> + @multi.del + .calledWith("Pathname:#{@doc_id}") + .should.equal true + describe "clearProjectState", -> beforeEach (done) -> @rclient.del = sinon.stub().callsArg(1) diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index 4f456992f0..946f5d9fbe 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -19,7 +19,7 @@ describe "UpdateManager", -> "./Metrics": @Metrics = Timer: class Timer done: sinon.stub() - "settings-sharelatex": Settings = {} + "settings-sharelatex": @Settings = {} "./DocumentManager": @DocumentManager = {} "./RangesManager": @RangesManager = {} "./Profiler": class Profiler @@ -60,14 +60,14 @@ describe "UpdateManager", -> it "should process the outstanding updates", -> @UpdateManager.processOutstandingUpdates.calledWith(@project_id, @doc_id).should.equal true - + it "should do everything with the lock acquired", -> @UpdateManager.processOutstandingUpdates.calledAfter(@LockManager.tryLock).should.equal true @UpdateManager.processOutstandingUpdates.calledBefore(@LockManager.releaseLock).should.equal true it "should continue processing new updates that may have come in", -> @UpdateManager.continueProcessingUpdatesWithLock.calledWith(@project_id, @doc_id).should.equal true - + it "should return the callback", -> @callback.called.should.equal true @@ -78,7 +78,7 @@ describe "UpdateManager", -> it "should free the lock", -> @LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true - + it "should return the error in the callback", -> @callback.calledWith(@error).should.equal true @@ -93,7 +93,7 @@ describe "UpdateManager", -> it "should not process the updates", -> @UpdateManager.processOutstandingUpdates.called.should.equal false - + describe "continueProcessingUpdatesWithLock", -> describe "when there are outstanding updates", -> beforeEach -> @@ -137,7 +137,7 @@ describe "UpdateManager", -> @UpdateManager.applyUpdate .calledWith(@project_id, @doc_id, update) .should.equal true - + it "should call the callback", -> @callback.called.should.equal true @@ -154,7 +154,7 @@ describe "UpdateManager", -> it "should call the callback", -> @callback.called.should.equal true - + describe "applyUpdate", -> beforeEach -> @update = {op: [{p: 42, i: "foo"}]} @@ -163,23 +163,27 @@ describe "UpdateManager", -> @lines = ["original", "lines"] @ranges = { entries: "mock", comments: "mock" } @updated_ranges = { entries: "updated", comments: "updated" } - @appliedOps = ["mock-applied-ops"] - @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) + @appliedOps = [ {v: 42, op: "mock-op-42"}, { v: 45, op: "mock-op-45" }] + @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) @RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) - @RedisManager.updateDocument = sinon.stub().yields() + @RedisManager.updateDocument = sinon.stub().yields(null, @doc_ops_length, @project_ops_length) @RealTimeRedisManager.sendData = sinon.stub() - @HistoryManager.recordAndFlushHistoryOps = sinon.stub().callsArg(4) - + @UpdateManager._addProjectHistoryMetadataToOps = sinon.stub() + @HistoryManager.recordAndFlushHistoryOps = sinon.stub().callsArg(5) + describe "normally", -> beforeEach -> @UpdateManager.applyUpdate @project_id, @doc_id, @update, @callback - + it "should apply the updates via ShareJS", -> @ShareJsUpdateManager.applyUpdate .calledWith(@project_id, @doc_id, @update, @lines, @version) .should.equal true - + it "should update the ranges", -> @RangesManager.applyUpdate .calledWith(@project_id, @doc_id, @ranges, @appliedOps, @updatedDocLines) @@ -187,12 +191,17 @@ describe "UpdateManager", -> it "should save the document", -> @RedisManager.updateDocument - .calledWith(@doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges) + .calledWith(@project_id, @doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges) .should.equal true - + + it "shoould add metadata to the ops" , -> + @UpdateManager._addProjectHistoryMetadataToOps + .calledWith(@appliedOps, @pathname, @updatedDocLines) + .should.equal true + it "should push the applied ops into the history queue", -> @HistoryManager.recordAndFlushHistoryOps - .calledWith(@project_id, @doc_id, @appliedOps) + .calledWith(@project_id, @doc_id, @appliedOps, @doc_ops_length, @project_ops_length) .should.equal true it "should call the callback", -> @@ -207,16 +216,16 @@ describe "UpdateManager", -> @ShareJsUpdateManager.applyUpdate .calledWith(@project_id, @doc_id, @update) .should.equal true - + # \uFFFD is 'replacement character' @update.op[0].i.should.equal "\uFFFD\uFFFD" - + describe "with an error", -> beforeEach -> @error = new Error("something went wrong") @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(@error) @UpdateManager.applyUpdate @project_id, @doc_id, @update, @callback - + it "should call RealTimeRedisManager.sendData with the error", -> @RealTimeRedisManager.sendData .calledWith({ @@ -228,7 +237,29 @@ describe "UpdateManager", -> it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true - + + describe "_addProjectHistoryMetadataToOps", -> + it "should add pathname and doc_length metadata to the ops", -> + lines = [ + 'some' + 'test' + 'data' + ] + appliedOps = [ {v: 42, op: "mock-op-42"}, { v: 45, op: "mock-op-45" }] + @UpdateManager._addProjectHistoryMetadataToOps(appliedOps, @pathname, lines) + appliedOps.should.deep.equal [{ + v: 42 + op: "mock-op-42" + meta: + pathname: @pathname + doc_length: 14 + }, { + v: 45 + op: "mock-op-45" + meta: + pathname: @pathname + doc_length: 14 + }] describe "lockUpdatesAndDo", -> beforeEach -> @@ -283,7 +314,7 @@ describe "UpdateManager", -> it "should free the lock", -> @LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true - + it "should return the error in the callback", -> @callback.calledWith(@error).should.equal true @@ -295,7 +326,7 @@ describe "UpdateManager", -> it "should free the lock", -> @LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true - + it "should return the error in the callback", -> @callback.calledWith(@error).should.equal true