Use docstore for updating documents

services/web/app/coffee/Features/Docstore/DocstoreManager.coffee

@@ -46,7 +46,7 @@ module.exports = DocstoreManager =
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore"
 				callback(error)
 
-	updateDoc: (project_id, doc_id, lines, version, callback = (error, rev) ->) ->
+	updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) ->
 		logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc in docstore api"
 		url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}"
 		request.post {
@@ -54,10 +54,10 @@ module.exports = DocstoreManager =
 			json:
 				lines: lines
 				version: version
-		}, (error, res, body) ->
+		}, (error, res, result) ->
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300
-				callback(null)
+				callback(null, result.modified, result.rev)
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error updating doc in docstore"

services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee

@@ -58,6 +58,22 @@ module.exports =
 				error = new Error("document updater returned a failure status code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, sl_req_id: sl_req_id, "document updater returned failure status code: #{res.statusCode}"
 				return callback(error)
+
+	flushDocToMongo: (project_id, doc_id, callback = (error) ->) ->
+		logger.log project_id:project_id, doc_id: doc_id, "flushing doc from document updater"
+		timer = new metrics.Timer("flushing.mongo.doc")
+		url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/flush"
+		request.post url, (error, res, body)->
+			if error?
+				logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc from document updater"
+				return callback(error)
+			else if res.statusCode >= 200 and res.statusCode < 300
+				logger.log project_id: project_id, doc_id: doc_id, "flushed doc from document updater"
+				return callback(null)
+			else
+				error = new Error("document updater returned a failure status code: #{res.statusCode}")
+				logger.error err: error, project_id: project_id, doc_id: doc_id, "document updater returned failure status code: #{res.statusCode}"
+				return callback(error)
 	
 	deleteDoc : (project_id, doc_id, sl_req_id, callback = ()->)->
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)

services/web/app/coffee/Features/Documents/DocumentController.coffee

@@ -22,8 +22,9 @@ module.exports =
 		project_id = req.params.Project_id
 		doc_id = req.params.doc_id
 		lines = req.body.lines
+		version = req.body.version
 		logger.log doc_id:doc_id, project_id:project_id, "receiving set document request from api (docupdater)"
-		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, (error) ->
+		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, version, (error) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)

services/web/app/coffee/Features/Editor/EditorController.coffee

@@ -158,8 +158,7 @@ module.exports = EditorController =
 		DocumentUpdaterHandler.setDocument project_id, doc_id, docLines, (err)=>
 			logger.log project_id:project_id, doc_id:doc_id, "notifying users that the document has been updated"
 			EditorRealTimeController.emitToRoom(project_id, "entireDocUpdate", doc_id)
-			ProjectEntityHandler.updateDocLines project_id, doc_id, docLines, sl_req_id, (err)->
-				callback(err)
+			DocumentUpdaterHandler.flushDocToMongo project_id, doc_id, callback
 
 	addDoc: (project_id, folder_id, docName, docLines, sl_req_id, callback = (error, doc)->)->
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -123,7 +123,7 @@ module.exports = ProjectEntityHandler =
 				doc = new Doc name: docName
 				Project.putElement project._id, folder_id, doc, "doc", (err, result)=>
 					return callback(err) if err?
-					DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, rev) ->
+					DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, modified, rev) ->
 						return callback(err) if err? 
 						tpdsUpdateSender.addDoc {
 							project_id:   project._id,
@@ -234,45 +234,31 @@ module.exports = ProjectEntityHandler =
 					if callback?
 						callback(err, folder, parentFolder_id)
 
-	updateDocLines : (project_or_id, doc_id, docLines, sl_req_id, callback = (error) ->)->
-		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)
-		Project.getProject project_or_id, "", (err, project)->
+	updateDocLines : (project_id, doc_id, lines, version, callback = (error) ->)->
+		ProjectGetter.getProjectWithoutDocLines project_id, (err, project)->
 			return callback(err) if err?
 			return callback(new Errors.NotFoundError("project not found")) if !project?
-			project_id = project._id
-			if err?
-				logger.err err:err,project_id:project_id, "error finding project"
-				callback err
-			else if !project?
-				logger.err project_id:project_id, doc_id:doc_id, err: new Error("project #{project_id} could not be found for doc #{doc_id}")
-				callback "could not find project #{project_id}"
-			else
-				projectLocator.findElement {project:project, element_id:doc_id, type:"docs"}, (err, doc, path)->
+			logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc lines"
+			projectLocator.findElement {project:project, element_id:doc_id, type:"docs"}, (err, doc, path)->
+				if err?
+					logger.error err: err, doc_id: doc_id, project_id: project_id, version: version, lines: lines, "error finding doc while updating doc lines"
+					return callback err
+				if !doc?
+					error = new Errors.NotFoundError("doc not found")
+					logger.error err: error, doc_id: doc_id, project_id: project_id, version: version, lines: lines, "doc not found while updating doc lines"
+					return callback(error)
+
+				DocstoreManager.updateDoc project_id, doc_id, lines, version, (err, modified, rev) ->
 					if err?
-						logger.err "error putting doc #{doc_id} in project #{project_id} #{err}"
-						callback err
-					else if docComparitor.areSame docLines, doc.lines
-						logger.log sl_req_id: sl_req_id, project_id:project_id, doc_id:doc_id, rev:doc.rev, "old doc lines are same as the new doc lines, not updating them"
-						callback()
+						logger.error err: err, doc_id: doc_id, project_id:project_id, lines: lines, version: version, "error sending doc to docstore"
+						return callback(err)
+
+					if modified
+						# Don't need to block for marking as updated
+						projectUpdateHandler.markAsUpdated project_id
+						tpdsUpdateSender.addDoc {project_id:project_id, path:path.fileSystem, docLines:lines, project_name:project.name, rev:rev}, callback
 					else
-						logger.log sl_req_id: sl_req_id, project_id:project_id, doc_id:doc_id, docLines: docLines, oldDocLines: doc.lines, rev:doc.rev, "updating doc lines"
-						conditons = _id:project_id
-						update = {$set:{}, $inc:{}}
-						changeLines = {}
-						changeLines["#{path.mongo}.lines"] = docLines
-						inc = {}
-						inc["#{path.mongo}.rev"] = 1
-						update["$set"] = changeLines
-						update["$inc"] = inc
-						Project.update conditons, update, {}, (err, second)->
-							if(err)
-								logger.err(sl_req_id:sl_req_id, doc_id:doc_id, project_id:project_id, err:err, "error saving doc to mongo")
-								callback(err)
-							else
-								logger.log sl_req_id:sl_req_id, doc_id:doc_id, project_id:project_id, newDocLines:docLines, oldDocLines:doc.lines,	 "doc saved to mongo"
-								rev = doc.rev+1
-								projectUpdateHandler.markAsUpdated project_id
-							tpdsUpdateSender.addDoc {project_id:project_id, path:path.fileSystem, docLines:docLines, project_name:project.name, rev:rev}, sl_req_id, callback
+						callback()
 
 	moveEntity: (project_id, entity_id, folder_id, entityType, sl_req_id, callback = (error) ->)->
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)

services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee

@@ -56,10 +56,12 @@ describe "DocstoreManager", ->
 		beforeEach ->
 			@lines = ["mock", "doc", "lines"]
 			@version = 42
+			@rev = 5
+			@modified = true
 
 		describe "with a successful response code", ->
 			beforeEach ->
-				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, "")
+				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, { modified: @modified, rev: @rev })
 				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
 
 			it "should update the doc in the docstore api", ->
@@ -72,8 +74,8 @@ describe "DocstoreManager", ->
 					})
 					.should.equal true
 
-			it "should call the callback without an error", ->
-				@callback.calledWith(null).should.equal true
+			it "should call the callback with the modified status and revision", ->
+				@callback.calledWith(null, @modified, @rev).should.equal true
 
 		describe "with a failed response code", ->
 			beforeEach ->

services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee

@@ -156,6 +156,40 @@ describe 'Flushing documents :', ->
 					.calledWith(new Error("doc updater returned failure status code: 500"))
 					.should.equal true
 
+	describe 'flushDocToMongo', ->
+		beforeEach ->
+			@callback = sinon.stub()
+
+		describe "successfully", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "")
+				@handler.flushDocToMongo @project_id, @doc_id, @callback
+
+			it 'should flush the document from the document updater', ->
+				url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/flush"
+				@request.post.calledWith(url).should.equal true
+
+			it "should call the callback with no error", ->
+				@callback.calledWith(null).should.equal true
+
+		describe "when the document updater API returns an error", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null)
+				@handler.flushDocToMongo @project_id, @doc_id, @callback
+
+			it "should return an error to the callback", ->
+				@callback.calledWith(@error).should.equal true
+
+		describe "when the document updater returns a failure error code", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "")
+				@handler.flushDocToMongo @project_id, @doc_id, @callback
+
+			it "should return the callback with an error", ->
+				@callback
+					.calledWith(new Error("doc updater returned failure status code: 500"))
+					.should.equal true
+
 	describe "deleteDoc", ->
 		beforeEach ->
 			@callback = sinon.stub()

services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee

@@ -61,14 +61,15 @@ describe "DocumentController", ->
 
 		describe "when the document exists", ->
 			beforeEach ->
-				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(3)
+				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(4)
 				@req.body =
 					lines: @doc_lines
+					version: @version
 				@DocumentController.setDocument(@req, @res, @next)
 
 			it "should update the document in Mongo", ->
 				@ProjectEntityHandler.updateDocLines
-					.calledWith(@project_id, @doc_id, @doc_lines)
+					.calledWith(@project_id, @doc_id, @doc_lines, @version)
 					.should.equal true
 
 			it "should return a successful response", ->
@@ -76,9 +77,10 @@ describe "DocumentController", ->
 
 		describe "when the document doesn't exist", ->
 			beforeEach ->
-				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArgWith(3, new Errors.NotFoundError("document does not exist"))
+				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArgWith(4, new Errors.NotFoundError("document does not exist"))
 				@req.body =
 					lines: @doc_lines
+					version: @version
 				@DocumentController.setDocument(@req, @res, @next)
 
 			it "should call next with the NotFoundError", ->

services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee

@@ -372,7 +372,7 @@ describe "EditorController", ->
 	describe 'set document', ->
 		beforeEach ->
 			@docLines = ["foo", "bar"]
-			@ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(4)
+			@DocumentUpdaterHandler.flushDocToMongo = sinon.stub().callsArg(2)
 			@DocumentUpdaterHandler.setDocument = sinon.stub().callsArg(3)
 			@EditorRealTimeController.emitToRoom = sinon.stub()
 
@@ -394,12 +394,9 @@ describe "EditorController", ->
 				mock.verify()
 				done()
 
-		it 'should update the document lines', (done)->
-			@ProjectEntityHandler.updateDocLines = ->
-			mock = sinon.mock(@ProjectEntityHandler).expects("updateDocLines").withArgs(@project_id, @doc_id, @docLines).once().callsArg(4)
-
-			@EditorController.setDoc @project_id, @doc_id, @docLines, (err)->
-				mock.verify()
+		it 'should flush the doc to mongo', (done)->
+			@EditorController.setDoc @project_id, @doc_id, @docLines, (err)=>
+				@DocumentUpdaterHandler.flushDocToMongo.calledWith(@project_id, @doc_id).should.equal true
 				done()
 
 

services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee

@@ -7,6 +7,7 @@ modulePath = "../../../../app/js/Features/Project/ProjectEntityHandler"
 SandboxedModule = require('sandboxed-module')
 ObjectId = require("mongoose").Types.ObjectId
 tk = require 'timekeeper'
+Errors = require "../../../../app/js/errors"
 
 describe 'ProjectEntityHandler', ->
 	project_id = '4eecb1c1bffa66588e0000a1'
@@ -65,7 +66,7 @@ describe 'ProjectEntityHandler', ->
 			'./ProjectLocator':@projectLocator = {}
 			'../../Features/DocumentUpdater/DocumentUpdaterHandler':@documentUpdaterHandler = {}
 			'../Docstore/DocstoreManager': @DocstoreManager = {}
-			'logger-sharelatex':{log:->}
+			'logger-sharelatex': @logger = {log:sinon.stub(), error: sinon.stub()}
 			'./ProjectUpdateHandler': @projectUpdater
 			"./ProjectGetter": @ProjectGetter = {}
 
@@ -305,7 +306,7 @@ describe 'ProjectEntityHandler', ->
 			@ProjectModel.putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}})
 			@callback = sinon.stub()
 			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(2)
-			@DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, 0)
+			@DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, true, 0)
 
 			@ProjectEntityHandler.addDoc project_id, folder_id, @name, @lines, @callback
 
@@ -457,54 +458,105 @@ describe 'ProjectEntityHandler', ->
 				done()
 
 
-	describe 'updating document lines', ->
-		docId = "123456"
-		docLines = ['1234','abc', '543543']
-		mongoPath = "folders[0].folders[5]"
-		fileSystemPath = "/somehwere/something.tex"
+	describe 'updateDocLines', ->
+		beforeEach ->
+			@lines = ['mock', 'doc', 'lines']
+			@path = "/somewhere/something.tex"
+			@doc = {
+				_id: doc_id
+			}
+			@version = 42
+			@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project)
+			@projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, {fileSystem: @path})
+			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(1)
+			@projectUpdater.markAsUpdated = sinon.stub()
+			@callback = sinon.stub()
 
-		it 'should find project via getProject', (done)->
-			@ProjectModel.getProject = (passedProject_id, callback)->
-				passedProject_id.should.equal project_id
-				done()
+		describe "when the doc has been modified", ->
+			beforeEach ->
+				@DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, true, @rev = 5)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
 
-			@ProjectEntityHandler.updateDocLines project_id, "", [], ->
+			it "should get the project without doc lines", ->
+				@ProjectGetter.getProjectWithoutDocLines
+					.calledWith(project_id)
+					.should.equal true
 
-		it 'should find the doc', (done)->
-			
-			@projectLocator.findElement = (options, callback)->
-				options.element_id.should.equal docId
-				options.type.should.equal 'docs'
-				done()
+			it "should find the doc", ->
+				@projectLocator.findElement
+					.calledWith({
+						project: @project
+						type: "docs"
+						element_id: doc_id
+					})
+					.should.equal true
 
-			@ProjectEntityHandler.updateDocLines project_id, docId, "", ->
+			it "should update the doc in the docstore", ->
+				@DocstoreManager.updateDoc
+					.calledWith(project_id, doc_id, @lines, @version)
+					.should.equal true
 
-		it 'should build mongo update statment', (done)->
-			@projectLocator.findElement = (opts, callback)->
-				callback(null, {lines:[], rev:0}, {mongo:mongoPath})
+			it "should mark the project as updated", ->
+				@projectUpdater.markAsUpdated
+					.calledWith(project_id)
+					.should.equal true
 
-			@ProjectModel.update = (conditions, update, options, callback)->
-				conditions._id.should.equal project_id
-				update.$set["#{mongoPath}.lines"].should.equal docLines
-				update.$inc["#{mongoPath}.rev"].should.equal 1
-				done()
+			it "should send the doc the to the TPDS", ->
+				@tpdsUpdateSender.addDoc
+					.calledWith({
+						project_id: project_id
+						project_name: @project.name
+						docLines: @lines
+						rev: @rev
+						path: @path
+					})
+					.should.equal true
 
-			@ProjectEntityHandler.updateDocLines project_id, docId, docLines, ->
+			it "should call the callback", ->
+				@callback.called.should.equal true
+
+		describe "when the doc has not been modified", ->
+			beforeEach ->
+				@DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, false, @rev = 5)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+
+			it "should not mark the project as updated", ->
+				@projectUpdater.markAsUpdated.called.should.equal false
+
+			it "should not send the doc the to the TPDS", ->
+				@tpdsUpdateSender.addDoc.called.should.equal false
+
+			it "should call the callback", ->
+				@callback.called.should.equal true
+
+		describe "when the project is not found", ->
+			beforeEach ->
+				@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, null)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+
+			it "should return a not found error", ->
+				@callback.calledWith(new Errors.NotFoundError()).should.equal true
+
+		describe "when the doc is not found", ->
+			beforeEach ->
+				@projectLocator.findElement = sinon.stub().callsArgWith(1, null, null, null)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+
+			it "should log out the error", ->
+				@logger.error
+					.calledWith(
+						project_id: project_id
+						doc_id: doc_id
+						version: @version
+						lines: @lines
+						err: new Errors.NotFoundError("doc not found")
+						"doc not found while updating doc lines"
+					)
+					.should.equal true
+
+			it "should return a not found error", ->
+				@callback.calledWith(new Errors.NotFoundError()).should.equal true
 
-		it 'should call third party data store ', (done)->
-			rev = 3
-			@projectLocator.findElement = (opts, callback)->
-				callback(null, {lines:[],rev:rev}, {fileSystem:fileSystemPath})
-			@ProjectModel.update = (conditions, update, options, callback)-> callback()
-			@tpdsUpdateSender.addDoc = (options, _, callback)=>
-				options.project_id.should.equal project_id
-				options.docLines.should.equal docLines
-				options.path.should.equal fileSystemPath
-				options.project_name.should.equal @project.name
-				options.rev.should.equal (rev+1)
-				callback()
-				@projectUpdater.markAsUpdated.calledWith(project_id).should.equal true
-			@ProjectEntityHandler.updateDocLines project_id, docId, docLines, done
 
 	describe "getting folders, docs and files", ->
 		beforeEach ->