Просмотр исходного кода

Merge pull request #11 from sharelatex/ja-set-version-in-docstore

Set version in docstore, not doc-updater
James Allen 10 лет назад
Родитель
Сommit
0677bcb110

+ 10 - 2
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

+ 26 - 5
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
@@ -51,10 +61,21 @@ module.exports = DocManager =
 					oldDocLines: doc?.lines
 					newDocLines: lines
 					rev: oldRev
+					oldVersion: doc?.version
+					newVersion: version
 				}, "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) ->

+ 6 - 2
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")

+ 22 - 1
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)
+			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

+ 1 - 1
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

+ 2 - 2
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()
 		

+ 1 - 1
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) ->

+ 31 - 9
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()
+

+ 5 - 1
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) ->) ->

+ 122 - 50
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
+
+			it "should return the error", ->
+				@callback.calledWith(@stubbedError).should.equal true
 
-		describe "when the doc is deleted", ->
+		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
@@ -179,6 +196,8 @@ describe "DocManager", ->
 						oldDocLines: @oldDocLines
 						newDocLines: @newDocLines
 						rev: @doc.rev
+						oldVersion: @version
+						newVersion: @version
 						"updating doc lines"
 					)
 					.should.equal true
@@ -186,12 +205,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 +279,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 +288,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 +299,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
-
-
-

+ 25 - 4
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 =

+ 45 - 2
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()
 
-	
+	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