Merge pull request #299 from sharelatex/hof-tpds-locking

Project Locking: TPDS locking

services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee

@@ -6,41 +6,46 @@ Settings = require('settings-sharelatex')
 FileTypeManager = require('../Uploads/FileTypeManager')
 uuid = require('uuid')
 fs = require('fs')
+LockManager = require("../../infrastructure/LockManager")
 
-module.exports =
+module.exports = UpdateMerger =
 	mergeUpdate: (user_id, project_id, path, updateRequest, source, callback = (error) ->)->
-		self = @
 		logger.log project_id:project_id, path:path, "merging update from tpds"
-		projectLocator.findElementByPath project_id, path, (err, element)=>
-			# Returns an error if the element is not found
-			#return callback(err) if err?
-			logger.log project_id:project_id, path:path, "found element by path for merging update from tpds"
-			elementId = undefined
-			if element?
-				elementId = element._id
-			self.p.writeStreamToDisk project_id, elementId, updateRequest, (err, fsPath)->
-				callback = _.wrap callback, (cb, arg) ->
-					fs.unlink fsPath, (err) ->
-						if err?
+		UpdateMerger.p.writeStreamToDisk project_id, updateRequest, (err, fsPath)->
+			return callback(err) if err?
+			LockManager.runWithLock project_id,
+				(cb) => UpdateMerger.mergeUpdateWithoutLock user_id, project_id, path, fsPath, source, cb
+				(mergeErr) ->
+					fs.unlink fsPath, (deleteErr) ->
+						if deleteErr?
 							logger.err project_id:project_id, fsPath:fsPath, "error deleting file"
-						cb(arg)
+						callback mergeErr
+
+	mergeUpdateWithoutLock: (user_id, project_id, path, fsPath, source, callback = (error) ->)->
+		projectLocator.findElementByPath project_id, path, (err, element)=>
+			logger.log {project_id, path, fsPath}, "found element by path for merging update from tpds"
+			elementId = element?._id
+			FileTypeManager.isBinary path, fsPath, (err, isFile)->
 				return callback(err) if err?
-				FileTypeManager.isBinary path, fsPath, (err, isFile)->
-					return callback(err) if err?
-					if isFile
-						self.p.processFile project_id, elementId, fsPath, path, source, user_id, callback
-					else
-						self.p.processDoc project_id, elementId, user_id, fsPath, path, source, callback
+				if isFile
+					UpdateMerger.p.processFile project_id, elementId, fsPath, path, source, user_id, callback
+				else
+					UpdateMerger.p.processDoc project_id, elementId, user_id, fsPath, path, source, callback
 
 	deleteUpdate: (user_id, project_id, path, source, callback)->
+		LockManager.runWithLock project_id,
+			(cb) => UpdateMerger.deleteUpdateWithoutLock(user_id, project_id, path, source, cb)
+			(err, doc)  ->
+				logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds"
+				callback()
+
+	deleteUpdateWithoutLock: (user_id, project_id, path, source, callback)->
 		projectLocator.findElementByPath project_id, path, (err, element, type)->
 			if  err? || !element?
 				logger.log element:element, project_id:project_id, path:path, "could not find entity for deleting, assuming it was already deleted"
 				return callback()
 			logger.log project_id:project_id, path:path, type:type, element:element, "processing update to delete entity from tpds"
-			editorController.deleteEntity project_id, element._id, type, source, user_id, (err)->
-				logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds"
-				callback()
+			editorController.deleteEntityWithoutLock project_id, element._id, type, source, user_id, callback
 
 	p:
 
@@ -57,7 +62,7 @@ module.exports =
 						if err?
 							logger.err err:err, project_id:project_id, doc_id:doc_id, path:path, "error processing file"
 							return callback(err)
-						editorController.addDoc project_id, folder._id, fileName, docLines, source, user_id, callback
+						editorController.addDocWithoutLock project_id, folder._id, fileName, docLines, source, user_id, callback
 
 		processFile: (project_id, file_id, fsPath, path, source, user_id, callback)->
 			finish = (err)->
@@ -71,29 +76,27 @@ module.exports =
 				else if file_id?
 					editorController.replaceFileWithoutLock project_id, file_id, fsPath, source, user_id, finish
 				else
-					editorController.addFile project_id, folder?._id, fileName, fsPath, source, user_id, finish
+					editorController.addFileWithoutLock project_id, folder?._id, fileName, fsPath, source, user_id, finish
 
-		writeStreamToDisk: (project_id, file_id, stream, callback = (err, fsPath)->)->
-			if !file_id?
-				file_id = uuid.v4()
-			dumpPath = "#{Settings.path.dumpFolder}/#{project_id}_#{file_id}"
+		writeStreamToDisk: (project_id, stream, callback = (err, fsPath)->)->
+			dumpPath = "#{Settings.path.dumpFolder}/#{project_id}_#{uuid.v4()}"
 
 			writeStream = fs.createWriteStream(dumpPath)
 			stream.pipe(writeStream)
 
 			stream.on 'error', (err)->
-				logger.err err:err, project_id:project_id, file_id:file_id, dumpPath:dumpPath,
+				logger.err {err, project_id, dumpPath},
 					"something went wrong with incoming tpds update stream"
 			writeStream.on 'error', (err)->
-				logger.err err:err, project_id:project_id, file_id:file_id, dumpPath:dumpPath,
+				logger.err {err, project_id, dumpPath},
 					"something went wrong with writing tpds update to disk"
 
 			stream.on 'end', ->
-				logger.log project_id:project_id, file_id:file_id, dumpPath:dumpPath, "incoming tpds update stream ended"
+				logger.log {project_id, dumpPath}, "incoming tpds update stream ended"
 			writeStream.on "finish", ->
-				logger.log project_id:project_id, file_id:file_id, dumpPath:dumpPath, "tpds update write stream finished"
+				logger.log {project_id, dumpPath}, "tpds update write stream finished"
 				callback null, dumpPath
-				
+
 readFileIntoTextArray = (path, callback)->
 	fs.readFile path, "utf8", (error, content = "") ->
 		if error?
@@ -107,5 +110,5 @@ setupNewEntity = (project_id, path, callback)->
 	lastIndexOfSlash = path.lastIndexOf("/")
 	fileName = path[lastIndexOfSlash+1 .. -1]
 	folderPath = path[0 .. lastIndexOfSlash]
-	editorController.mkdirp project_id, folderPath, (err, newFolders, lastFolder)->
+	editorController.mkdirpWithoutLock project_id, folderPath, (err, newFolders, lastFolder)->
 		callback err, lastFolder, fileName

services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee

@@ -11,9 +11,11 @@ describe 'UpdateMerger :', ->
 		@updateReciver = {}
 		@projectLocator = {}
 		@projectEntityHandler = {}
-		@fs = 
+		@fs =
 			unlink:sinon.stub().callsArgWith(1)
 		@FileTypeManager = {}
+		@LockManager =
+			runWithLock : sinon.spy((key, runner, callback) -> runner(callback))
 		@updateMerger = SandboxedModule.require modulePath, requires:
 			'../Editor/EditorController': @editorController
 			'../Project/ProjectLocator': @projectLocator
@@ -24,135 +26,149 @@ describe 'UpdateMerger :', ->
 			'logger-sharelatex':
 				log: ->
 				err: ->
-			"metrics-sharelatex": 
+			"metrics-sharelatex":
 				Timer:->
 					done:->
+			"../../infrastructure/LockManager":@LockManager
 		@project_id = "project_id_here"
 		@user_id = "mock-user-id"
 		@source = "dropbox"
 		@update = new BufferedStream()
 		@update.headers = {}
 
-	describe 'mergeUpdate :', ->
+	describe 'mergeUpdate', ->
 		beforeEach ->
 			@path = "/doc1"
 			@fsPath = "file/system/path.tex"
-			@updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(3, null, @fsPath)
+			@updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(2, null, @fsPath)
 			@FileTypeManager.isBinary = sinon.stub()
 
-		it 'should get the element id', (done)->
-			@projectLocator.findElementByPath = sinon.spy()
-			@updateMerger.mergeUpdate @user_id, @project_id, @path, @update, @source, =>
-			@projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true
-			done()
-
-		it 'should process update as doc when it is a doc', (done)->
-			doc_id = "231312s"
-			@FileTypeManager.isBinary.callsArgWith(2, null, false)
-			@projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:doc_id})
-			@updateMerger.p.processDoc = sinon.stub().callsArgWith(6)
-			filePath = "/folder/doc.tex"
-
-			@updateMerger.mergeUpdate @user_id, @project_id, filePath, @update, @source, =>
-				@FileTypeManager.isBinary.calledWith(filePath, @fsPath).should.equal true
-				@updateMerger.p.processDoc.calledWith(@project_id, doc_id, @user_id, @fsPath, filePath, @source).should.equal true
-				@fs.unlink.calledWith(@fsPath).should.equal true
-				done()
-
-		it 'should process update as file when it is not a doc', (done)->
-			file_id = "1231"
-			@projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:file_id})
-			@FileTypeManager.isBinary.callsArgWith(2, null, true)
-			@updateMerger.p.processFile = sinon.stub().callsArgWith(6)
-			filePath = "/folder/file1.png"
-
-			@updateMerger.mergeUpdate @user_id, @project_id, filePath, @update, @source, =>
-				@updateMerger.p.processFile.calledWith(@project_id, file_id, @fsPath, filePath, @source, @user_id).should.equal true
-				@FileTypeManager.isBinary.calledWith(filePath, @fsPath).should.equal true
-				@fs.unlink.calledWith(@fsPath).should.equal true
-				done()
-
-
-	describe 'processDoc :', (done)->
-		beforeEach ->
-			@doc_id = "312312klnkld"
-			@docLines = "\\documentclass{article}\n\\usepackage[utf8]{inputenc}\n\n\\title{42}\n\\author{Jane Doe}\n\\date{June 2011}"
-			@splitDocLines = @docLines.split("\n")
-			@fs.readFile = sinon.stub().callsArgWith(2, null, @docLines)
-
-		it 'should set the doc text in the editor controller', (done)->
-			@editorController.setDoc = ->
-			mock = sinon.mock(@editorController).expects("setDoc").withArgs(@project_id, @doc_id, @user_id, @splitDocLines, @source).callsArg(5)
-
-			@update.write(@docLines)
-			@update.end()
-
-			@updateMerger.p.processDoc @project_id, @doc_id, @user_id, @update, "path", @source, ->
-				mock.verify()
-				done()
-
-		it 'should create a new doc when it doesnt exist', (done)->
-			folder = {_id:"adslkjioj"}
-			docName = "main.tex"
-			path = "folder1/folder2/#{docName}"
-			@editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder)
-			@editorController.addDoc = ->
-			mock = sinon.mock(@editorController).expects("addDoc").withArgs(@project_id, folder._id, docName, @splitDocLines, @source, @user_id).callsArg(6)
-
-			@update.write(@docLines)
-			@update.end()
-
-			@updateMerger.p.processDoc @project_id, undefined, @user_id, @update, path, @source, ->
-				mock.verify()
-				done()
-
-	describe 'processFile :', (done)->
-		beforeEach ->
-			@file_id = "file_id_here"
-			@folder_id = "folder_id_here"
-			@path = "folder/file.png"
-			@folder = _id: @folder_id
-			@fileName = "file.png"
-			@fsPath = "fs/path.tex"
-			@editorController.addFile = sinon.stub().callsArg(6)
-			@editorController.replaceFileWithoutLock = sinon.stub().callsArg(5)
-			@editorController.deleteEntity = sinon.stub()
-			@editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder)
-			@updateMerger.p.writeStreamToDisk = sinon.stub().withArgs(@project_id, @file_id, @update).callsArgWith(3, null, @fsPath)
-
-		it 'should replace file if the file already exists', (done)->
-			@updateMerger.p.processFile @project_id, @file_id, @fsPath, @path, @source, @user_id, =>
-				@editorController.addFile.called.should.equal false
-				@editorController.replaceFileWithoutLock.calledWith(@project_id, @file_id, @fsPath, @source, @user_id).should.equal true
-				done()
-
-		it 'should call add file if the file does not exist', (done)->
-			@updateMerger.p.processFile @project_id, undefined, @fsPath, @path, @source, @user_id, =>
-				@editorController.mkdirp.calledWith(@project_id, "folder/").should.equal true
-				@editorController.addFile.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source, @user_id).should.equal true
-				@editorController.replaceFileWithoutLock.called.should.equal false
-				done()
-
-	describe 'delete entity :', (done)->
-
+		describe "doc updates", () ->
+			beforeEach ->
+				@doc_id = "231312s"
+				@FileTypeManager.isBinary.callsArgWith(2, null, false)
+				@projectLocator.findElementByPath = sinon.stub().callsArgWith(2, null, _id: @doc_id)
+				@updateMerger.p.processDoc = sinon.stub().callsArgWith(6)
+				@filePath = "/folder/doc.tex"
+
+			it 'should get the element id', (done)->
+				@updateMerger.mergeUpdate @user_id, @project_id, @path, @update, @source, =>
+					@projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true
+					done()
+
+			it 'should take a project lock', (done)->
+				@updateMerger.mergeUpdate @user_id, @project_id, @path, @update, @source, =>
+					@LockManager.runWithLock.calledWith(@project_id).should.equal true
+					done()
+
+			it 'should process update as doc', (done)->
+				@updateMerger.mergeUpdate @user_id, @project_id, @filePath, @update, @source, =>
+					@FileTypeManager.isBinary.calledWith(@filePath, @fsPath).should.equal true
+					@updateMerger.p.processDoc.calledWith(@project_id, @doc_id, @user_id, @fsPath, @filePath, @source).should.equal true
+					@fs.unlink.calledWith(@fsPath).should.equal true
+					done()
+
+		describe "file updates", () ->
+			beforeEach ->
+				@file_id = "1231"
+				@projectLocator.findElementByPath = sinon.stub().callsArgWith(2, null, _id: @file_id)
+				@FileTypeManager.isBinary.callsArgWith(2, null, true)
+				@updateMerger.p.processFile = sinon.stub().callsArgWith(6)
+				@filePath = "/folder/file1.png"
+
+			it 'should process update as file when it is not a doc', (done)->
+				@updateMerger.mergeUpdate @user_id, @project_id, @filePath, @update, @source, =>
+					@updateMerger.p.processFile.calledWith(@project_id, @file_id, @fsPath, @filePath, @source, @user_id).should.equal true
+					@FileTypeManager.isBinary.calledWith(@filePath, @fsPath).should.equal true
+					@fs.unlink.calledWith(@fsPath).should.equal true
+					done()
+
+	describe 'deleteUpdate', (done)->
 		beforeEach ->
 			@path = "folder/doc1"
 			@type = "mock-type"
-			@editorController.deleteEntity = ->
+			@editorController.deleteEntityWithoutLock = ->
 			@entity_id = "entity_id_here"
 			@entity = _id:@entity_id
-			@projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @type)
+			@projectLocator.findElementByPath = sinon.stub().callsArgWith(2, null, @entity, @type)
+			@editorController.deleteEntityWithoutLock = sinon.stub().callsArg(5)
+
+		it 'should get the element id', (done)->
+			@updateMerger.deleteUpdate @user_id, @project_id, @path, @source, =>
+				@projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true
+				done()
 
-		it 'should get the element id', ->
-			@projectLocator.findElementByPath = sinon.spy()
-			@updateMerger.deleteUpdate @user_id, @project_id, @path, @source, ->
-			@projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true
+		it 'should take a project lock', (done)->
+			@updateMerger.deleteUpdate @user_id, @project_id, @path, @source, =>
+				@LockManager.runWithLock.calledWith(@project_id).should.equal true
+				done()
 
 		it 'should delete the entity in the editor controller with the correct type', (done)->
 			@entity.lines = []
-			mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source, @user_id).callsArg(5)
-			@updateMerger.deleteUpdate @user_id, @project_id, @path, @source, ->
-				mock.verify()
+			@updateMerger.deleteUpdate @user_id, @project_id, @path, @source, =>
+				@editorController.deleteEntityWithoutLock
+					.calledWith(@project_id, @entity_id, @type, @source, @user_id)
+					.should.equal true
 				done()
 
-	
+	describe 'private methods', () ->
+		describe 'processDoc', (done)->
+			beforeEach ->
+				@doc_id = "312312klnkld"
+				@docLines = "\\documentclass{article}\n\\usepackage[utf8]{inputenc}\n\n\\title{42}\n\\author{Jane Doe}\n\\date{June 2011}"
+				@splitDocLines = @docLines.split("\n")
+				@fs.readFile = sinon.stub().callsArgWith(2, null, @docLines)
+
+				@editorController.setDoc = sinon.stub().callsArg(5)
+
+				@update.write(@docLines)
+				@update.end()
+
+			it 'should set the doc text in the editor controller', (done)->
+				@updateMerger.p.processDoc @project_id, @doc_id, @user_id, @update, "path", @source, =>
+					@editorController.setDoc
+						.calledWith(@project_id, @doc_id, @user_id, @splitDocLines, @source)
+						.should.equal true
+					done()
+
+			it 'should create a new doc when it doesnt exist', (done)->
+				folder = {_id:"adslkjioj"}
+				docName = "main.tex"
+				path = "folder1/folder2/#{docName}"
+				@editorController.mkdirpWithoutLock = sinon.stub().callsArgWith(2, null, [folder], folder)
+				@editorController.addDocWithoutLock = sinon.stub().callsArg(6)
+
+				@updateMerger.p.processDoc @project_id, undefined, @user_id, @update, path, @source, =>
+					@editorController.mkdirpWithoutLock
+						.calledWith(@project_id)
+						.should.equal true
+					@editorController.addDocWithoutLock
+						.calledWith(@project_id, folder._id, docName, @splitDocLines, @source, @user_id)
+						.should.equal true
+					done()
+
+		describe 'processFile', (done)->
+			beforeEach ->
+				@file_id = "file_id_here"
+				@folder_id = "folder_id_here"
+				@path = "folder/file.png"
+				@folder = _id: @folder_id
+				@fileName = "file.png"
+				@fsPath = "fs/path.tex"
+				@editorController.addFileWithoutLock = sinon.stub().callsArg(6)
+				@editorController.replaceFileWithoutLock = sinon.stub().callsArg(5)
+				@editorController.deleteEntityWithoutLock = sinon.stub()
+				@editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder)
+
+			it 'should replace file if the file already exists', (done)->
+				@updateMerger.p.processFile @project_id, @file_id, @fsPath, @path, @source, @user_id, =>
+					@editorController.addFileWithoutLock.called.should.equal false
+					@editorController.replaceFileWithoutLock.calledWith(@project_id, @file_id, @fsPath, @source, @user_id).should.equal true
+					done()
+
+			it 'should call add file if the file does not exist', (done)->
+				@updateMerger.p.processFile @project_id, undefined, @fsPath, @path, @source, @user_id, =>
+					@editorController.mkdirpWithoutLock.calledWith(@project_id, "folder/").should.equal true
+					@editorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source, @user_id).should.equal true
+					@editorController.replaceFileWithoutLock.called.should.equal false
+					done()