Kaynağa Gözat

Merge branch 'docstore_integration'

James Allen 12 yıl önce
ebeveyn
işleme
4a27342b01

+ 35 - 2
services/web/app/coffee/Features/Docstore/DocstoreManager.coffee

@@ -27,5 +27,38 @@ module.exports = DocstoreManager =
 				callback(null, docs)
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
-				logger.error err: error, project_id: project_id, "error getting all docs in docstore"
-				callback(error)
+				logger.error err: error, project_id: project_id, "error getting all docs from docstore"
+				callback(error)
+
+	getDoc: (project_id, doc_id, callback = (error, lines, version, rev) ->) ->
+		logger.log project_id: project_id, doc_id: doc_id, "getting doc in docstore api"
+		url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}"
+		request.get {
+			url: url
+			json: true
+		}, (error, res, doc) ->
+			return callback(error) if error?
+			if 200 <= res.statusCode < 300
+				logger.log doc_id: doc_id, project_id: project_id, version: doc.version, rev: doc.rev, "got doc from docstore api"
+				callback(null, doc.lines, doc.version, doc.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 getting doc from docstore"
+				callback(error)
+
+	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 {
+			url: url
+			json:
+				lines: lines
+				version: version
+		}, (error, res, result) ->
+			return callback(error) if error?
+			if 200 <= res.statusCode < 300
+				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"
+				callback(error)

+ 16 - 0
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)

+ 6 - 7
services/web/app/coffee/Features/Documents/DocumentController.coffee

@@ -1,31 +1,30 @@
-ProjectLocator = require "../Project/ProjectLocator"
 ProjectEntityHandler = require "../Project/ProjectEntityHandler"
 Errors = require "../../errors"
 logger = require("logger-sharelatex")
 
-module.exports = 
-
+module.exports =
 	getDocument: (req, res, next = (error) ->) ->
 		project_id = req.params.Project_id
 		doc_id = req.params.doc_id
 		logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)"
-		ProjectLocator.findElement project_id: project_id, element_id: doc_id, type: "doc", (error, doc) ->
+		ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, version, rev) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)
 			res.type "json"
 			res.send JSON.stringify {
-				lines: doc.lines
+				lines: lines
+				version: version
 			}
 			req.session.destroy()
 
-
 	setDocument: (req, res, next = (error) ->) ->
 		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)

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

+ 37 - 38
services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -111,16 +111,29 @@ module.exports = ProjectEntityHandler =
 		logger.log sl_req_id: sl_req_id, project_id: project_id, "removing root doc"
 		Project.update {_id:project_id}, {$unset: {rootDoc_id: true}}, {}, callback
 
+	getDoc: (project_id, doc_id, callback = (error, lines, version, rev) ->) ->
+		DocstoreManager.getDoc project_id, doc_id, callback
+
 	addDoc: (project_or_id, folder_id, docName, docLines, sl_req_id, callback = (error, doc, folder_id) ->)=>
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)
 		Project.getProject project_or_id, "", (err, project) ->
 			logger.log sl_req_id: sl_req_id, project: project._id, folder_id: folder_id, doc_name: docName, "adding doc"
 			return callback(err) if err?
 			confirmFolder project, folder_id, (folder_id)=>
-				doc = new Doc name: docName, lines: docLines
+				doc = new Doc name: docName
 				Project.putElement project._id, folder_id, doc, "doc", (err, result)=>
-					tpdsUpdateSender.addDoc {project_id:project._id, docLines:docLines, path:result.path.fileSystem, project_name:project.name, rev:doc.rev}, sl_req_id, ->
-						callback(err, doc, folder_id)
+					return callback(err) if err?
+					DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, modified, rev) ->
+						return callback(err) if err? 
+						tpdsUpdateSender.addDoc {
+							project_id:   project._id,
+							docLines:     docLines,
+							path:         result.path.fileSystem,
+							project_name: project.name,
+							rev:          0
+						}, sl_req_id, (err) ->
+							return callback(err) if err?
+							callback(null, doc, folder_id)
 
 	addFile: (project_or_id, folder_id, fileName, path, sl_req_id, callback = (error, fileRef, folder_id) ->)->
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)
@@ -221,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)

+ 82 - 1
services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee

@@ -52,6 +52,87 @@ describe "DocstoreManager", ->
 					}, "error deleting doc in docstore")
 					.should.equal true
 
+	describe "updateDoc", ->
+		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, { modified: @modified, rev: @rev })
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
+
+			it "should update the doc in the docstore api", ->
+				@request.post
+					.calledWith({
+						url: "#{@settings.apis.docstore.url}/project/#{@project_id}/doc/#{@doc_id}"
+						json:
+							lines: @lines
+							version: @version
+					})
+					.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 ->
+				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true
+
+			it "should log the error", ->
+				@logger.error
+					.calledWith({
+						err: new Error("docstore api responded with a non-success code: 500")
+						project_id: @project_id
+						doc_id: @doc_id
+					}, "error updating doc in docstore")
+					.should.equal true
+
+	describe "getDoc", ->
+		beforeEach ->
+			@doc =
+				lines:   @lines = ["mock", "doc", "lines"]
+				version: @version = 42
+				rev:     @rev = 5
+
+		describe "with a successful response code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, statusCode: 204, @doc)
+				@DocstoreManager.getDoc @project_id, @doc_id, @callback
+
+			it "should get the doc from the docstore api", ->
+				@request.get
+					.calledWith({
+						url: "#{@settings.apis.docstore.url}/project/#{@project_id}/doc/#{@doc_id}"
+						json: true
+					})
+					.should.equal true
+
+			it "should call the callback with the lines, version and rev", ->
+				@callback.calledWith(null, @lines, @version, @rev).should.equal true
+
+		describe "with a failed response code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
+				@DocstoreManager.getDoc @project_id, @doc_id, @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true
+
+			it "should log the error", ->
+				@logger.error
+					.calledWith({
+						err: new Error("docstore api responded with a non-success code: 500")
+						project_id: @project_id
+						doc_id: @doc_id
+					}, "error getting doc from docstore")
+					.should.equal true
 
 	describe "getAllDocs", ->
 		describe "with a successful response code", ->
@@ -83,5 +164,5 @@ describe "DocstoreManager", ->
 					.calledWith({
 						err: new Error("docstore api responded with a non-success code: 500")
 						project_id: @project_id
-					}, "error getting all docs in docstore")
+					}, "error getting all docs from docstore")
 					.should.equal true

+ 34 - 0
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()

+ 13 - 10
services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee

@@ -11,8 +11,7 @@ Errors = require "../../../../app/js/errors"
 
 describe "DocumentController", ->
 	beforeEach ->
-		@DocumentController = SandboxedModule.require modulePath, requires:
-			"../Project/ProjectLocator": @ProjectLocator = {}
+		@DocumentController = SandboxedModule.require modulePath, requires:  
 			"../Project/ProjectEntityHandler": @ProjectEntityHandler = {}
 		@res = new MockResponse()
 		@req = new MockRequest()
@@ -20,32 +19,34 @@ describe "DocumentController", ->
 		@project_id = "project-id-123"
 		@doc_id = "doc-id-123"
 		@doc_lines = ["one", "two", "three"]
+		@version = 42
+		@rev = 5
 
 	describe "getDocument", ->
 		beforeEach ->
 			@req.params =
 				Project_id: @project_id
 				doc_id: @doc_id
-				
 
 		describe "when the document exists", ->
 			beforeEach ->
-				@ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, lines: @doc_lines)
+				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @version, @rev)
 				@DocumentController.getDocument(@req, @res, @next)
 
 			it "should get the document from Mongo", ->
-				@ProjectLocator.findElement
-					.calledWith(project_id: @project_id, element_id: @doc_id, type: "doc")
+				@ProjectEntityHandler.getDoc
+					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
 			it "should return the document data to the client as JSON", ->
 				@res.type.should.equal "json"
 				@res.body.should.equal JSON.stringify
 					lines: @doc_lines
+					version: @version
 
 		describe "when the document doesn't exist", ->
 			beforeEach ->
-				@ProjectLocator.findElement = sinon.stub().callsArgWith(1, new Errors.NotFoundError("not found"), null)
+				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, new Errors.NotFoundError("not found"), null)
 				@DocumentController.getDocument(@req, @res, @next)
 
 			it "should call next with the NotFoundError", ->
@@ -60,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", ->
@@ -75,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", ->

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

+ 148 - 72
services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee

@@ -1,14 +1,17 @@
 chai = require('chai')
 assert = require('chai').assert
 should = chai.should()
+expect = chai.expect
 sinon = require 'sinon'
 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'
+	doc_id = '4eecb1c1bffa66588e0000a2'
 	folder_id = "4eecaffcbffa66588e000008"
 	rootFolderId = "4eecaffcbffa66588e000007"
 	
@@ -34,7 +37,8 @@ describe 'ProjectEntityHandler', ->
 			rootFolder:[@rootFolder]
 		@DocModel = class Doc
 			constructor:(options)->
-				{@name, @lines} = options		
+				{@name, @lines} = options
+				@_id = "mock-id"	
 				@rev = 0
 		@FileModel =  class File
 			constructor:(options)->
@@ -62,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 = {}
 
@@ -277,42 +281,63 @@ describe 'ProjectEntityHandler', ->
 						done()
 			@ProjectEntityHandler._removeElementFromMongoArray model, id, mongoPath, ->
 
-	describe 'adding doc', ->
-		docName = "some new doc"
-		docLines = ['1234','abc']
+	describe 'getDoc', ->
+		beforeEach ->
+			@lines = ["mock", "doc", "lines"]
+			@version = 42
+			@rev = 5
+			@DocstoreManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @rev)
+			@ProjectEntityHandler.getDoc project_id, doc_id, @callback
+
+		it "should call the docstore", ->
+			@DocstoreManager.getDoc
+				.calledWith(project_id, doc_id)
+				.should.equal true
 
-		it 'should call put element', (done)->
-			@ProjectModel.putElement = (passedProject_id, passedFolder_id, passedDoc, passedType, callback)-> 
-				passedProject_id.should.equal project_id
-				passedFolder_id.should.equal folder_id
-				passedDoc.name.should.equal docName
-				passedDoc.lines[0].should.equal docLines[0]
-				passedDoc.lines[1].should.equal docLines[1]
-				passedType.should.equal 'doc'
-				done()
-			@ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "", (err, doc, parentFolder)->
+		it "should call the callback with the lines, version and rev", ->
+			@callback.calledWith(null, @lines, @version, @rev).should.equal true
 
-		it 'should return doc and parent folder', (done)->
-			@ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "", (err, doc, parentFolder)->
-				parentFolder.should.equal folder_id
-				doc.name.should.equal docName
-				done()
+	describe 'addDoc', ->
+		beforeEach ->
+			@name = "some new doc"
+			@lines = ['1234','abc']
+			@path = "/path/to/doc"
 
-		it 'should call third party data store', (done)->
-			fileSystemPath = "/somehwere/#{docName}"
-			@ProjectModel.putElement = (project_id, folder_id, doc, type, callback)-> callback(null, {path:{fileSystem:fileSystemPath}})
+			@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, true, 0)
 
-			@tpdsUpdateSender.addDoc = (options)=>
-				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 0
-				done()
+			@ProjectEntityHandler.addDoc project_id, folder_id, @name, @lines, @callback
 
-			@ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "",->
+			# Created doc
+			@doc = @ProjectModel.putElement.args[0][2]
+			@doc.name.should.equal @name
+			expect(@doc.lines).to.be.undefined
 
-		
+		it 'should call put element', ->
+			@ProjectModel.putElement
+				.calledWith(project_id, folder_id, @doc)
+				.should.equal true
+
+		it 'should return doc and parent folder', ->
+			@callback.calledWith(null, @doc, folder_id).should.equal true
+
+		it 'should call third party data store', ->
+			@tpdsUpdateSender.addDoc
+				.calledWith({
+					project_id: project_id
+					docLines: @lines
+					path: @path
+					project_name: @project.name
+					rev: 0
+				})
+				.should.equal true
+
+		it "should send the doc lines to the doc store", ->
+			@DocstoreManager.updateDoc
+				.calledWith(project_id, @doc._id.toString(), @lines, 0)
+				.should.equal true
 
 	describe 'adding file', ->
 		fileName = "something.jpg"
@@ -433,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
+
+			it "should call the callback", ->
+				@callback.called.should.equal true
 
-			@ProjectEntityHandler.updateDocLines project_id, docId, docLines, ->
+		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 ->