From 63deb0a508210849442f957e5b9545d6dc1dd325 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 11:46:17 +0000 Subject: [PATCH] Revert "added lock to update merger" This reverts commit 8cdac1d74fd63d6ef83ce1e60ba41b9195ed0cac. --- .../ThirdPartyDataStore/UpdateMerger.coffee | 45 +++++++---------- .../UpdateMergerTests.coffee | 50 ++++++------------- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index d05443b6e9..887168fc9e 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -6,35 +6,26 @@ Settings = require('settings-sharelatex') FileTypeManager = require('../Uploads/FileTypeManager') uuid = require('node-uuid') fs = require('fs') -LockManager = require("../../infrastructure/LockManager") - module.exports = mergeUpdate: (project_id, path, updateRequest, source, callback = (error) ->)-> self = @ logger.log project_id:project_id, path:path, "merging update from tpds" - LockManager.getLock project_id, (err)-> - if err? - logger.err project_id:project_id, "could not get lock for merge update" - return callback() - 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)-> + 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)-> + return callback(err) if err? + FileTypeManager.isBinary path, fsPath, (err, isFile)-> return callback(err) if err? - FileTypeManager.isBinary path, fsPath, (err, isFile)-> - return callback(err) if err? - finish = (err)-> - LockManager.releaseLock project_id, -> - callback(err) - if isFile - self.p.processFile project_id, elementId, fsPath, path, source, finish #TODO clean up the stream written to disk here - else - self.p.processDoc project_id, elementId, fsPath, path, source, finish + if isFile + self.p.processFile project_id, elementId, fsPath, path, source, callback #TODO clean up the stream written to disk here + else + self.p.processDoc project_id, elementId, fsPath, path, source, callback deleteUpdate: (project_id, path, source, callback)-> projectLocator.findElementByPath project_id, path, (err, element)-> @@ -47,7 +38,7 @@ module.exports = else if element.folders? type = 'folder' logger.log project_id:project_id, updateType:path, updateType:type, element:element, "processing update to delete entity from tpds" - editorController.deleteEntityWithoutLock project_id, element._id, type, source, (err)-> + editorController.deleteEntity project_id, element._id, type, source, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() @@ -64,7 +55,7 @@ module.exports = callback() else setupNewEntity project_id, path, (err, folder, fileName)-> - editorController.addDocWithoutLock project_id, folder._id, fileName, docLines, source, (err)-> + editorController.addDoc project_id, folder._id, fileName, docLines, source, (err)-> callback() processFile: (project_id, file_id, fsPath, path, source, callback)-> @@ -76,7 +67,7 @@ module.exports = if file_id? editorController.replaceFile project_id, file_id, fsPath, source, finish else - editorController.addFileWithoutLock project_id, folder._id, fileName, fsPath, source, finish + editorController.addFile project_id, folder._id, fileName, fsPath, source, finish writeStreamToDisk: (project_id, file_id, stream, callback = (err, fsPath)->)-> if !file_id? @@ -112,5 +103,5 @@ setupNewEntity = (project_id, path, callback)-> lastIndexOfSlash = path.lastIndexOf("/") fileName = path[lastIndexOfSlash+1 .. -1] folderPath = path[0 .. lastIndexOfSlash] - editorController.mkdirpWithoutLock project_id, folderPath, (err, newFolders, lastFolder)-> + editorController.mkdirp project_id, folderPath, (err, newFolders, lastFolder)-> callback err, lastFolder, fileName diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index 4785a0517e..61ff8da023 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -13,16 +13,12 @@ describe 'UpdateMerger :', -> @projectEntityHandler = {} @fs = {} @FileTypeManager = {} - @LockManager = - getLock:sinon.stub() - releaseLock:sinon.stub() @updateMerger = SandboxedModule.require modulePath, requires: '../Editor/EditorController': @editorController '../Project/ProjectLocator': @projectLocator '../Project/ProjectEntityHandler': @projectEntityHandler 'fs': @fs '../Uploads/FileTypeManager':@FileTypeManager - "../../infrastructure/LockManager": @LockManager 'logger-sharelatex': log: -> err: -> @@ -36,17 +32,13 @@ describe 'UpdateMerger :', -> @path = "/doc1" @fsPath = "file/system/path.tex" @updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(3, null, @fsPath) - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @FileTypeManager.isBinary = sinon.stub() it 'should get the element id', (done)-> - @projectLocator.findElementByPath = sinon.stub().callsArgWith(2) - @FileTypeManager.isBinary.callsArgWith(2, null, false) - @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) + @projectLocator.findElementByPath = sinon.spy() @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true - done() + @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" @@ -72,16 +64,6 @@ describe 'UpdateMerger :', -> @FileTypeManager.isBinary.calledWith(filePath, @fsPath).should.equal true done() - it "should get the lock", (done)-> - - @FileTypeManager.isBinary.callsArgWith(2, null, false) - @projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:"doc_id"}) - @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) - - @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @LockManager.getLock.calledWith(@project_id).should.equal true - done() - describe 'processDoc :', (done)-> beforeEach -> @@ -105,9 +87,9 @@ describe 'UpdateMerger :', -> folder = {_id:"adslkjioj"} docName = "main.tex" path = "folder1/folder2/#{docName}" - @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder) - @editorController.addDocWithoutLock = -> - mock = sinon.mock(@editorController).expects("addDocWithoutLock").withArgs(@project_id, folder._id, docName, @splitDocLines, @source).callsArg(5) + @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).callsArg(5) @update.write(@docLines) @update.end() @@ -124,22 +106,22 @@ describe 'UpdateMerger :', -> @folder = _id: @folder_id @fileName = "file.png" @fsPath = "fs/path.tex" - @editorController.addFileWithoutLock = sinon.stub().callsArg(5) + @editorController.addFile = sinon.stub().callsArg(5) @editorController.replaceFile = sinon.stub().callsArg(4) - @editorController.deleteEntityWithoutLock = sinon.stub() - @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder) + @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, => - @editorController.addFileWithoutLock.called.should.equal false + @editorController.addFile.called.should.equal false @editorController.replaceFile.calledWith(@project_id, @file_id, @fsPath, @source).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, => - @editorController.mkdirpWithoutLock.calledWith(@project_id, "folder/").should.equal true - @editorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true + @editorController.mkdirp.calledWith(@project_id, "folder/").should.equal true + @editorController.addFile.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true @editorController.replaceFile.called.should.equal false done() @@ -147,7 +129,7 @@ describe 'UpdateMerger :', -> beforeEach -> @path = "folder/doc1" - @editorController.deleteEntityWithoutLock = -> + @editorController.deleteEntity = -> @entity_id = "entity_id_here" @entity = _id:@entity_id @projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @path) @@ -159,20 +141,20 @@ describe 'UpdateMerger :', -> it 'should delete the entity in the editor controller with type doc when entity has docLines array', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type folder when entity has folders array', (done)-> @entity.folders = [] - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type file when entity has no interesting properties', (done)-> - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done()