diff --git a/services/document-updater/Gruntfile.coffee b/services/document-updater/Gruntfile.coffee index 30dd63e708..a013653187 100644 --- a/services/document-updater/Gruntfile.coffee +++ b/services/document-updater/Gruntfile.coffee @@ -45,7 +45,7 @@ module.exports = (grunt) -> clean: app: ["app/js"] - acceptance_tests: ["test/unit/js"] + acceptance_tests: ["test/acceptance/js"] mochaTest: unit: @@ -103,7 +103,7 @@ module.exports = (grunt) -> grunt.registerTask 'install', "Compile everything when installing as an npm module", ['compile'] - grunt.registerTask 'test:unit', 'Run the unit tests (use --grep= for individual tests)', ['compile:unit_tests', 'mochaTest:unit'] + grunt.registerTask 'test:unit', 'Run the unit tests (use --grep= for individual tests)', ['compile:server', 'compile:unit_tests', 'mochaTest:unit'] grunt.registerTask 'test:acceptance', 'Run the acceptance tests (use --grep= for individual tests)', ['compile:acceptance_tests', 'mochaTest:acceptance'] grunt.registerTask 'run', "Compile and run the document-updater-sharelatex server", ['compile', 'bunyan', 'execute'] diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 0e90f5b462..70e643dae1 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -1,127 +1,11 @@ RedisManager = require "./RedisManager" -mongojs = require("./mongojs") -db = mongojs.db -ObjectId = mongojs.ObjectId -logger = require "logger-sharelatex" -async = require "async" -Metrics = require("./Metrics") module.exports = DocOpsManager = - flushDocOpsToMongo: (project_id, doc_id, _callback = (error) ->) -> - timer = new Metrics.Timer("docOpsManager.flushDocOpsToMongo") - callback = (args...) -> - timer.done() - _callback(args...) - - DocOpsManager.getDocVersionInMongo doc_id, (error, mongoVersion) -> + getPreviousDocOps: (project_id, doc_id, start, end, callback = (error, ops) ->) -> + RedisManager.getPreviousDocOps doc_id, start, end, (error, ops) -> return callback(error) if error? - RedisManager.getDocVersion doc_id, (error, redisVersion) -> - return callback(error) if error? - if !mongoVersion? or !redisVersion? or mongoVersion > redisVersion - logger.error doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, "mongo version is ahead of redis" - return callback(new Error("inconsistent versions")) - - RedisManager.getPreviousDocOps doc_id, mongoVersion, -1, (error, ops) -> - return callback(error) if error? - if ops.length != redisVersion - mongoVersion - logger.error doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, opsLength: ops.length, "version difference does not match ops length" - return callback(new Error("inconsistent versions")) - logger.log doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, "flushing doc ops to mongo" - DocOpsManager._appendDocOpsInMongo doc_id, ops, redisVersion, (error) -> - return callback(error) if error? - callback null - - getPreviousDocOps: (project_id, doc_id, start, end, _callback = (error, ops) ->) -> - timer = new Metrics.Timer("docOpsManager.getPreviousDocOps") - callback = (args...) -> - timer.done() - _callback(args...) - - DocOpsManager._ensureOpsAreLoaded project_id, doc_id, start, (error) -> - return callback(error) if error? - RedisManager.getPreviousDocOps doc_id, start, end, (error, ops) -> - return callback(error) if error? - callback null, ops + callback null, ops pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> RedisManager.pushDocOp doc_id, op, callback - _ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) -> - RedisManager.getDocVersion doc_id, (error, redisVersion) -> - return callback(error) if error? - RedisManager.getDocOpsLength doc_id, (error, opsLength) -> - return callback(error) if error? - oldestVersionInRedis = redisVersion - opsLength - if oldestVersionInRedis > backToVersion - # _getDocOpsFromMongo(, 4, 6, ...) will return the ops in positions 4 and 5, but not 6. - logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, "loading old ops from mongo" - DocOpsManager._getDocOpsFromMongo doc_id, backToVersion, oldestVersionInRedis, (error, ops) -> - logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, ops: ops, "loaded old ops from mongo" - return callback(error) if error? - RedisManager.prependDocOps doc_id, ops, (error) -> - return callback(error) if error? - callback null - else - logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, "ops already in redis" - callback() - - getDocVersionInMongo: (doc_id, callback = (error, version) ->) -> - t = new Metrics.Timer("mongo-time") - db.docOps.find { - doc_id: ObjectId(doc_id) - }, { - version: 1 - }, (error, docs) -> - t.done() - return callback(error) if error? - if docs.length < 1 or !docs[0].version? - return callback null, 0 - else - return callback null, docs[0].version - - APPEND_OPS_BATCH_SIZE: 100 - - _appendDocOpsInMongo: (doc_id, docOps, newVersion, callback = (error) ->) -> - currentVersion = newVersion - docOps.length - batchSize = DocOpsManager.APPEND_OPS_BATCH_SIZE - noOfBatches = Math.ceil(docOps.length / batchSize) - if noOfBatches <= 0 - return callback() - jobs = [] - for batchNo in [0..(noOfBatches-1)] - do (batchNo) -> - jobs.push (callback) -> - batch = docOps.slice(batchNo * batchSize, (batchNo + 1) * batchSize) - currentVersion += batch.length - logger.log doc_id: doc_id, batchNo: batchNo, "appending doc op batch to Mongo" - t = new Metrics.Timer("mongo-time") - db.docOps.update { - doc_id: ObjectId(doc_id) - }, { - $push: docOps: { $each: batch, $slice: -100 } - $set: version: currentVersion - }, { - upsert: true - }, (err)-> - t.done() - callback(err) - - async.series jobs, (error) -> callback(error) - - _getDocOpsFromMongo: (doc_id, start, end, callback = (error, ops) ->) -> - DocOpsManager.getDocVersionInMongo doc_id, (error, version) -> - return callback(error) if error? - offset = - (version - start) # Negative tells mongo to count from the end backwards - limit = end - start - t = new Metrics.Timer("mongo-time") - db.docOps.find { - doc_id: ObjectId(doc_id) - }, { - docOps: $slice: [offset, limit] - }, (error, docs) -> - t.done() - if docs.length < 1 or !docs[0].docOps? - return callback null, [] - else - return callback null, docs[0].docOps - diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index aa64ac3d7f..423693a5e4 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -16,14 +16,12 @@ module.exports = DocumentManager = return callback(error) if error? if !lines? or !version? logger.log project_id: project_id, doc_id: doc_id, "doc not in redis so getting from persistence API" - PersistenceManager.getDoc project_id, doc_id, (error, lines) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version) -> return callback(error) if error? - DocOpsManager.getDocVersionInMongo doc_id, (error, version) -> + logger.log project_id: project_id, doc_id: doc_id, lines: lines, version: version, "got doc from persistence API" + RedisManager.putDocInMemory project_id, doc_id, lines, version, (error) -> return callback(error) if error? - logger.log project_id: project_id, doc_id: doc_id, lines: lines, version: version, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, (error) -> - return callback(error) if error? - callback null, lines, version + callback null, lines, version else callback null, lines, version @@ -87,12 +85,10 @@ module.exports = DocumentManager = logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing" callback null else - logger.log project_id: project_id, doc_id: doc_id, "flushing doc" - PersistenceManager.setDoc project_id, doc_id, lines, (error) -> + logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc" + PersistenceManager.setDoc project_id, doc_id, lines, version, (error) -> return callback(error) if error? - DocOpsManager.flushDocOpsToMongo project_id, doc_id, (error) -> - return callback(error) if error? - callback null + callback null flushAndDeleteDoc: (project_id, doc_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.flushAndDeleteDoc") diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index eb1a7366c2..03cbe78cbe 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -28,13 +28,13 @@ module.exports = PersistenceManager = body = JSON.parse body catch e return callback(e) - return callback null, body.lines + return callback null, body.lines, body.version 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, _callback = (error) ->) -> + setDoc: (project_id, doc_id, lines, version, _callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -46,6 +46,7 @@ module.exports = PersistenceManager = method: "POST" body: JSON.stringify lines: lines + version: parseInt(version, 10) headers: "content-type": "application/json" auth: diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index b2c4c9c9d0..db7f3afb37 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -9,7 +9,10 @@ keys = require('./RedisKeyBuilder') logger = require('logger-sharelatex') metrics = require('./Metrics') -module.exports = +# Make times easy to read +minutes = 60 # seconds for Redis expire + +module.exports = RedisManager = putDocInMemory : (project_id, doc_id, docLines, version, callback)-> timer = new metrics.Timer("redis.put-doc") logger.log project_id:project_id, doc_id:doc_id, docLines:docLines, version: version, "putting doc in redis" @@ -17,7 +20,6 @@ module.exports = 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.del keys.docOps(doc_id:doc_id) multi.sadd keys.allDocs, doc_id multi.sadd keys.docsInProject(project_id:project_id), doc_id multi.exec (err, replys)-> @@ -31,7 +33,6 @@ module.exports = 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.docOps(doc_id:doc_id) multi.srem keys.docsInProject(project_id:project_id), doc_id multi.srem keys.allDocs, doc_id multi.exec (err, replys)-> @@ -111,7 +112,6 @@ module.exports = rclient.srem keys.docsWithPendingUpdates, doc_key, callback getPreviousDocOps: (doc_id, start, end, callback = (error, jsonOps) ->) -> - # TODO: parse the ops and return them as objects, not JSON rclient.llen keys.docOps(doc_id: doc_id), (error, length) -> return callback(error) if error? rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -141,19 +141,20 @@ module.exports = return callback(e) callback null, ops + DOC_OPS_TTL: 60 * minutes + DOC_OPS_MAX_LENGTH: 100 pushDocOp: (doc_id, op, callback = (error, new_version) ->) -> - # TODO: take a raw op object and JSONify it here jsonOp = JSON.stringify op - rclient.rpush keys.docOps(doc_id: doc_id), jsonOp, (error) -> + multi = rclient.multi() + multi.rpush keys.docOps(doc_id: doc_id), jsonOp + 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.incr keys.docVersion(doc_id: doc_id) + multi.exec (error, replys) -> + [_, __, ___, version] = replys return callback(error) if error? - rclient.incr keys.docVersion(doc_id: doc_id), (error, version) -> - return callback(error) if error? - version = parseInt(version, 10) - callback null, version - - prependDocOps: (doc_id, ops, callback = (error) ->) -> - jsonOps = ops.map (op) -> JSON.stringify op - rclient.lpush keys.docOps(doc_id: doc_id), jsonOps.reverse(), callback + version = parseInt(version, 10) + callback null, version getDocOpsLength: (doc_id, callback = (error, length) ->) -> rclient.llen keys.docOps(doc_id: doc_id), callback diff --git a/services/document-updater/app/coffee/mongojs.coffee b/services/document-updater/app/coffee/mongojs.coffee deleted file mode 100644 index 9a1ae72bc0..0000000000 --- a/services/document-updater/app/coffee/mongojs.coffee +++ /dev/null @@ -1,7 +0,0 @@ -Settings = require "settings-sharelatex" -mongojs = require "mongojs" -db = mongojs.connect(Settings.mongo.url, ["docOps"]) -module.exports = - db: db - ObjectId: mongojs.ObjectId - diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 5108a4c2cc..f42d296952 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -2,9 +2,6 @@ sinon = require "sinon" chai = require("chai") chai.should() async = require "async" -mongojs = require "../../../app/js/mongojs" -db = mongojs.db -ObjectId = mongojs.ObjectId MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -12,13 +9,14 @@ DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Applying updates to a doc", -> before -> @lines = ["one", "two", "three"] + @version = 42 @update = doc: @doc_id op: [{ i: "one and a half\n" p: 4 }] - v: 0 + v: @version @result = ["one", "one and a half", "two", "three"] describe "when the document is not loaded", -> @@ -26,6 +24,7 @@ describe "Applying updates to a doc", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } sinon.spy MockWebApi, "getDocument" DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> @@ -50,6 +49,7 @@ describe "Applying updates to a doc", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => throw error if error? @@ -76,6 +76,7 @@ describe "Applying updates to a doc", -> @lines = ["", "", ""] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: 0 } @updates = [ @@ -92,9 +93,6 @@ describe "Applying updates to a doc", -> { doc_id: @doc_id, v: 10, op: [i: "d", p: 10] } ] @result = ["hello world", "", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } it "should be able to continue applying updates when the project has been deleted", (done) -> actions = [] @@ -118,6 +116,7 @@ describe "Applying updates to a doc", -> @lines = ["", "", ""] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: 0 } @updates = [ @@ -129,9 +128,6 @@ describe "Applying updates to a doc", -> { doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] } ] @result = ["hello", "world", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } it "should be able to continue applying updates when the project has been deleted", (done) -> actions = [] @@ -148,61 +144,13 @@ 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 mongo array has been trimmed", -> - before -> - [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - @lines = ["", "", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } - - @updates = [ - { doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] } - { doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] } - { doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] } - { doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] } - { doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] } - { doc_id: @doc_id, v: 3, op: [i: "world", p: 4 ] } - ] - @result = ["hello", "world", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } - - it "should be able to reload the required ops from the trimmed mongo array", (done) -> - actions = [] - # Apply first set of ops - for update in @updates.slice(0,5) - do (update) => - actions.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc_id, update, callback - # Delete doc from redis and trim ops back to version 3 - actions.push (callback) => DocUpdaterClient.deleteDoc @project_id, @doc_id, callback - actions.push (callback) => - db.docOps.update({doc_id: ObjectId(@doc_id)}, {$push: docOps: { $each: [], $slice: -2 }}, callback) - # Apply older update back from version 3 - for update in @updates.slice(5) - do (update) => - actions.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc_id, update, callback - # Flush ops to mongo - actions.push (callback) => DocUpdaterClient.flushDoc @project_id, @doc_id, callback - - async.series actions, (error) => - throw error if error? - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => - db.docOps.find {doc_id: ObjectId(@doc_id)}, (error, docOps) => - # Check mongo array has been trimmed - docOps = docOps[0] - docOps.docOps.length.should.equal 3 - # Check ops have all be applied properly - doc.lines.should.deep.equal @result - done() describe "with a broken update", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) -> throw error if error? diff --git a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee index d28f37cd6d..171dfcc6e2 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee @@ -8,13 +8,14 @@ DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Deleting a document", -> before -> @lines = ["one", "two", "three"] + @version = 42 @update = doc: @doc_id op: [{ i: "one and a half\n" p: 4 }] - v: 0 + v: @version @result = ["one", "one and a half", "two", "three"] describe "when the updated doc exists in the doc updater", -> @@ -22,6 +23,7 @@ describe "Deleting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } sinon.spy MockWebApi, "setDocumentLines" sinon.spy MockWebApi, "getDocument" @@ -60,6 +62,7 @@ describe "Deleting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } sinon.spy MockWebApi, "setDocumentLines" sinon.spy MockWebApi, "getDocument" diff --git a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee index 7b07ed6a25..5bfc5a6ee8 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee @@ -35,6 +35,7 @@ describe "Deleting a project", -> for doc in @docs MockWebApi.insertDoc @project_id, doc.id, { lines: doc.lines + version: doc.update.v } describe "with documents which have been updated", -> diff --git a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee index 02b44e3fd6..4949d529a2 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee @@ -35,6 +35,7 @@ describe "Flushing a project", -> for doc in @docs MockWebApi.insertDoc @project_id, doc.id, { lines: doc.lines + version: doc.update.v } describe "with documents which have been updated", -> diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index aaaef99936..01db25fb40 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -5,23 +5,22 @@ async = require "async" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" -mongojs = require "../../../app/js/mongojs" -db = mongojs.db -ObjectId = mongojs.ObjectId describe "Flushing a doc to Mongo", -> before -> @lines = ["one", "two", "three"] + @version = 42 @update = doc: @doc_id op: [{ i: "one and a half\n" p: 4 }] - v: 0 + v: @version @result = ["one", "one and a half", "two", "three"] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } describe "when the updated doc exists in the doc updater", -> @@ -29,8 +28,10 @@ describe "Flushing a doc to Mongo", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockWebApi, "setDocumentVersion" DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => throw error if error? @@ -40,58 +41,36 @@ describe "Flushing a doc to Mongo", -> after -> MockWebApi.setDocumentLines.restore() + MockWebApi.setDocumentVersion.restore() - it "should flush the updated document to the web api", -> + it "should flush the updated doc lines to the web api", -> MockWebApi.setDocumentLines .calledWith(@project_id, @doc_id, @result) .should.equal true - it "should flush the doc ops to Mongo", (done) -> - db.docOps.find doc_id: ObjectId(@doc_id), (error, docs) => - doc = docs[0] - doc.docOps[0].op.should.deep.equal @update.op - done() - - describe "when the doc has a large number of ops to be flushed", -> - before (done) -> - [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } - @updates = [] - for v in [0..999] - @updates.push - doc_id: @doc_id, - op: [i: v.toString(), p: 0] - v: v - - DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => - throw error if error? - setTimeout () => - DocUpdaterClient.flushDoc @project_id, @doc_id, done - , 200 - - it "should flush the doc ops to Mongo in order", (done) -> - db.docOps.find doc_id: ObjectId(@doc_id), (error, docs) => - doc = docs[0] - updates = @updates.slice(-100) - for update, i in doc.docOps - update.op.should.deep.equal updates[i].op - done() + it "should flush the updated doc version to the web api", -> + MockWebApi.setDocumentVersion + .calledWith(@project_id, @doc_id, @version + 1) + .should.equal true describe "when the doc does not exist in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockWebApi, "setDocumentVersion" DocUpdaterClient.flushDoc @project_id, @doc_id, done after -> MockWebApi.setDocumentLines.restore() + MockWebApi.setDocumentVersion.restore() it "should not flush the doc to the web api", -> MockWebApi.setDocumentLines.called.should.equal false + MockWebApi.setDocumentVersion.called.should.equal false + diff --git a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee index 0e8456e45f..43c039a802 100644 --- a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee @@ -11,6 +11,7 @@ describe "Getting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines = ["one", "two", "three"] + version: @version = 42 } sinon.spy MockWebApi, "getDocument" DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done() @@ -26,8 +27,8 @@ describe "Getting a document", -> it "should return the document lines", -> @returnedDoc.lines.should.deep.equal @lines - it "should return the document at version 0", -> - @returnedDoc.version.should.equal 0 + it "should return the document at its current version", -> + @returnedDoc.version.should.equal @version describe "when the document is already loaded", -> before (done) -> @@ -55,6 +56,7 @@ describe "Getting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines = ["one", "two", "three"] + version: 0 } @updates = for v in [0..99] diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index cc0f30834a..5218a15281 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -9,22 +9,25 @@ describe "Setting a document", -> before -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @lines = ["one", "two", "three"] + @version = 42 @update = doc: @doc_id op: [{ i: "one and a half\n" p: 4 }] - v: 0 + v: @version @result = ["one", "one and a half", "two", "three"] @newLines = ["these", "are", "the", "new", "lines"] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines + version: @version } describe "when the updated doc exists in the doc updater", -> before (done) -> sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockWebApi, "setDocumentVersion" DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => throw error if error? DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => @@ -37,15 +40,21 @@ describe "Setting a document", -> after -> MockWebApi.setDocumentLines.restore() + MockWebApi.setDocumentVersion.restore() it "should return a 204 status code", -> @statusCode.should.equal 204 - it "should send the updated document to the web api", -> + it "should send the updated doc lines to the web api", -> MockWebApi.setDocumentLines .calledWith(@project_id, @doc_id, @newLines) .should.equal true + it "should send the updated doc version to the web api", -> + MockWebApi.setDocumentVersion + .calledWith(@project_id, @doc_id, @version + 2) + .should.equal true + it "should update the lines in the doc updater", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @newLines @@ -53,6 +62,6 @@ describe "Setting a document", -> it "should bump the version in the doc updater", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => - doc.version.should.equal 2 + doc.version.should.equal @version + 2 done() diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index 4ddef90d26..f4789854c1 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -4,7 +4,9 @@ async = require "async" module.exports = DocUpdaterClient = randomId: () -> - return require("../../../../app/js/mongojs").ObjectId().toString() + chars = for i in [1..24] + Math.random().toString(16)[2] + return chars.join("") sendUpdate: (project_id, doc_id, update, callback = (error) ->) -> rclient.rpush "PendingUpdates:#{doc_id}", JSON.stringify(update), (error)-> diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 7d50eb8377..21a914dc4b 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -14,6 +14,11 @@ module.exports = MockWebApi = @docs["#{project_id}:#{doc_id}"].lines = lines callback null + setDocumentVersion: (project_id, doc_id, version, callback = (error) ->) -> + @docs["#{project_id}:#{doc_id}"] ||= {} + @docs["#{project_id}:#{doc_id}"].version = version + callback null + getDocument: (project_id, doc_id, callback = (error, doc) ->) -> callback null, @docs["#{project_id}:#{doc_id}"] @@ -28,11 +33,12 @@ module.exports = MockWebApi = res.send 404 app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) => - @setDocumentLines req.params.project_id, req.params.doc_id, req.body.lines, (error) -> - if error? - res.send 500 - else - res.send 204 + MockWebApi.setDocumentLines req.params.project_id, req.params.doc_id, req.body.lines, (error) -> + MockWebApi.setDocumentVersion req.params.project_id, req.params.doc_id, req.body.version, (error) -> + if error1? or error2? + res.send 500 + else + res.send 204 app.listen(3000) diff --git a/services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee b/services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee index 1ca00bb305..019f32bc74 100644 --- a/services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee +++ b/services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee @@ -20,9 +20,6 @@ describe 'putting a doc into memory', ()-> potentialSAdds[keys.allDocs] = doc_id potentialSAdds[keys.docsInProject(project_id:project_id)] = doc_id - potentialDels = {} - potentialDels[keys.docOps(doc_id:doc_id)] = true - mocks = "logger-sharelatex": log:-> redis: @@ -53,6 +50,5 @@ describe 'putting a doc into memory', ()-> redisManager.putDocInMemory project_id, doc_id, lines, version, ()-> assert.deepEqual potentialSets, {} assert.deepEqual potentialSAdds, {} - assert.deepEqual potentialDels, {} done() diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee index 83e0ff48cf..decca4e14d 100644 --- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee @@ -3,109 +3,15 @@ chai = require('chai') should = chai.should() modulePath = "../../../../app/js/DocOpsManager.js" SandboxedModule = require('sandboxed-module') -ObjectId = require("../../../../app/js/mongojs").ObjectId describe "DocOpsManager", -> beforeEach -> - @doc_id = ObjectId().toString() + @doc_id = "doc-id" @project_id = "project-id" @callback = sinon.stub() @DocOpsManager = SandboxedModule.require modulePath, requires: "./RedisManager": @RedisManager = {} - "./mongojs": - db: @db = { docOps: {} } - ObjectId: ObjectId "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } - "./Metrics": @Metrics = - Timer: class Timer - done: sinon.stub() - - describe "flushDocOpsToMongo", -> - describe "when versions are consistent", -> - beforeEach -> - @mongo_version = 40 - @redis_version = 42 - @ops = [ "mock-op-1", "mock-op-2" ] - @DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version) - @RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version) - @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) - @DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3) - @DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback - - it "should get the version from Mongo", -> - @DocOpsManager.getDocVersionInMongo - .calledWith(@doc_id) - .should.equal true - - it "should get the version from REdis", -> - @RedisManager.getDocVersion - .calledWith(@doc_id) - .should.equal true - - it "should get all doc ops since the version in Mongo", -> - @RedisManager.getPreviousDocOps - .calledWith(@doc_id, @mongo_version, -1) - .should.equal true - - it "should update Mongo with the new ops", -> - @DocOpsManager._appendDocOpsInMongo - .calledWith(@doc_id, @ops, @redis_version) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true - - it "should time the execution", -> - @Metrics.Timer::done.called.should.equal true - - describe "when the number of ops does not match the difference in versions", -> - beforeEach -> - @mongo_version = 40 - @redis_version = 45 - @ops = [ "mock-op-1", "mock-op-2" ] - @DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version) - @RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version) - @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) - @DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3) - @DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback - - it "should call the callback with an error", -> - @callback.calledWith(new Error("inconsistet versions")).should.equal true - - it "should log an error", -> - @logger.error - .calledWith(doc_id: @doc_id, mongoVersion: @mongo_version, redisVersion: @redis_version, opsLength: @ops.length, "version difference does not match ops length") - .should.equal true - - it "should not modify mongo", -> - @DocOpsManager._appendDocOpsInMongo.called.should.equal false - - it "should time the execution", -> - @Metrics.Timer::done.called.should.equal true - - describe "when redis version is behind mongo version", -> - beforeEach -> - @mongo_version = 40 - @redis_version = 30 - @DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version) - @RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version) - @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) - @DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3) - @DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback - - it "should call the callback with an error", -> - @callback.calledWith(new Error("inconsistet versions")).should.equal true - - it "should log an error", -> - @logger.error - .calledWith(doc_id: @doc_id, mongoVersion: @mongo_version, redisVersion: @redis_version, "mongo version is ahead of redis") - .should.equal true - - it "should not modify mongo", -> - @DocOpsManager._appendDocOpsInMongo.called.should.equal false - - it "should time the execution", -> - @Metrics.Timer::done.called.should.equal true describe "getPreviousDocOps", -> beforeEach -> @@ -113,14 +19,8 @@ describe "DocOpsManager", -> @start = 30 @end = 32 @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) - @DocOpsManager._ensureOpsAreLoaded = sinon.stub().callsArg(3) @DocOpsManager.getPreviousDocOps @project_id, @doc_id, @start, @end, @callback - it "should ensure the ops are loaded back far enough", -> - @DocOpsManager._ensureOpsAreLoaded - .calledWith(@project_id, @doc_id, @start) - .should.equal true - it "should get the previous doc ops", -> @RedisManager.getPreviousDocOps .calledWith(@doc_id, @start, @end) @@ -128,182 +28,3 @@ describe "DocOpsManager", -> it "should call the callback with the ops", -> @callback.calledWith(null, @ops).should.equal true - - it "should time the execution", -> - @Metrics.Timer::done.called.should.equal true - - describe "_ensureOpsAreLoaded", -> - describe "when the ops are not loaded", -> - beforeEach -> - @redisVersion = 42 - @redisOpsLength = 10 - @backToVersion = 30 - @ops = [ "mock-op-1", "mock-op-2" ] - @RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redisVersion) - @RedisManager.getDocOpsLength = sinon.stub().callsArgWith(1, null, @redisOpsLength) - @DocOpsManager._getDocOpsFromMongo = sinon.stub().callsArgWith(3, null, @ops) - @RedisManager.prependDocOps = sinon.stub().callsArgWith(2, null) - @DocOpsManager._ensureOpsAreLoaded @project_id, @doc_id, @backToVersion, @callback - - it "should get the doc version from redis", -> - @RedisManager.getDocVersion - .calledWith(@doc_id) - .should.equal true - - it "should get the doc ops length in redis", -> - @RedisManager.getDocOpsLength - .calledWith(@doc_id) - .should.equal true - - it "should get the doc ops that need loading from Mongo", -> - @DocOpsManager._getDocOpsFromMongo - .calledWith(@doc_id, @backToVersion, @redisVersion - @redisOpsLength) - .should.equal true - - it "should prepend the retrieved ops to redis", -> - @RedisManager.prependDocOps - .calledWith(@doc_id, @ops) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true - - describe "when the ops are loaded", -> - beforeEach -> - @redisVersion = 42 - @redisOpsLength = 10 - @backToVersion = 35 - @RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redisVersion) - @RedisManager.getDocOpsLength = sinon.stub().callsArgWith(1, null, @redisOpsLength) - @DocOpsManager._getDocOpsFromMongo = sinon.stub().callsArgWith(3, null, @ops) - @RedisManager.prependDocOps = sinon.stub().callsArgWith(2, null) - @DocOpsManager._ensureOpsAreLoaded @project_id, @doc_id, @backToVersion, @callback - - it "should not need to get the docs from Mongo or put any into redis", -> - @DocOpsManager._getDocOpsFromMongo.called.should.equal false - @RedisManager.prependDocOps.called.should.equal false - - it "should call the callback", -> - @callback.called.should.equal true - - describe "getDocVersionInMongo", -> - describe "when the doc exists", -> - beforeEach -> - @doc = - version: @version = 42 - @db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc]) - @DocOpsManager.getDocVersionInMongo @doc_id, @callback - - it "should look for the doc in the database", -> - @db.docOps.find - .calledWith({ doc_id: ObjectId(@doc_id) }, {version: 1}) - .should.equal true - - it "should call the callback with the version", -> - @callback.calledWith(null, @version).should.equal true - - describe "when the doc doesn't exist", -> - beforeEach -> - @db.docOps.find = sinon.stub().callsArgWith(2, null, []) - @DocOpsManager.getDocVersionInMongo @doc_id, @callback - - it "should call the callback with 0", -> - @callback.calledWith(null, 0).should.equal true - - describe "_appendDocOpsInMongo", -> - describe "with a small set of updates", -> - beforeEach (done) -> - @ops = [ "mock-op-1", "mock-op-2" ] - @version = 42 - @db.docOps.update = sinon.stub().callsArg(3) - @DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) => - @callback(error) - done() - - it "should update the database", -> - @db.docOps.update - .calledWith({ - doc_id: ObjectId(@doc_id) - }, { - $push: docOps: { $each: @ops, $slice: -100 } - $set: version: @version - }, { - upsert: true - }) - .should.equal true - - it "should call the callbak", -> - @callback.called.should.equal true - - describe "with a large set of updates", -> - beforeEach (done) -> - @ops = [ "mock-op-1", "mock-op-2", "mock-op-3", "mock-op-4", "mock-op-5" ] - @version = 42 - @DocOpsManager.APPEND_OPS_BATCH_SIZE = 2 - @db.docOps.update = sinon.stub().callsArg(3) - @DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) => - @callback(error) - done() - - it "should update the database in batches", -> - @db.docOps.update - .calledWith({ doc_id: ObjectId(@doc_id) }, { - $push: docOps: { $each: @ops.slice(0,2), $slice: -100 } - $set: version: @version - 3 - }, { upsert: true }) - .should.equal true - @db.docOps.update - .calledWith({ doc_id: ObjectId(@doc_id) }, { - $push: docOps: { $each: @ops.slice(2,4), $slice: -100 } - $set: version: @version - 1 - }, { upsert: true }) - .should.equal true - @db.docOps.update - .calledWith({ doc_id: ObjectId(@doc_id) }, { - $push: docOps: { $each: @ops.slice(4,5), $slice: -100 } - $set: version: @version - }, { upsert: true }) - .should.equal true - - it "should call the callbak", -> - @callback.called.should.equal true - - describe "with no updates", -> - beforeEach (done) -> - @ops = [] - @version = 42 - @db.docOps.update = sinon.stub().callsArg(3) - @DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) => - @callback(error) - done() - - it "should not try to update the database", -> - @db.docOps.update.called.should.equal false - - describe "_getDocsOpsFromMongo", -> - beforeEach -> - @version = 42 - @start = 32 - @limit = 5 - @doc = - docOps: ["mock-ops"] - @DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) - @db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc]) - @DocOpsManager._getDocOpsFromMongo @doc_id, @start, @start + @limit, @callback - - it "should get the current version", -> - @DocOpsManager.getDocVersionInMongo - .calledWith(@doc_id) - .should.equal true - - it "should get the doc ops", -> - @db.docOps.find - .calledWith({ doc_id: ObjectId(@doc_id) }, { - docOps: $slice: [-(@version - @start), @limit] - }) - .should.equal true - - it "should return the ops", -> - @callback.calledWith(null, @doc.docOps).should.equal true - - diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee index 079341a536..6bdba1a2b7 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee @@ -23,8 +23,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version) - @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) - @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) + @PersistenceManager.setDoc = sinon.stub().callsArgWith(4) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> @@ -34,14 +33,9 @@ describe "DocumentUpdater - flushDocIfLoaded", -> it "should write the doc lines to the persistence layer", -> @PersistenceManager.setDoc - .calledWith(@project_id, @doc_id, @lines) + .calledWith(@project_id, @doc_id, @lines, @version) .should.equal true - it "should write the doc ops to mongo", -> - @DocOpsManager.flushDocOpsToMongo - .calledWith(@project_id, @doc_id) - .should.equal true - it "should call the callback without error", -> @callback.calledWith(null).should.equal true @@ -51,7 +45,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> describe "when the document is not in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null) - @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) + @PersistenceManager.setDoc = sinon.stub().callsArgWith(4) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback diff --git a/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee index 93de1725fa..ea68890199 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee @@ -40,8 +40,7 @@ describe "DocumentUpdater - getDoc", -> describe "when the doc does not exist in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines) - @DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version) @RedisManager.putDocInMemory = sinon.stub().callsArg(4) @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -50,11 +49,6 @@ describe "DocumentUpdater - getDoc", -> .calledWith(@doc_id) .should.equal true - it "should get the doc version from Mongo", -> - @DocOpsManager.getDocVersionInMongo - .calledWith(@doc_id) - .should.equal true - it "should get the doc from the PersistenceManager", -> @PersistenceManager.getDoc .calledWith(@project_id, @doc_id) diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee index c5cfc35ac8..0bb881b3ee 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee @@ -16,6 +16,7 @@ describe "PersistenceManager.getDoc", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] + @version = 42 @callback = sinon.stub() @Settings.apis = web: @@ -25,7 +26,7 @@ describe "PersistenceManager.getDoc", -> describe "with a successful response from the web api", -> beforeEach -> - @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines)) + @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version)) @PersistenceManager.getDoc(@project_id, @doc_id, @callback) it "should call the web api", -> @@ -43,8 +44,8 @@ describe "PersistenceManager.getDoc", -> }) .should.equal true - it "should call the callback with the doc lines", -> - @callback.calledWith(null, @lines).should.equal true + it "should call the callback with the doc lines and version", -> + @callback.calledWith(null, @lines, @version).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee index cd9d962d3b..82850e3074 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee @@ -16,6 +16,7 @@ describe "PersistenceManager.setDoc", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] + @version = 42 @callback = sinon.stub() @Settings.apis = web: @@ -25,8 +26,8 @@ describe "PersistenceManager.setDoc", -> describe "with a successful response from the web api", -> beforeEach -> - @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines)) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback) + @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version)) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback) it "should call the web api", -> @request @@ -34,6 +35,7 @@ describe "PersistenceManager.setDoc", -> url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}" body: JSON.stringify lines: @lines + version: @version method: "POST" headers: "content-type": "application/json" @@ -54,7 +56,7 @@ describe "PersistenceManager.setDoc", -> describe "when request returns an error", -> beforeEach -> @request.callsArgWith(1, @error = new Error("oops"), null, null) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback) it "should return the error", -> @callback.calledWith(@error).should.equal true @@ -65,7 +67,7 @@ describe "PersistenceManager.setDoc", -> describe "when the request returns 404", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback) it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -76,7 +78,7 @@ describe "PersistenceManager.setDoc", -> describe "when the request returns an error status code", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @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/prependDocOpsTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee deleted file mode 100644 index b4a8192d12..0000000000 --- a/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee +++ /dev/null @@ -1,32 +0,0 @@ -sinon = require('sinon') -chai = require('chai') -should = chai.should() -modulePath = "../../../../app/js/RedisManager" -SandboxedModule = require('sandboxed-module') - -describe "RedisManager.clearDocFromPendingUpdatesSet", -> - beforeEach -> - @doc_id = "document-id" - @callback = sinon.stub() - @RedisManager = SandboxedModule.require modulePath, requires: - "redis" : createClient: () => - @rclient = auth:-> - - @rclient.lpush = sinon.stub().callsArg(2) - @ops = [ - { "mock" : "op-1" }, - { "mock" : "op-2" } - ] - @reversedJsonOps = @ops.map((op) -> JSON.stringify op).reverse() - @RedisManager.prependDocOps(@doc_id, @ops, @callback) - - it "should push the reversed JSONed ops", -> - @rclient.lpush - .calledWith("DocOps:#{@doc_id}", @reversedJsonOps) - .should.equal true - - it "should return the callback", -> - @callback.called.should.equal true - - - diff --git a/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee index 0c76730437..247862a257 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee @@ -1,37 +1,54 @@ sinon = require('sinon') chai = require('chai') should = chai.should() -modulePath = "../../../../app/js/RedisManager" +modulePath = "../../../../app/js/RedisManager.js" SandboxedModule = require('sandboxed-module') -describe "RedisManager.getPreviousDocOpsTests", -> +describe "RedisManager.pushDocOp", -> beforeEach -> - @callback = sinon.stub() @RedisManager = SandboxedModule.require modulePath, requires: - "redis" : createClient: () => + "redis": createClient: () => @rclient = - auth: -> - multi: => @rclient + auth: () -> + multi: () => @rclient + "logger-sharelatex": @logger = {log: sinon.stub()} @doc_id = "doc-id-123" + @callback = sinon.stub() + @rclient.rpush = sinon.stub() + @rclient.expire = sinon.stub() + @rclient.incr = sinon.stub() + @rclient.ltrim = sinon.stub() + + describe "successfully", -> + beforeEach -> + @op = { op: [{ i: "foo", p: 4 }] } + @version = 42 + _ = null + @rclient.exec = sinon.stub().callsArgWith(0, null, [_, _, _, @version]) + @RedisManager.pushDocOp @doc_id, @op, @callback + + it "should push the doc op into the doc ops list", -> + @rclient.rpush + .calledWith("DocOps:#{@doc_id}", JSON.stringify(@op)) + .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 truncate the list to 100 members", -> + @rclient.ltrim + .calledWith("DocOps:#{@doc_id}", -@RedisManager.DOC_OPS_MAX_LENGTH, -1) + .should.equal true + + it "should increment the version number", -> + @rclient.incr + .calledWith("DocVersion:#{@doc_id}") + .should.equal true + + it "should call the callback with the version number", -> + @callback.calledWith(null, parseInt(@version, 10)).should.equal true - beforeEach -> - @version = 70 - @op = - { "mock": "op-1" } - @jsonOp = JSON.stringify @op - @rclient.rpush = sinon.stub().callsArgWith(2, null) - @rclient.incr = sinon.stub().callsArgWith(1, null, @version.toString()) - @RedisManager.pushDocOp(@doc_id, @op, @callback) - it "should push the op into redis", -> - @rclient.rpush - .calledWith("DocOps:#{@doc_id}", @jsonOp) - .should.equal true - it "should increment the version number", -> - @rclient.incr - .calledWith("DocVersion:#{@doc_id}") - .should.equal true - - it "should call the callback with the new version", -> - @callback.calledWith(null, @version).should.equal true