From eb9a3b314a6e0fb8cedb11b4739df408f4e62bb3 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 28 Nov 2016 14:55:16 +0000 Subject: [PATCH] Set/get version in docOps via docstore HTTP API --- .../app/coffee/DocArchiveManager.coffee | 12 +- .../docstore/app/coffee/DocManager.coffee | 29 ++- .../docstore/app/coffee/HttpController.coffee | 8 +- .../docstore/app/coffee/MongoManager.coffee | 23 ++- services/docstore/app/coffee/mongojs.coffee | 2 +- .../docstore/config/settings.defaults.coffee | 14 +- .../acceptance/coffee/ArchiveDocsTests.coffee | 4 +- .../coffee/GettingAllDocsTests.coffee | 2 +- .../coffee/UpdatingDocsTests.coffee | 40 ++++- .../coffee/helpers/DocstoreClient.coffee | 6 +- .../test/unit/coffee/DocManagerTests.coffee | 170 ++++++++++++------ .../unit/coffee/HttpControllerTests.coffee | 29 ++- .../test/unit/coffee/MongoManagerTests.coffee | 47 ++++- 13 files changed, 300 insertions(+), 86 deletions(-) diff --git a/services/docstore/app/coffee/DocArchiveManager.coffee b/services/docstore/app/coffee/DocArchiveManager.coffee index 87c0f3aaf0..5e0272466c 100644 --- a/services/docstore/app/coffee/DocArchiveManager.coffee +++ b/services/docstore/app/coffee/DocArchiveManager.coffee @@ -25,7 +25,10 @@ module.exports = DocArchive = archiveDoc: (project_id, doc, callback)-> logger.log project_id: project_id, doc_id: doc._id, "sending doc to s3" - options = DocArchive.buildS3Options(doc.lines, project_id+"/"+doc._id) + try + options = DocArchive.buildS3Options(doc.lines, project_id+"/"+doc._id) + catch e + return callback e request.put options, (err, res)-> if err? || res.statusCode != 200 logger.err err:err, res:res, project_id:project_id, doc_id: doc._id, statusCode: res?.statusCode, "something went wrong archiving doc in aws" @@ -56,7 +59,10 @@ module.exports = DocArchive = unarchiveDoc: (project_id, doc_id, callback)-> logger.log project_id: project_id, doc_id: doc_id, "getting doc from s3" - options = DocArchive.buildS3Options(true, project_id+"/"+doc_id) + try + options = DocArchive.buildS3Options(true, project_id+"/"+doc_id) + catch e + return callback e request.get options, (err, res, lines)-> if err? || res.statusCode != 200 logger.err err:err, res:res, project_id:project_id, doc_id:doc_id, "something went wrong unarchiving doc from aws" @@ -71,6 +77,8 @@ module.exports = DocArchive = callback() buildS3Options: (content, key)-> + if !settings.docstore.s3? + throw new Error("S3 settings are not configured") return { aws: key: settings.docstore.s3.key diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index e09d156f22..6a963dc291 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -5,7 +5,10 @@ _ = require "underscore" DocArchive = require "./DocArchiveManager" module.exports = DocManager = - + # TODO: For historical reasons, the doc version is currently stored in the docOps + # collection (which is all that this collection contains). In future, we should + # migrate this version property to be part of the docs collection, to guarantee + # consitency between lines and version when writing/reading, and for a simpler schema. getDoc: (project_id, doc_id, callback = (error, doc) ->) -> MongoManager.findDoc project_id, doc_id, (err, doc)-> if err? @@ -19,7 +22,10 @@ module.exports = DocManager = return callback(err) DocManager.getDoc project_id, doc_id, callback else - callback err, doc + MongoManager.getDocVersion doc_id, (error, version) -> + return callback(error) if error? + doc.version = version + callback err, doc getAllDocs: (project_id, callback = (error, docs) ->) -> DocArchive.unArchiveAllDocs project_id, (error) -> @@ -31,7 +37,7 @@ module.exports = DocManager = else return callback(null, docs) - updateDoc: (project_id, doc_id, lines, callback = (error, modified, rev) ->) -> + updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) -> DocManager.getDoc project_id, doc_id, (err, doc)-> if err? and !(err instanceof Errors.NotFoundError) logger.err project_id: project_id, doc_id: doc_id, err:err, "error getting document for update" @@ -39,8 +45,12 @@ module.exports = DocManager = isNewDoc = lines.length == 0 linesAreSame = _.isEqual(doc?.lines, lines) + if version? + versionsAreSame = (doc?.version == version) + else + versionsAreSame = true - if linesAreSame and !isNewDoc + if linesAreSame and versionsAreSame and !isNewDoc logger.log project_id: project_id, doc_id: doc_id, rev: doc?.rev, "doc lines have not changed - not updating" return callback null, false, doc?.rev else @@ -54,7 +64,16 @@ module.exports = DocManager = }, "updating doc lines" MongoManager.upsertIntoDocCollection project_id, doc_id, lines, (error)-> return callback(callback) if error? - callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc + # TODO: While rolling out this code, setting the version via the docstore is optional, + # so if it hasn't been passed, just ignore it. Once the docupdater has totally + # handed control of this to the docstore, we can assume it will always be passed + # and an error guard on it not being set instead. + if version? + MongoManager.setDocVersion doc_id, version, (error) -> + return callback(error) if error? + callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc + else + callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc deleteDoc: (project_id, doc_id, callback = (error) ->) -> DocManager.getDoc project_id, doc_id, (error, doc) -> diff --git a/services/docstore/app/coffee/HttpController.coffee b/services/docstore/app/coffee/HttpController.coffee index 7d6ada30c8..0dfb30c31e 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -49,6 +49,7 @@ module.exports = HttpController = project_id = req.params.project_id doc_id = req.params.doc_id lines = req.body?.lines + version = req.body?.version if !lines? or lines not instanceof Array logger.error project_id: project_id, doc_id: doc_id, "no doc lines provided" @@ -56,7 +57,7 @@ module.exports = HttpController = return logger.log project_id: project_id, doc_id: doc_id, "got http request to update doc" - DocManager.updateDoc project_id, doc_id, lines, (error, modified, rev) -> + DocManager.updateDoc project_id, doc_id, lines, version, (error, modified, rev) -> return next(error) if error? res.json { modified: modified @@ -72,12 +73,15 @@ module.exports = HttpController = res.send 204 _buildDocView: (doc) -> - return { + doc_view = { _id: doc._id?.toString() lines: doc.lines rev: doc.rev deleted: !!doc.deleted } + if doc.version? + doc_view.version = doc.version + return doc_view _buildRawDocView: (doc)-> return (doc?.lines or []).join("\n") diff --git a/services/docstore/app/coffee/MongoManager.coffee b/services/docstore/app/coffee/MongoManager.coffee index 00dd935632..e44c995490 100644 --- a/services/docstore/app/coffee/MongoManager.coffee +++ b/services/docstore/app/coffee/MongoManager.coffee @@ -44,4 +44,25 @@ module.exports = MongoManager = _id: doc_id rev: rev db.docs.update query, update, (err)-> - callback(err) \ No newline at end of file + callback(err) + + getDocVersion: (doc_id, callback = (error, version) ->) -> + db.docOps.find { + doc_id: ObjectId(doc_id) + }, { + version: 1 + }, (error, docs) -> + return callback(error) if error? + if docs.length < 1 or !docs[0].version? + return callback null, 0 + else + return callback null, docs[0].version + + setDocVersion: (doc_id, version, callback = (error) ->) -> + db.docOps.update { + doc_id: ObjectId(doc_id) + }, { + $set: version: version + }, { + upsert: true + }, callback \ No newline at end of file diff --git a/services/docstore/app/coffee/mongojs.coffee b/services/docstore/app/coffee/mongojs.coffee index 9a8d6c38e5..db3d3c00d8 100644 --- a/services/docstore/app/coffee/mongojs.coffee +++ b/services/docstore/app/coffee/mongojs.coffee @@ -1,6 +1,6 @@ Settings = require "settings-sharelatex" mongojs = require "mongojs" -db = mongojs.connect(Settings.mongo.url, ["docs"]) +db = mongojs.connect(Settings.mongo.url, ["docs", "docOps"]) module.exports = db: db ObjectId: mongojs.ObjectId diff --git a/services/docstore/config/settings.defaults.coffee b/services/docstore/config/settings.defaults.coffee index e474aea7b9..e74334d2ad 100644 --- a/services/docstore/config/settings.defaults.coffee +++ b/services/docstore/config/settings.defaults.coffee @@ -1,7 +1,7 @@ http = require('http') http.globalAgent.maxSockets = 300 -module.exports = +module.exports = Settings = internal: docstore: port: 3016 @@ -12,10 +12,12 @@ module.exports = docstore: healthCheck: - project_id: "5620bece05509b0a7a3cbc61" - # s3: - # key: "" - # secret: "" - # bucket: "something" + project_id: "" max_doc_length: 2 * 1024 * 1024 # 2mb + +if process.env['AWS_ACCESS_KEY_ID']? and process.env['AWS_SECRET_ACCESS_KEY']? and process.env['AWS_BUCKET']? + Settings.docstore.s3 = + key: process.env['AWS_ACCESS_KEY_ID'] + secret: process.env['AWS_SECRET_ACCESS_KEY'] + bucket: process.env['AWS_BUCKET'] \ No newline at end of file diff --git a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee index 52d62eb2c6..26c57e7712 100644 --- a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee @@ -30,7 +30,7 @@ describe "Archiving all docs", -> (callback) => DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=> doc.lines[0] = doc.lines[0]+" added" - DocstoreClient.updateDoc @project_id, doc._id, doc.lines, callback + DocstoreClient.updateDoc @project_id, doc._id, doc.lines, null, callback async.series jobs, done afterEach (done) -> @@ -109,7 +109,7 @@ describe "Archiving all docs", -> quarterMegInBytes = 250000 lines = require("crypto").randomBytes(quarterMegInBytes).toString("hex") @docs[1].lines = [lines,lines,lines,lines] - DocstoreClient.updateDoc @project_id, @docs[1]._id, @docs[1].lines, => + DocstoreClient.updateDoc @project_id, @docs[1]._id, @docs[1].lines, null, => DocstoreClient.archiveAllDoc @project_id, (error, @res) => done() diff --git a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee index 4463abcdad..eb4261b7ae 100644 --- a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee @@ -27,7 +27,7 @@ describe "Getting all docs", -> (callback) => DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=> doc.lines[0] = doc.lines[0]+" added" - DocstoreClient.updateDoc @project_id, doc._id, doc.lines, callback + DocstoreClient.updateDoc @project_id, doc._id, doc.lines, null, callback async.series jobs, done it "should return all the docs", (done) -> diff --git a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee index b18be15c25..5f21f55845 100644 --- a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee @@ -11,13 +11,16 @@ describe "Applying updates to a doc", -> @doc_id = ObjectId() @originalLines = ["original", "lines"] @newLines = ["new", "lines"] - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + @version = 42 + DocstoreClient.createDoc @project_id, @doc_id, @originalLines, (error) => throw error if error? - done() + DocstoreClient.setDocVersion @doc_id, @version, (error) => + throw error if error? + done() describe "when the content has changed", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @newLines, (error, res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @newLines, null, (error, res, @body) => done() it "should return modified = true", -> @@ -26,14 +29,15 @@ describe "Applying updates to a doc", -> it "should return the rev", -> @body.rev.should.equal 2 - it "should update the doc in the API", (done) -> + it "should update the doc in the API but not change the version", (done) -> DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @newLines + doc.version.should.equal @version done() describe "when the content has not been updated", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, (error, res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, null, (error, res, @body) => done() it "should return modified = false", -> @@ -47,7 +51,7 @@ describe "Applying updates to a doc", -> describe "when the doc does not exist", -> beforeEach (done) -> @missing_doc_id = ObjectId() - DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, null, (error, @res, @body) => done() it "should create the doc", -> @@ -56,12 +60,13 @@ describe "Applying updates to a doc", -> it "should be retreivable", (done)-> DocstoreClient.getDoc @project_id, @missing_doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @originalLines + doc.version.should.equal 0 done() describe "when malformed doc lines are provided", -> describe "when the lines are not an array", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, null, (error, @res, @body) => done() it "should return 400", -> @@ -74,7 +79,7 @@ describe "Applying updates to a doc", -> describe "when the lines are not present", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, null, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, null, null, (error, @res, @body) => done() it "should return 400", -> @@ -89,7 +94,7 @@ describe "Applying updates to a doc", -> beforeEach (done) -> line = new Array(1025).join("x") # 1kb @largeLines = Array.apply(null, Array(1024)).map(() -> line) # 1mb - DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, (error, res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, null, (error, res, @body) => done() it "should return modified = true", -> @@ -100,3 +105,20 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @largeLines done() + describe "when the version has changed", -> + beforeEach (done) -> + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version + 1, (error, res, @body) => + done() + + it "should return modified = true", -> + @body.modified.should.equal true + + it "should return the rev", -> + @body.rev.should.equal 2 + + it "should update the doc in the API", (done) -> + DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => + doc.lines.should.deep.equal @originalLines + doc.version.should.equal @version + 1 + done() + diff --git a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee index b81408b6f3..6c12628826 100644 --- a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee +++ b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee @@ -7,6 +7,9 @@ module.exports = DocstoreClient = createDoc: (project_id, doc_id, lines, callback = (error) ->) -> db.docs.save({_id: doc_id, project_id:project_id, lines: lines, rev:1}, callback) + + setDocVersion: (doc_id, version, callback = (error) ->) -> + db.docOps.save({doc_id: doc_id, version: version}, callback) createDeletedDoc: (project_id, doc_id, lines, callback = (error) ->) -> db.docs.insert { @@ -29,11 +32,12 @@ module.exports = DocstoreClient = json: true }, callback - updateDoc: (project_id, doc_id, lines, callback = (error, res, body) ->) -> + updateDoc: (project_id, doc_id, lines, version, callback = (error, res, body) ->) -> request.post { url: "http://localhost:#{settings.internal.docstore.port}/project/#{project_id}/doc/#{doc_id}" json: lines: lines + version: version }, callback deleteDoc: (project_id, doc_id, callback = (error, res, body) ->) -> diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index c84f64b75b..d603f672d7 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -26,49 +26,59 @@ describe "DocManager", -> beforeEach -> @project = { name: "mock-project" } @doc = { _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] } - @docCollectionDoc = { _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] } - - describe "when the doc is in the doc collection not projects collection", -> + @version = 42 + @MongoManager.findDoc = sinon.stub() + @MongoManager.getDocVersion = sinon.stub().yields(null, @version) + describe "when the doc is in the doc collection", -> beforeEach -> - @MongoManager.findDoc = sinon.stub() + @MongoManager.findDoc.yields(null, @doc) + @DocManager.getDoc @project_id, @doc_id, @callback - it "should get the doc from the doc collection when it is present there", (done)-> - @MongoManager.findDoc.callsArgWith(2, null, @docCollectionDoc) - @DocManager.getDoc @project_id, @doc_id, (err, doc)=> - doc.should.equal @docCollectionDoc - done() + it "should get the doc from the doc collection", -> + @MongoManager.findDoc + .calledWith(@project_id, @doc_id) + .should.equal true - it "should return the error from find doc", (done)-> - @MongoManager.findDoc.callsArgWith(2, @stubbedError) - @DocManager.getDoc @project_id, @doc_id, (err, doc)=> - err.should.equal @stubbedError - done() + it "should get the doc version from the docOps collection", -> + @MongoManager.getDocVersion + .calledWith(@doc_id) + .should.equal true + + it "should return the callback with the doc with the version", -> + @callback.called.should.equal true + doc = @callback.args[0][1] + doc.lines.should.equal @doc.lines + doc.version.should.equal @version + + describe "when MongoManager.findDoc errors", -> + beforeEach -> + @MongoManager.findDoc.yields(@stubbedError) + @DocManager.getDoc @project_id, @doc_id, @callback - describe "when the doc is deleted", -> + it "should return the error", -> + @callback.calledWith(@stubbedError).should.equal true + + describe "when the doc is archived", -> beforeEach -> - @doc = { _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] } - @s3doc = { _id: @doc_id, project_id: @project_id, inS3: true } - @MongoManager.findDoc = sinon.stub() - @MongoManager.findDoc.callsArgWith(2, null, @s3doc) - @MongoManager.findDoc.callsArgWith(2, null, @doc) - spy = sinon.spy @DocManager.getDoc - @DocArchiveManager.unarchiveDoc = sinon.stub().callsArgWith(2) + @doc = { _id: @doc_id, project_id: @project_id, lines: ["mock-lines"], inS3: true } + @MongoManager.findDoc.yields(null, @doc) + @DocArchiveManager.unarchiveDoc = (project_id, doc_id, callback) => + @doc.inS3 = false + callback() + sinon.spy @DocArchiveManager, "unarchiveDoc" @DocManager.getDoc @project_id, @doc_id, @callback it "should call the DocArchive to unarchive the doc", -> @DocArchiveManager.unarchiveDoc.calledWith(@project_id, @doc_id).should.equal true + + it "should look up the doc twice", -> + @MongoManager.findDoc.calledTwice.should.equal true it "should return the doc", -> @callback.calledWith(null, @doc).should.equal true - describe "when the doc is in s3", -> - beforeEach -> - @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, @doc) - - @DocManager.getDoc @project_id, @doc_id, @callback - - describe "when the doc does not exist anywhere", -> + describe "when the doc does not exist in the docs collection", -> beforeEach -> @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, null) @DocManager.getDoc @project_id, @doc_id, @callback @@ -151,25 +161,32 @@ describe "DocManager", -> beforeEach -> @oldDocLines = ["old", "doc", "lines"] @newDocLines = ["new", "doc", "lines"] - @doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5 } + @version = 42 + @doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5, version: @version } @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3) - @MongoManager.findDoc = sinon.stub() + @MongoManager.setDocVersion = sinon.stub().yields() + @DocManager.getDoc = sinon.stub() describe "when the doc lines have changed", -> beforeEach -> - @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @callback + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should get the existing doc", -> - @MongoManager.findDoc + @DocManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true it "should upsert the document to the doc collection", -> @MongoManager.upsertIntoDocCollection .calledWith(@project_id, @doc_id, @newDocLines) - .should.equal true + .should.equal true + + it "should update the version", -> + @MongoManager.setDocVersion + .calledWith(@doc_id, @version) + .should.equal true it "should log out the old and new doc lines", -> @logger.log @@ -186,12 +203,71 @@ describe "DocManager", -> it "should return the callback with the new rev", -> @callback.calledWith(null, true, @rev + 1).should.equal true - describe "when there is a generic error getting the doc", -> + describe "when the version has changed", -> + beforeEach -> + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @callback + it "should get the existing doc", -> + @DocManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should upsert the document to the doc collection", -> + @MongoManager.upsertIntoDocCollection + .calledWith(@project_id, @doc_id, @oldDocLines) + .should.equal true + + it "should update the version", -> + @MongoManager.setDocVersion + .calledWith(@doc_id, @version + 1) + .should.equal true + + it "should return the callback with the new rev", -> + @callback.calledWith(null, true, @rev + 1).should.equal true + + describe "when the version is null and the lines are different", -> + beforeEach -> + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @callback + + it "should get the existing doc", -> + @DocManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should upsert the document to the doc collection", -> + @MongoManager.upsertIntoDocCollection + .calledWith(@project_id, @doc_id, @newDocLines) + .should.equal true + + it "should not update the version", -> + @MongoManager.setDocVersion + .called + .should.equal false + + it "should return the callback with the new rev", -> + @callback.calledWith(null, true, @rev + 1).should.equal true + + describe "when the version is null and the lines are the same", -> + beforeEach -> + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, null, @callback + + it "should not update the version", -> + @MongoManager.setDocVersion.called.should.equal false + + it "should not update the doc", -> + @MongoManager.upsertIntoDocCollection.called.should.equal false + + it "should return the callback with the existing rev", -> + @callback.calledWith(null, false, @rev).should.equal true + + describe "when there is a generic error getting the doc", -> beforeEach -> @error = new Error("doc could not be found") - @MongoManager.findDoc = sinon.stub().callsArgWith(2, @error, null, null) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @callback + @DocManager.getDoc = sinon.stub().callsArgWith(2, @error, null, null) + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should not upsert the document to the doc collection", -> @MongoManager.upsertIntoDocCollection.called.should.equal false @@ -201,8 +277,8 @@ describe "DocManager", -> describe "when the doc lines have not changed", -> beforeEach -> - @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @callback + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @callback it "should not update the doc", -> @MongoManager.upsertIntoDocCollection.called.should.equal false @@ -210,12 +286,10 @@ describe "DocManager", -> it "should return the callback with the existing rev", -> @callback.calledWith(null, false, @rev).should.equal true - describe "when the doc lines are an empty array", -> beforeEach -> - @doc.lines = [] - @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @doc.lines, @callback it "should upsert the document to the doc collection", -> @@ -223,19 +297,15 @@ describe "DocManager", -> .calledWith(@project_id, @doc_id, @doc.lines) .should.equal true - describe "when the doc does not exist", -> beforeEach -> - @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, null, null) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @callback + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, null, null) + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should upsert the document to the doc collection", -> @MongoManager.upsertIntoDocCollection .calledWith(@project_id, @doc_id, @newDocLines) - .should.equal true + .should.equal true it "should return the callback with the new rev", -> @callback.calledWith(null, true, 1).should.equal true - - - diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index c76ced9696..4d9ae795ab 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -13,6 +13,7 @@ describe "HttpController", -> "./DocManager": @DocManager = {} "./DocArchiveManager": @DocArchiveManager = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } + "./HealthChecker": {} @res = { send: sinon.stub(), json: sinon.stub(), setHeader:sinon.stub() } @req = { query:{}} @next = sinon.stub() @@ -54,6 +55,7 @@ describe "HttpController", -> lines: @doc.lines rev: @doc.rev deleted: false + version: @doc.version }) .should.equal true @@ -81,6 +83,7 @@ describe "HttpController", -> lines: @doc.lines rev: @doc.rev deleted: true + version: @doc.version }) .should.equal true @@ -192,12 +195,12 @@ describe "HttpController", -> beforeEach -> @req.body = lines: @lines = ["hello", "world"] - @DocManager.updateDoc = sinon.stub().callsArgWith(3, null, true, @rev = 5) + @DocManager.updateDoc = sinon.stub().yields(null, true, @rev = 5) @HttpController.updateDoc @req, @res, @next it "should update the document", -> @DocManager.updateDoc - .calledWith(@project_id, @doc_id, @lines) + .calledWith(@project_id, @doc_id, @lines, undefined) .should.equal true it "should return a modified status", -> @@ -209,7 +212,7 @@ describe "HttpController", -> beforeEach -> @req.body = lines: @lines = ["hello", "world"] - @DocManager.updateDoc = sinon.stub().callsArgWith(3, null, false, @rev = 5) + @DocManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) @HttpController.updateDoc @req, @res, @next it "should return a modified status", -> @@ -220,7 +223,7 @@ describe "HttpController", -> describe "when the doc lines are not provided", -> beforeEach -> @req.body = {} - @DocManager.updateDoc = sinon.stub().callsArgWith(3, null, false) + @DocManager.updateDoc = sinon.stub().yields(null, false) @HttpController.updateDoc @req, @res, @next it "should not update the document", -> @@ -231,6 +234,24 @@ describe "HttpController", -> .calledWith(400) .should.equal true + describe "when the doc version is provided", -> + beforeEach -> + @req.body = + lines: @lines = ["hello", "world"] + version: @version = 42 + @DocManager.updateDoc = sinon.stub().yields(null, true, @rev = 5) + @HttpController.updateDoc @req, @res, @next + + it "should update the document with the lines and version", -> + @DocManager.updateDoc + .calledWith(@project_id, @doc_id, @lines, @version) + .should.equal true + + it "should return a modified status", -> + @res.json + .calledWith(modified: true, rev: @rev) + .should.equal true + describe "deleteDoc", -> beforeEach -> @req.params = diff --git a/services/docstore/test/unit/coffee/MongoManagerTests.coffee b/services/docstore/test/unit/coffee/MongoManagerTests.coffee index b64473b5ec..54a352417a 100644 --- a/services/docstore/test/unit/coffee/MongoManagerTests.coffee +++ b/services/docstore/test/unit/coffee/MongoManagerTests.coffee @@ -9,7 +9,7 @@ describe "MongoManager", -> beforeEach -> @MongoManager = SandboxedModule.require modulePath, requires: "./mongojs": - db: @db = { docs: {} } + db: @db = { docs: {}, docOps: {} } ObjectId: ObjectId @project_id = ObjectId().toString() @doc_id = ObjectId().toString() @@ -88,4 +88,47 @@ describe "MongoManager", -> err.should.equal @stubbedErr done() - \ No newline at end of file + describe "getDocVersion", -> + describe "when the doc exists", -> + beforeEach -> + @doc = + version: @version = 42 + @db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc]) + @MongoManager.getDocVersion @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, []) + @MongoManager.getDocVersion @doc_id, @callback + + it "should call the callback with 0", -> + @callback.calledWith(null, 0).should.equal true + + describe "setDocVersion", -> + beforeEach -> + @version = 42 + @db.docOps.update = sinon.stub().callsArg(3) + @MongoManager.setDocVersion @doc_id, @version, @callback + + it "should update the doc version", -> + @db.docOps.update + .calledWith({ + doc_id: ObjectId(@doc_id) + }, { + $set: + version: @version + }, { + upsert: true + }) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true \ No newline at end of file