diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 85a7a4263c..c6d4773036 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -13,33 +13,33 @@ module.exports = DocumentManager = timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> 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, track_changes_entries) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> 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, track_changes_entries, (error) -> + RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, (error) -> return callback(error) if error? - callback null, lines, version, track_changes_entries, false + callback null, lines, version, ranges, false else - callback null, lines, version, track_changes_entries, true + callback null, lines, version, ranges, true - getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps, track_changes_entries) ->) -> + 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, track_changes_entries) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? if fromVersion == -1 - callback null, lines, version, [], track_changes_entries + callback null, lines, version, [], ranges else RedisManager.getPreviousDocOps doc_id, fromVersion, version, (error, ops) -> return callback(error) if error? - callback null, lines, version, ops, track_changes_entries + callback null, lines, version, ops, ranges setDoc: (project_id, doc_id, newLines, source, user_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") @@ -51,7 +51,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, track_changes, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, alreadyLoaded) -> return callback(error) if error? if oldLines? and oldLines.length > 0 and oldLines[0].text? @@ -90,14 +90,14 @@ module.exports = DocumentManager = callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? if !lines? or !version? logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing" callback null # TODO: return a flag to bail out, as we go on to remove doc from memory? else logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc" - PersistenceManager.setDoc project_id, doc_id, lines, version, track_changes_entries, (error) -> + PersistenceManager.setDoc project_id, doc_id, lines, version, ranges, (error) -> return callback(error) if error? callback null diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index fb916da045..e138fe8bc4 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -18,7 +18,7 @@ module.exports = HttpController = else fromVersion = -1 - DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops, track_changes_entries) -> + DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops, ranges) -> timer.done() return next(error) if error? logger.log project_id: project_id, doc_id: doc_id, "got doc via http" @@ -29,7 +29,7 @@ module.exports = HttpController = lines: lines version: version ops: ops - track_changes_entries: track_changes_entries + ranges: ranges _getTotalSizeOfLines: (lines) -> size = 0 diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index ee7674d80a..457627982f 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -10,7 +10,7 @@ logger = require "logger-sharelatex" MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds module.exports = PersistenceManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, track_changes_entries) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -39,13 +39,13 @@ 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.track_changes_entries + return callback null, body.lines, body.version, body.ranges else if res.statusCode == 404 return callback(new Errors.NotFoundError("doc not not found: #{url}")) else return callback(new Error("error accessing web API: #{url} #{res.statusCode}")) - setDoc: (project_id, doc_id, lines, version, track_changes_entries, _callback = (error) ->) -> + setDoc: (project_id, doc_id, lines, version, ranges, _callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -57,7 +57,7 @@ module.exports = PersistenceManager = method: "POST" json: lines: lines - track_changes_entries: track_changes_entries + ranges: ranges version: version auth: user: Settings.apis.web.user diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index a38fe08397..cd4c66ae8d 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -56,31 +56,3 @@ module.exports = ProjectManager = callback new Error("Errors deleting docs. See log for details") else callback(null) - - setTrackChangesWithLocks: (project_id, track_changes_on, _callback = (error) ->) -> - timer = new Metrics.Timer("projectManager.toggleTrackChangesWithLocks") - callback = (args...) -> - timer.done() - _callback(args...) - - RedisManager.getDocIdsInProject project_id, (error, doc_ids) -> - return callback(error) if error? - jobs = [] - errors = [] - for doc_id in (doc_ids or []) - do (doc_id) -> - jobs.push (callback) -> - DocumentManager.setTrackChangesWithLock project_id, doc_id, track_changes_on, (error) -> - if error? - logger.error {err: error, project_id, doc_ids, track_changes_on}, "error toggle track changes for doc" - errors.push(error) - callback() - # TODO: If no docs, turn on track changes in Mongo manually - - logger.log {project_id, doc_ids, track_changes_on}, "toggling track changes for docs" - async.series jobs, () -> - if errors.length > 0 - callback new Error("Errors toggling track changes for docs. See log for details") - else - callback(null) - diff --git a/services/document-updater/app/coffee/RangesManager.coffee b/services/document-updater/app/coffee/RangesManager.coffee new file mode 100644 index 0000000000..1e19a63b0d --- /dev/null +++ b/services/document-updater/app/coffee/RangesManager.coffee @@ -0,0 +1,24 @@ +RangesTracker = require "./RangesTracker" +logger = require "logger-sharelatex" + +module.exports = RangesManager = + applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> + {changes, comments} = entries + logger.log {changes, comments, updates}, "appliyng updates to ranges" + rangesTracker = new RangesTracker(changes, comments) + for update in updates + rangesTracker.track_changes = !!update.meta.tc + for op in update.op + rangesTracker.applyOp(op, { user_id: update.meta?.user_id }) + + # Return the minimal data structure needed, since most documents won't have any + # changes or comments + response = {} + if rangesTracker.changes?.length > 0 + response ?= {} + response.changes = rangesTracker.changes + if rangesTracker.comments?.length > 0 + response ?= {} + response.comments = rangesTracker.comments + logger.log {response}, "applied updates to ranges" + callback null, response \ No newline at end of file diff --git a/services/document-updater/app/coffee/ChangesTracker.coffee b/services/document-updater/app/coffee/RangesTracker.coffee similarity index 95% rename from services/document-updater/app/coffee/ChangesTracker.coffee rename to services/document-updater/app/coffee/RangesTracker.coffee index 8bc4cf9380..6a3625fd09 100644 --- a/services/document-updater/app/coffee/ChangesTracker.coffee +++ b/services/document-updater/app/coffee/RangesTracker.coffee @@ -1,5 +1,5 @@ load = (EventEmitter) -> - class ChangesTracker extends EventEmitter + class RangesTracker extends EventEmitter # The purpose of this class is to track a set of inserts and deletes to a document, like # track changes in Word. We store these as a set of ShareJs style ranges: # {i: "foo", p: 42} # Insert 'foo' at offset 42 @@ -97,7 +97,7 @@ load = (EventEmitter) -> return if !change? @_removeChange(change) - applyOp: (op, metadata = {}) -> + applyOp: (op, metadata) -> metadata.ts ?= new Date() # Apply an op that has been applied to the document to our changes to keep them up to date if op.i? @@ -371,7 +371,23 @@ load = (EventEmitter) -> @emit "changes:moved", moved_changes _newId: () -> - (@id++).toString() + # Generate a Mongo ObjectId + # Reference: https://github.com/dreampulse/ObjectId.js/blob/master/src/main/javascript/Objectid.js + @_pid ?= Math.floor(Math.random() * (32767)) + @_machine ?= Math.floor(Math.random() * (16777216)) + timestamp = Math.floor(new Date().valueOf() / 1000) + @_increment ?= 0 + @_increment++ + + timestamp = timestamp.toString(16) + machine = @_machine.toString(16) + pid = @_pid.toString(16) + increment = @_increment.toString(16) + + return '00000000'.substr(0, 8 - timestamp.length) + timestamp + + '000000'.substr(0, 6 - machine.length) + machine + + '0000'.substr(0, 4 - pid.length) + pid + + '000000'.substr(0, 6 - increment.length) + increment; _addOp: (op, metadata) -> change = { diff --git a/services/document-updater/app/coffee/RedisKeyBuilder.coffee b/services/document-updater/app/coffee/RedisKeyBuilder.coffee index c09fb43f00..1b5e548809 100644 --- a/services/document-updater/app/coffee/RedisKeyBuilder.coffee +++ b/services/document-updater/app/coffee/RedisKeyBuilder.coffee @@ -34,10 +34,8 @@ module.exports = RedisKeyBuilder = return (key_schema) -> key_schema.uncompressedHistoryOp({doc_id}) pendingUpdates: ({doc_id}) -> return (key_schema) -> key_schema.pendingUpdates({doc_id}) - trackChangesEnabled: ({doc_id}) -> - return (key_schema) -> key_schema.trackChangesEnabled({doc_id}) - trackChangesEntries: ({doc_id}) -> - return (key_schema) -> key_schema.trackChangesEntries({doc_id}) + ranges: ({doc_id}) -> + return (key_schema) -> key_schema.ranges({doc_id}) docsInProject: ({project_id}) -> return (key_schema) -> key_schema.docsInProject({project_id}) docsWithHistoryOps: ({project_id}) -> diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index cad5bd9f04..be5166b94c 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -13,17 +13,21 @@ minutes = 60 # seconds for Redis expire module.exports = RedisManager = rclient: rclient - putDocInMemory : (project_id, doc_id, docLines, version, track_changes_entries, _callback)-> + putDocInMemory : (project_id, doc_id, docLines, version, ranges, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> timer.done() _callback(error) logger.log project_id:project_id, doc_id:doc_id, version: version, "putting doc in redis" + ranges = RedisManager._serializeRanges(ranges) multi = rclient.multi() multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines) multi.set keys.projectKey({doc_id:doc_id}), project_id multi.set keys.docVersion(doc_id:doc_id), version - multi.set keys.trackChangesEntries(doc_id:doc_id), JSON.stringify(track_changes_entries) + if ranges? + multi.set keys.ranges(doc_id:doc_id), ranges + else + multi.del keys.ranges(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? rclient.sadd keys.docsInProject(project_id:project_id), doc_id, callback @@ -42,32 +46,33 @@ module.exports = RedisManager = multi.del keys.docLines(doc_id:doc_id) multi.del keys.projectKey(doc_id:doc_id) multi.del keys.docVersion(doc_id:doc_id) - multi.del keys.trackChangesEntries(doc_id:doc_id) + multi.del keys.ranges(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? rclient.srem keys.docsInProject(project_id:project_id), doc_id, callback - getDoc : (project_id, doc_id, callback = (error, lines, version, track_changes_entries) ->)-> + getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi() multi.get keys.docLines(doc_id:doc_id) multi.get keys.docVersion(doc_id:doc_id) multi.get keys.projectKey(doc_id:doc_id) - multi.get keys.trackChangesEntries(doc_id:doc_id) - multi.exec (error, [docLines, version, doc_project_id, track_changes_entries])-> + multi.get keys.ranges(doc_id:doc_id) + multi.exec (error, [docLines, version, doc_project_id, ranges])-> timer.done() return callback(error) if error? try docLines = JSON.parse docLines - track_changes_entries = JSON.parse track_changes_entries + ranges = RedisManager._deserializeRanges(ranges) catch e return callback(e) + version = parseInt(version or 0, 10) # check doc is in requested project if doc_project_id? and doc_project_id isnt project_id 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")) - callback null, docLines, version, track_changes_entries + callback null, docLines, version, ranges getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -107,7 +112,7 @@ module.exports = RedisManager = DOC_OPS_TTL: 60 * minutes DOC_OPS_MAX_LENGTH: 100 - updateDocument : (doc_id, docLines, newVersion, appliedOps = [], track_changes_entries, callback = (error) ->)-> + updateDocument : (doc_id, docLines, newVersion, appliedOps = [], ranges, callback = (error) ->)-> RedisManager.getDocVersion doc_id, (error, currentVersion) -> return callback(error) if error? if currentVersion + appliedOps.length != newVersion @@ -122,10 +127,27 @@ module.exports = RedisManager = multi.rpush keys.docOps(doc_id: doc_id), jsonOps... multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL multi.ltrim keys.docOps(doc_id: doc_id), -RedisManager.DOC_OPS_MAX_LENGTH, -1 - multi.set keys.trackChangesEntries(doc_id:doc_id), JSON.stringify(track_changes_entries) + ranges = RedisManager._serializeRanges(ranges) + if ranges? + multi.set keys.ranges(doc_id:doc_id), ranges + else + multi.del keys.ranges(doc_id:doc_id) multi.exec (error, replys) -> return callback(error) if error? return callback() getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) -> rclient.smembers keys.docsInProject(project_id: project_id), callback + + _serializeRanges: (ranges) -> + jsonRanges = JSON.stringify(ranges) + if jsonRanges == '{}' + # Most doc will have empty ranges so don't fill redis with lots of '{}' keys + jsonRanges = null + return jsonRanges + + _deserializeRanges: (ranges) -> + if !ranges? or ranges == "" + return {} + else + return JSON.parse(ranges) \ No newline at end of file diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee deleted file mode 100644 index 126b9ec7e0..0000000000 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ /dev/null @@ -1,21 +0,0 @@ -ChangesTracker = require "./ChangesTracker" - -module.exports = TrackChangesManager = - applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> - {changes, comments} = entries - changesTracker = new ChangesTracker(changes, comments) - for update in updates - changesTracker.track_changes = !!update.meta.tc - for op in update.op - changesTracker.applyOp(op, { user_id: update.meta?.user_id }) - - # Return the minimal data structure needed, since most documents won't have any - # changes or comments - response = null - if changesTracker.changes?.length > 0 - response ?= {} - response.changes = changesTracker.changes - if changesTracker.comments?.length > 0 - response ?= {} - response.comments = changesTracker.comments - callback null, response \ No newline at end of file diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 7c98b97eee..1678c4d4c0 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -9,7 +9,7 @@ logger = require('logger-sharelatex') Metrics = require "./Metrics" Errors = require "./Errors" DocumentManager = require "./DocumentManager" -TrackChangesManager = require "./TrackChangesManager" +RangesManager = require "./RangesManager" module.exports = UpdateManager = processOutstandingUpdates: (project_id, doc_id, callback = (error) ->) -> @@ -48,16 +48,16 @@ module.exports = UpdateManager = applyUpdate: (project_id, doc_id, update, callback = (error) ->) -> UpdateManager._sanitizeUpdate update - DocumentManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? if !lines? or !version? return callback(new Errors.NotFoundError("document not found: #{doc_id}")) ShareJsUpdateManager.applyUpdate project_id, doc_id, update, lines, version, (error, updatedDocLines, version, appliedOps) -> return callback(error) if error? - TrackChangesManager.applyUpdate project_id, doc_id, track_changes_entries, appliedOps, (error, new_track_changes_entries) -> + RangesManager.applyUpdate project_id, doc_id, ranges, appliedOps, (error, new_ranges) -> return callback(error) if error? logger.log doc_id: doc_id, version: version, "updating doc in redis" - RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_track_changes_entries, (error) -> + RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_ranges, (error) -> return callback(error) if error? HistoryManager.pushUncompressedHistoryOps project_id, doc_id, appliedOps, callback diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index edb8c56ad3..42daf505d9 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -32,8 +32,7 @@ module.exports = docVersion: ({doc_id}) -> "DocVersion:#{doc_id}" projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" - trackChangesEnabled: ({doc_id}) -> "TrackChangesEnabled:#{doc_id}" - trackChangesEntries: ({doc_id}) -> "TrackChangesEntries:#{doc_id}" + ranges: ({doc_id}) -> "Ranges:#{doc_id}" # }, { # cluster: [{ # port: "7000" @@ -46,8 +45,7 @@ module.exports = # docVersion: ({doc_id}) -> "DocVersion:{#{doc_id}}" # projectKey: ({doc_id}) -> "ProjectId:{#{doc_id}}" # docsInProject: ({project_id}) -> "DocsIn:{#{project_id}}" - # trackChangesEnabled: ({doc_id}) -> "TrackChangesEnabled:{#{doc_id}}" - # trackChangesEntries: ({doc_id}) -> "TrackChangesEntries:{#{doc_id}}" + # ranges: ({doc_id}) -> "Ranges:{#{doc_id}}" }] max_doc_length: 2 * 1024 * 1024 # 2mb diff --git a/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee b/services/document-updater/test/acceptance/coffee/RangesTests.coffee similarity index 83% rename from services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee rename to services/document-updater/test/acceptance/coffee/RangesTests.coffee index 88250f82a7..95a7fae9d4 100644 --- a/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee +++ b/services/document-updater/test/acceptance/coffee/RangesTests.coffee @@ -7,8 +7,8 @@ rclient = require("redis").createClient() MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" -describe "Track changes", -> - describe "tracking changes", -> +describe "Ranges", -> + describe "tracking changes from ops", -> before (done) -> @project_id = DocUpdaterClient.randomId() @user_id = DocUpdaterClient.randomId() @@ -46,16 +46,16 @@ describe "Track changes", -> throw error if error? setTimeout done, 200 - it "should update the tracked entries", (done) -> + it "should update the ranges", (done) -> DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => throw error if error? - entries = data.track_changes_entries - change = entries.changes[0] + ranges = data.ranges + change = ranges.changes[0] change.op.should.deep.equal { i: "456", p: 3 } change.metadata.user_id.should.equal @user_id done() - describe "Loading changes from persistence layer", -> + describe "Loading ranges from persistence layer", -> before (done) -> @project_id = DocUpdaterClient.randomId() @user_id = DocUpdaterClient.randomId() @@ -72,7 +72,7 @@ describe "Track changes", -> MockWebApi.insertDoc @project_id, @doc.id, { lines: @doc.lines version: 0 - track_changes_entries: { + ranges: { changes: [{ op: { i: "123", p: 1 } metadata: @@ -87,19 +87,19 @@ describe "Track changes", -> throw error if error? setTimeout done, 200 - it "should have preloaded the existing changes", (done) -> + it "should have preloaded the existing ranges", (done) -> DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => throw error if error? - {changes} = data.track_changes_entries + {changes} = data.ranges changes[0].op.should.deep.equal { i: "123", p: 1 } changes[1].op.should.deep.equal { i: "456", p: 5 } done() - it "should flush the changes to the persistence layer again", (done) -> + it "should flush the ranges to the persistence layer again", (done) -> DocUpdaterClient.flushDoc @project_id, @doc.id, (error) => throw error if error? MockWebApi.getDocument @project_id, @doc.id, (error, doc) => - {changes} = doc.track_changes_entries + {changes} = doc.ranges changes[0].op.should.deep.equal { i: "123", p: 1 } changes[1].op.should.deep.equal { i: "456", p: 5 } done() diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 4e9d073cc4..f2b8bce318 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -11,11 +11,11 @@ module.exports = MockWebApi = doc.lines ?= [] @docs["#{project_id}:#{doc_id}"] = doc - setDocument: (project_id, doc_id, lines, version, track_changes_entries, callback = (error) ->) -> + setDocument: (project_id, doc_id, lines, version, ranges, callback = (error) ->) -> doc = @docs["#{project_id}:#{doc_id}"] ||= {} doc.lines = lines doc.version = version - doc.track_changes_entries = track_changes_entries + doc.ranges = ranges callback null getDocument: (project_id, doc_id, callback = (error, doc) ->) -> @@ -32,7 +32,7 @@ module.exports = MockWebApi = res.send 404 app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) => - MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.track_changes_entries, (error) -> + MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.ranges, (error) -> if error? res.send 500 else diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 3a1db1961c..5966843f5a 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -23,7 +23,7 @@ describe "DocumentManager", -> @callback = sinon.stub() @lines = ["one", "two", "three"] @version = 42 - @track_changes_entries = { comments: "mock", entries: "mock" } + @ranges = { comments: "mock", entries: "mock" } describe "flushAndDeleteDoc", -> describe "successfully", -> @@ -49,7 +49,7 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - it "should flush to track changes", -> + it "should flush to the history api", -> @HistoryManager.flushDocChanges .calledWith(@project_id, @doc_id) .should.equal true @@ -57,7 +57,7 @@ describe "DocumentManager", -> describe "flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) @PersistenceManager.setDoc = sinon.stub().yields() @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback @@ -68,7 +68,7 @@ describe "DocumentManager", -> it "should write the doc lines to the persistence layer", -> @PersistenceManager.setDoc - .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_entries) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges) .should.equal true it "should call the callback without error", -> @@ -102,7 +102,7 @@ describe "DocumentManager", -> describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback @@ -117,14 +117,14 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ops, @track_changes_entries).should.equal true + @callback.calledWith(null, @lines, @version, @ops, @ranges).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, @track_changes_entries) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback @@ -137,7 +137,7 @@ describe "DocumentManager", -> @RedisManager.getPreviousDocOps.called.should.equal false it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, [], @track_changes_entries).should.equal true + @callback.calledWith(null, @lines, @version, [], @ranges).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -145,7 +145,7 @@ describe "DocumentManager", -> describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @@ -154,7 +154,7 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @track_changes_entries, true).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, true).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -162,7 +162,7 @@ describe "DocumentManager", -> 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, @track_changes_entries) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -178,11 +178,11 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_entries) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges) .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @track_changes_entries, false).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, false).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -192,7 +192,7 @@ describe "DocumentManager", -> beforeEach -> @beforeLines = ["before", "lines"] @afterLines = ["after", "lines"] - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @track_changes_entries, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) diff --git a/services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee b/services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee index 8ad2966b23..8fa3931d65 100644 --- a/services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee @@ -22,7 +22,7 @@ describe "HttpController.getDoc", -> @ops = ["mock-op-1", "mock-op-2"] @version = 42 @fromVersion = 42 - @track_changes_entries = { changes: "mock", comments: "mock" } + @ranges = { changes: "mock", comments: "mock" } @res = send: sinon.stub() @req = @@ -33,7 +33,7 @@ describe "HttpController.getDoc", -> describe "when the document exists and no recent ops are requested", -> beforeEach -> - @DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [], @track_changes_entries) + @DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [], @ranges) @HttpController.getDoc(@req, @res, @next) it "should get the doc", -> @@ -48,7 +48,7 @@ describe "HttpController.getDoc", -> lines: @lines version: @version ops: [] - track_changes_entries: @track_changes_entries + ranges: @ranges })) .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 35c276a4f2..19a3d547a2 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -19,7 +19,7 @@ describe "PersistenceManager", -> @lines = ["one", "two", "three"] @version = 42 @callback = sinon.stub() - @track_changes_entries = { comments: "mock", entries: "mock" } + @ranges = { comments: "mock", entries: "mock" } @Settings.apis = web: url: @url = "www.example.com" @@ -33,7 +33,7 @@ describe "PersistenceManager", -> @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify({ lines: @lines, version: @version, - track_changes_entries: @track_changes_entries + ranges: @ranges })) @PersistenceManager.getDoc(@project_id, @doc_id, @callback) @@ -53,8 +53,8 @@ describe "PersistenceManager", -> }) .should.equal true - it "should call the callback with the doc lines, version and track changes state", -> - @callback.calledWith(null, @lines, @version, @track_changes_entries).should.equal true + it "should call the callback with the doc lines, version and ranges", -> + @callback.calledWith(null, @lines, @version, @ranges).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -112,7 +112,7 @@ describe "PersistenceManager", -> describe "with a successful response from the web api", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 200}) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) it "should call the web api", -> @request @@ -121,7 +121,7 @@ describe "PersistenceManager", -> json: lines: @lines version: @version - track_changes_entries: @track_changes_entries + ranges: @ranges method: "POST" auth: user: @user @@ -141,7 +141,7 @@ describe "PersistenceManager", -> describe "when request returns an error", -> beforeEach -> @request.callsArgWith(1, @error = new Error("oops"), null, null) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) it "should return the error", -> @callback.calledWith(@error).should.equal true @@ -152,7 +152,7 @@ describe "PersistenceManager", -> describe "when the request returns 404", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) + @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 @@ -163,7 +163,7 @@ describe "PersistenceManager", -> describe "when the request returns an error status code", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) + @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 901af153c1..420d2039b4 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -22,7 +22,7 @@ describe "RedisManager", -> projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" - trackChangesEntries: ({doc_id}) -> "TrackChangesEntries:#{doc_id}" + ranges: ({doc_id}) -> "Ranges:#{doc_id}" "logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() } "./Metrics": @metrics = inc: sinon.stub() @@ -38,11 +38,10 @@ describe "RedisManager", -> @lines = ["one", "two", "three"] @jsonlines = JSON.stringify @lines @version = 42 - @track_changes_on = true - @track_changes_entries = { comments: "mock", entries: "mock" } - @json_track_changes_entries = JSON.stringify @track_changes_entries + @ranges = { comments: "mock", entries: "mock" } + @json_ranges = JSON.stringify @ranges @rclient.get = sinon.stub() - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @json_track_changes_entries]) + @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @json_ranges]) describe "successfully", -> beforeEach -> @@ -58,20 +57,20 @@ describe "RedisManager", -> .calledWith("DocVersion:#{@doc_id}") .should.equal true - it "should get the track changes entries", -> + it "should get the ranges", -> @rclient.get - .calledWith("TrackChangesEntries:#{@doc_id}") + .calledWith("Ranges:#{@doc_id}") .should.equal true it 'should return the document', -> @callback - .calledWith(null, @lines, @version, @track_changes_entries) + .calledWith(null, @lines, @version, @ranges) .should.equal true 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_track_changes_entries]) + @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id, @json_ranges]) @RedisManager.getDoc @project_id, @doc_id, @callback it 'should return an error', -> @@ -169,19 +168,20 @@ describe "RedisManager", -> @rclient.rpush = sinon.stub() @rclient.expire = sinon.stub() @rclient.ltrim = sinon.stub() + @rclient.del = sinon.stub() @RedisManager.getDocVersion = sinon.stub() @lines = ["one", "two", "three"] @ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 8 }] }] @version = 42 - @track_changes_entries = { comments: "mock", entries: "mock" } + @ranges = { comments: "mock", entries: "mock" } @rclient.exec = sinon.stub().callsArg(0) describe "with a consistent version", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @track_changes_entries, @callback + @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback it "should get the current doc version to check for consistency", -> @RedisManager.getDocVersion @@ -198,9 +198,9 @@ describe "RedisManager", -> .calledWith("DocVersion:#{@doc_id}", @version) .should.equal true - it "should set the track changes entries", -> + it "should set the ranges", -> @rclient.set - .calledWith("TrackChangesEntries:#{@doc_id}", JSON.stringify(@track_changes_entries)) + .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) .should.equal true it "should push the doc op into the doc ops list", -> @@ -224,7 +224,7 @@ describe "RedisManager", -> describe "with an inconsistent version", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length - 1) - @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @track_changes_entries, @callback + @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback it "should not call multi.exec", -> @rclient.exec.called.should.equal false @@ -237,7 +237,7 @@ describe "RedisManager", -> describe "with no updates", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version) - @RedisManager.updateDocument @doc_id, @lines, @version, [], @track_changes_entries, @callback + @RedisManager.updateDocument @doc_id, @lines, @version, [], @ranges, @callback it "should not do an rpush", -> @rclient.rpush @@ -248,42 +248,75 @@ describe "RedisManager", -> @rclient.set .calledWith("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 + + it "should not set the ranges", -> + @rclient.set + .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) + .should.equal false + + it "should delete the ranges key", -> + @rclient.del + .calledWith("Ranges:#{@doc_id}") + .should.equal true describe "putDocInMemory", -> - beforeEach (done) -> + beforeEach -> @rclient.set = sinon.stub() @rclient.sadd = sinon.stub().yields() + @rclient.del = sinon.stub() @rclient.exec.yields() @lines = ["one", "two", "three"] @version = 42 - @track_changes_entries = { comments: "mock", entries: "mock" } - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @track_changes_entries, done + @ranges = { comments: "mock", entries: "mock" } - it "should set the lines", -> - @rclient.set - .calledWith("doclines:#{@doc_id}", JSON.stringify @lines) - .should.equal true - - it "should set the version", -> - @rclient.set - .calledWith("DocVersion:#{@doc_id}", @version) - .should.equal true + describe "with non-empty ranges", -> + beforeEach (done) -> + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, done - it "should set the track changes entries", -> - @rclient.set - .calledWith("TrackChangesEntries:#{@doc_id}", JSON.stringify(@track_changes_entries)) - .should.equal true + it "should set the lines", -> + @rclient.set + .calledWith("doclines:#{@doc_id}", JSON.stringify @lines) + .should.equal true - it "should set the project_id for the doc", -> - @rclient.set - .calledWith("ProjectId:#{@doc_id}", @project_id) - .should.equal true - - it "should add the doc_id to the project set", -> - @rclient.sadd - .calledWith("DocsIn:#{@project_id}", @doc_id) - .should.equal true - + it "should set the version", -> + @rclient.set + .calledWith("DocVersion:#{@doc_id}", @version) + .should.equal true + + it "should set the ranges", -> + @rclient.set + .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) + .should.equal true + + it "should set the project_id for the doc", -> + @rclient.set + .calledWith("ProjectId:#{@doc_id}", @project_id) + .should.equal true + + it "should add the doc_id to the project set", -> + @rclient.sadd + .calledWith("DocsIn:#{@project_id}", @doc_id) + .should.equal true + + describe "with empty ranges", -> + beforeEach (done) -> + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, done + + it "should delete the ranges key", -> + @rclient.del + .calledWith("Ranges:#{@doc_id}") + .should.equal true + + it "should not set the ranges", -> + @rclient.set + .calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges)) + .should.equal false + describe "removeDocFromMemory", -> beforeEach (done) -> @rclient.del = sinon.stub() diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index fb9bc18eb1..e87391af44 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -21,7 +21,7 @@ describe "UpdateManager", -> done: sinon.stub() "settings-sharelatex": Settings = {} "./DocumentManager": @DocumentManager = {} - "./TrackChangesManager": @TrackChangesManager = {} + "./RangesManager": @RangesManager = {} describe "processOutstandingUpdates", -> beforeEach -> @@ -158,11 +158,11 @@ describe "UpdateManager", -> @updatedDocLines = ["updated", "lines"] @version = 34 @lines = ["original", "lines"] - @track_changes_entries = { entries: "mock", comments: "mock" } - @updated_track_changes_entries = { entries: "updated", comments: "updated" } + @ranges = { entries: "mock", comments: "mock" } + @updated_ranges = { entries: "updated", comments: "updated" } @appliedOps = ["mock-applied-ops"] - @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @track_changes_entries) - @TrackChangesManager.applyUpdate = sinon.stub().yields(null, @updated_track_changes_entries) + @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) + @RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) @RedisManager.updateDocument = sinon.stub().yields() @HistoryManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3) @@ -176,17 +176,17 @@ describe "UpdateManager", -> .calledWith(@project_id, @doc_id, @update, @lines, @version) .should.equal true - it "should update the track changes entries", -> - @TrackChangesManager.applyUpdate - .calledWith(@project_id, @doc_id, @track_changes_entries, @appliedOps) + it "should update the ranges", -> + @RangesManager.applyUpdate + .calledWith(@project_id, @doc_id, @ranges, @appliedOps) .should.equal true it "should save the document", -> @RedisManager.updateDocument - .calledWith(@doc_id, @updatedDocLines, @version, @appliedOps, @updated_track_changes_entries) + .calledWith(@doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges) .should.equal true - it "should push the applied ops into the track changes queue", -> + it "should push the applied ops into the history queue", -> @HistoryManager.pushUncompressedHistoryOps .calledWith(@project_id, @doc_id, @appliedOps) .should.equal true