Only ignore hidden files from Dropbox, not GitHub

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

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)->	

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)->

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)->

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"