diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index 1f8bf64514..7802482961 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -21,23 +21,29 @@ module.exports = DocManager = return callback(error) if error? return callback null, docs - updateDoc: (project_id, doc_id, lines, callback = (error, modified) ->) -> + updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) -> DocManager.getDoc project_id, doc_id, (error, doc, mongoPath) -> return callback(error) if error? return callback new Errors.NotFoundError("No such project/doc: #{project_id}/#{doc_id}") if !doc? - if _.isEqual(doc.lines, lines) + if _.isEqual(doc.lines, lines) and doc.version == version logger.log { - project_id: project_id, doc_id: doc_id, rev: doc.rev - }, "doc lines have not changed" - return callback null, false + project_id: project_id, doc_id: doc_id, rev: doc.rev, version: doc.version + }, "doc lines and version have not changed" + return callback null, false, doc.rev else logger.log { - project_id: project_id, doc_id: doc_id, oldDocLines: doc.lines, newDocLines: lines, rev: doc.rev + project_id: project_id + doc_id: doc_id, + oldDocLines: doc.lines + newDocLines: lines + rev: doc.rev + oldVersion: doc.version + newVersion: version }, "updating doc lines" - MongoManager.updateDoc project_id, mongoPath, lines, (error) -> + MongoManager.updateDoc project_id, mongoPath, lines, version, (error) -> return callback(error) if error? - callback null, true + callback null, true, doc.rev + 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 4a99b3a0cc..1e281d451e 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -30,17 +30,19 @@ 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" res.send 400 # Bad Request return - logger.log project_id: project_id, doc_id: doc_id, "updating doc" - DocManager.updateDoc project_id, doc_id, lines, (error, modified) -> + logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc" + DocManager.updateDoc project_id, doc_id, lines, version, (error, modified, rev) -> return next(error) if error? res.json { modified: modified + rev: rev } deleteDoc: (req, res, next = (error) ->) -> diff --git a/services/docstore/app/coffee/MongoManager.coffee b/services/docstore/app/coffee/MongoManager.coffee index 23e98e69c3..6cd05902c2 100644 --- a/services/docstore/app/coffee/MongoManager.coffee +++ b/services/docstore/app/coffee/MongoManager.coffee @@ -5,11 +5,12 @@ module.exports = MongoManager = db.projects.find _id: ObjectId(project_id.toString()), {}, (error, projects = []) -> callback error, projects[0] - updateDoc: (project_id, docPath, lines, callback = (error) ->) -> + updateDoc: (project_id, docPath, lines, version, callback = (error) ->) -> update = $set: {} $inc: {} update.$set["#{docPath}.lines"] = lines + update.$set["#{docPath}.version"] = version if version? update.$inc["#{docPath}.rev"] = 1 db.projects.update _id: ObjectId(project_id), update, callback diff --git a/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee b/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee index a1dbe937da..05a9a1bb2a 100644 --- a/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee @@ -10,9 +10,10 @@ describe "Deleting a doc", -> @project_id = ObjectId() @doc_id = ObjectId() @lines = ["original", "lines"] + @version = 42 DocstoreClient.createProject @project_id, (error) => throw error if error? - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, (error) => throw error if error? done() diff --git a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee index 568e8f9554..c3ec641312 100644 --- a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee @@ -29,7 +29,7 @@ describe "Getting all docs", -> throw error if error? jobs = for doc in @docs do (doc) => - (callback) => DocstoreClient.createDoc @project_id, doc._id, doc.lines, callback + (callback) => DocstoreClient.createDoc @project_id, doc._id, doc.lines, doc.version, callback async.series jobs, done afterEach (done) -> diff --git a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee index 248286b8e4..b93b07255e 100644 --- a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee @@ -10,9 +10,10 @@ describe "Getting a doc", -> @project_id = ObjectId() @doc_id = ObjectId() @lines = ["original", "lines"] + @version = 42 DocstoreClient.createProject @project_id, (error) => throw error if error? - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, (error) => throw error if error? done() @@ -20,9 +21,10 @@ describe "Getting a doc", -> DocstoreClient.deleteProject @project_id, done describe "when the doc exists", -> - it "should get the doc lines", (done) -> + it "should get the doc lines and version", (done) -> DocstoreClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @lines + doc.version.should.equal @version done() describe "when the doc does not exist", -> diff --git a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee index 740aadb446..924acb3373 100644 --- a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee @@ -11,9 +11,11 @@ describe "Applying updates to a doc", -> @doc_id = ObjectId() @originalLines = ["original", "lines"] @newLines = ["new", "lines"] + @originalVersion = 42 + @newVersion = 53 DocstoreClient.createProject @project_id, (error) => throw error if error? - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, (error) => throw error if error? done() @@ -22,7 +24,7 @@ describe "Applying updates to a doc", -> 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, @newVersion, (error, res, @body) => done() it "should return modified = true", -> @@ -31,11 +33,12 @@ describe "Applying updates to a doc", -> it "should update the doc in the API", (done) -> DocstoreClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @newLines + doc.version.should.deep.equal @newVersion 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, @originalVersion, (error, res, @body) => done() it "should return modified = false", -> @@ -44,12 +47,13 @@ describe "Applying updates to a doc", -> it "should not 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.deep.equal @originalVersion done() 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, @newVersion, (error, @res, @body) => done() it "should return a 404", -> @@ -58,7 +62,7 @@ describe "Applying updates to a doc", -> describe "when the project does not exist", -> beforeEach (done) -> missing_project_id = ObjectId() - DocstoreClient.updateDoc missing_project_id, @doc_id, @originalLines, (error, @res, @body) => + DocstoreClient.updateDoc missing_project_id, @doc_id, @originalLines, @newVersion, (error, @res, @body) => done() it "should return a 404", -> @@ -67,7 +71,7 @@ describe "Applying updates to a doc", -> 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" }, @newVersion, (error, @res, @body) => done() it "should return 400", -> @@ -80,7 +84,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, @newVersion, (error, @res, @body) => done() it "should return 400", -> diff --git a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee index 7640501537..812fd5c822 100644 --- a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee +++ b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee @@ -8,7 +8,7 @@ module.exports = DocstoreClient = rootFolder: [{ docs: [] }] }, callback - createDoc: (project_id, doc_id, lines, callback = (error) ->) -> + createDoc: (project_id, doc_id, lines, version, callback = (error) ->) -> db.projects.update { _id: project_id }, { @@ -16,6 +16,7 @@ module.exports = DocstoreClient = "rootFolder.0.docs": { _id: doc_id lines: lines + version: version } } }, callback @@ -35,11 +36,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:3016/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 0b2b70c38f..18f0758d18 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -149,15 +149,15 @@ describe "DocManager", -> beforeEach -> @oldDocLines = ["old", "doc", "lines"] @newDocLines = ["new", "doc", "lines"] - @doc = { _id: @doc_id, lines: @oldDocLines, rev: 42 } + @doc = { _id: @doc_id, lines: @oldDocLines, rev: @rev = 5, version: @version = 42 } @mongoPath = "mock.mongo.path" - @MongoManager.updateDoc = sinon.stub().callsArg(3) + @MongoManager.updateDoc = sinon.stub().callsArg(4) describe "when the doc lines have changed", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc, @mongoPath) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should get the existing doc", -> @DocManager.getDoc @@ -166,7 +166,7 @@ describe "DocManager", -> it "should update the doc with the new doc lines", -> @MongoManager.updateDoc - .calledWith(@project_id, @mongoPath, @newDocLines) + .calledWith(@project_id, @mongoPath, @newDocLines, @version) .should.equal true it "should log out the old and new doc lines", -> @@ -177,28 +177,63 @@ describe "DocManager", -> oldDocLines: @oldDocLines newDocLines: @newDocLines rev: @doc.rev + oldVersion: @version + newVersion: @version "updating doc lines" ) .should.equal true - it "should return the callback", -> - @callback.calledWith(null, true).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 has changed", -> + beforeEach -> + @newVersion = 1003 + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc, @mongoPath) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @newVersion, @callback + + it "should get the existing doc", -> + @DocManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should update the doc with the new version", -> + @MongoManager.updateDoc + .calledWith(@project_id, @mongoPath, @oldDocLines, @newVersion) + .should.equal true + + it "should log out the old and new doc lines", -> + @logger.log + .calledWith( + project_id: @project_id + doc_id: @doc_id + oldDocLines: @oldDocLines + newDocLines: @oldDocLines + rev: @doc.rev + oldVersion: @version + newVersion: @newVersion + "updating doc lines" + ) + .should.equal true + + it "should return the callback with the new rev", -> + @callback.calledWith(null, true, @rev + 1).should.equal true describe "when the doc lines have not changed", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc, @mongoPath) - @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @callback + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @callback it "should not update the doc", -> @MongoManager.updateDoc.called.should.equal false - it "should return the callback", -> - @callback.calledWith(null, false).should.equal true + it "should return the callback with the existing rev", -> + @callback.calledWith(null, false, @rev).should.equal true describe "when the doc does not exist", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(2, null, null, null) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should not try to update the doc", -> @MongoManager.updateDoc.called.should.equal false diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index 6d40a20612..b1d700608b 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -139,29 +139,30 @@ describe "HttpController", -> beforeEach -> @req.body = lines: @lines = ["hello", "world"] - @DocManager.updateDoc = sinon.stub().callsArgWith(3, null, true) + version: @version = 42 + @DocManager.updateDoc = sinon.stub().callsArgWith(4, 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, @version) .should.equal true it "should return a modified status", -> @res.json - .calledWith(modified: true) + .calledWith(modified: true, rev: @rev) .should.equal true describe "when the doc lines exist and were not updated", -> beforeEach -> @req.body = lines: @lines = ["hello", "world"] - @DocManager.updateDoc = sinon.stub().callsArgWith(3, null, false) + @DocManager.updateDoc = sinon.stub().callsArgWith(4, null, false, @rev = 5) @HttpController.updateDoc @req, @res, @next it "should return a modified status", -> @res.json - .calledWith(modified: false) + .calledWith(modified: false, rev: @rev) .should.equal true describe "when the doc lines are not provided", -> diff --git a/services/docstore/test/unit/coffee/MongoManagerTests.coffee b/services/docstore/test/unit/coffee/MongoManagerTests.coffee index d8c7411eec..e2ca10d531 100644 --- a/services/docstore/test/unit/coffee/MongoManagerTests.coffee +++ b/services/docstore/test/unit/coffee/MongoManagerTests.coffee @@ -31,10 +31,11 @@ describe "MongoManager", -> describe "updateDoc", -> beforeEach -> + @version = 42 @lines = ["mock-lines"] @docPath = "rootFolder.0.folders.1.docs.0" @db.projects.update = sinon.stub().callsArg(2) - @MongoManager.updateDoc @project_id, @docPath, @lines, @callback + @MongoManager.updateDoc @project_id, @docPath, @lines, @version, @callback it "should update the doc lines and increment the TPDS rev", -> @db.projects.update @@ -43,6 +44,7 @@ describe "MongoManager", -> }, { $set: "rootFolder.0.folders.1.docs.0.lines": @lines + "rootFolder.0.folders.1.docs.0.version": @version $inc: "rootFolder.0.folders.1.docs.0.rev": 1 })