From f1ddff4061eb16b3da05cd6e5d65f89b89129790 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 27 Oct 2014 14:39:20 +0000 Subject: [PATCH] Only ignore hidden files from Dropbox, not GitHub --- .../ThirdPartyDataStore/TpdsController.coffee | 7 +++++++ .../TpdsUpdateHandler.coffee | 8 ++++++-- .../ThirdPartyDataStore/UpdateMerger.coffee | 17 +++++++++-------- .../TpdsUpdateHandlerTests.coffee | 12 +++++++++++- .../UpdateMergerTests.coffee | 17 ----------------- 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsController.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsController.coffee index 8197f2a110..ef22e1764c 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsController.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsController.coffee @@ -5,6 +5,9 @@ Path = require('path') metrics = require("../../infrastructure/Metrics") module.exports = + # mergeUpdate and deleteUpdate are used by Dropbox, where the project is only passed as the name, as the + # first part of the file path. They have to check the project exists, find it, and create it if not. + # They also ignore 'noisy' files like .DS_Store, .gitignore, etc. mergeUpdate: (req, res)-> metrics.inc("tpds.merge-update") {filePath, user_id, projectName} = parseParams(req) @@ -35,6 +38,10 @@ module.exports = res.send 200 req.session.destroy() + # updateProjectContents and deleteProjectContents are used by GitHub. The project_id is known so we + # can skip right ahead to creating/updating/deleting the file. These methods will not ignore noisy + # files like .DS_Store, .gitignore, etc because people are generally more explicit with the files they + # want in git. updateProjectContents: (req, res, next = (error) ->) -> {project_id} = req.params path = "/" + req.params[0] # UpdateMerger expects leading slash diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee index efd34be640..7cd0e53284 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee @@ -4,6 +4,7 @@ projectLocator = require('../Project/ProjectLocator') projectCreationHandler = require('../Project/ProjectCreationHandler') projectDeleter = require('../Project/ProjectDeleter') ProjectRootDocManager = require "../Project/ProjectRootDocManager" +FileTypeManager = require('../Uploads/FileTypeManager') commitMessage = "Before update from Dropbox" @@ -22,8 +23,11 @@ module.exports = else cb err, project getOrCreateProject (err, project)-> - updateMerger.mergeUpdate project._id, path, updateRequest, source, (err)-> - callback(err) + return callback(err) if err? + FileTypeManager.shouldIgnore path, (err, shouldIgnore)-> + if shouldIgnore + return callback() + updateMerger.mergeUpdate project._id, path, updateRequest, source, callback deleteUpdate: (user_id, projectName, path, source, callback)-> diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index 337c315431..887168fc9e 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -12,19 +12,20 @@ module.exports = 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)-> - FileTypeManager.shouldIgnore path, (err, shouldIgnore)-> - if shouldIgnore - return callback() - FileTypeManager.isBinary path, fsPath, (err, isFile)-> - 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 + 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, 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)-> diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee index 4e2f72779b..914bdbc993 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee @@ -17,6 +17,8 @@ describe 'TpdsUpdateHandler', -> createBlankProject : sinon.stub().callsArgWith(2, null, @project) @projectDeleter = {markAsDeletedByExternalSource:sinon.stub().callsArgWith(1)} @rootDocManager = setRootDocAutomatically:sinon.stub() + @FileTypeManager = + shouldIgnore: sinon.stub().callsArgWith(1, null, false) @handler = SandboxedModule.require modulePath, requires: './UpdateMerger': @updateMerger './Editor/EditorController': @editorController @@ -24,6 +26,7 @@ describe 'TpdsUpdateHandler', -> '../Project/ProjectCreationHandler':@projectCreationHandler '../Project/ProjectDeleter': @projectDeleter "../Project/ProjectRootDocManager" : @rootDocManager + '../Uploads/FileTypeManager': @FileTypeManager 'logger-sharelatex': log:-> @user_id = "dsad29jlkjas" @source = "dropbox" @@ -55,7 +58,14 @@ describe 'TpdsUpdateHandler', -> done() ), 1 - + it 'should not update files that should be ignored', (done) -> + @FileTypeManager.shouldIgnore = sinon.stub().callsArgWith(1, null, true) + @projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2) + path = "/.gitignore" + @updateMerger.mergeUpdate = sinon.stub() + @handler.newUpdate @user_id, @project.name, path, {}, @source, => + @updateMerger.mergeUpdate.called.should.equal false + done() describe 'getting a delete :', -> it 'should call deleteEntity in the collaberation manager', (done)-> diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index 08f330e028..61ff8da023 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -33,33 +33,17 @@ describe 'UpdateMerger :', -> @fsPath = "file/system/path.tex" @updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(3, null, @fsPath) @FileTypeManager.isBinary = sinon.stub() - @FileTypeManager.shouldIgnore = sinon.stub() it 'should get the element id', (done)-> - @projectLocator.findElementByPath = sinon.spy() - @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true done() - - it 'should ignore update if FileTypeManger says ignore', (done)-> - filePath = ".gitignore" - @projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:"id"}) - @FileTypeManager.shouldIgnore.callsArgWith(1, null, true) - @updateMerger.mergeUpdate @project_id, filePath, @update, @source, => - @FileTypeManager.isBinary.called.should.equal false - @FileTypeManager.shouldIgnore.calledWith(filePath).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}) - @FileTypeManager.shouldIgnore.callsArgWith(1, null, false) @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) filePath = "/folder/doc.tex" @@ -72,7 +56,6 @@ describe 'UpdateMerger :', -> file_id = "1231" @projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:file_id}) @FileTypeManager.isBinary.callsArgWith(2, null, true) - @FileTypeManager.shouldIgnore.callsArgWith(1, null, false) @updateMerger.p.processFile = sinon.stub().callsArgWith(5) filePath = "/folder/file1.png"