Merge pull request #1035 from sharelatex/bg-fix-github-sync-filetypes

check the filetype before syncing file

GitOrigin-RevId: ec1a309e2423c6ff2c73202dcabae610edce4f6e

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -50,6 +50,11 @@ module.exports = ProjectEntityHandler = self =
 						files[path.join(folderPath, file.name)] = file
 			callback null, files
 
+	getAllEntities: (project_id, callback) ->
+		ProjectGetter.getProject project_id, (err, project) ->
+			return callback(err) if err?
+			self.getAllEntitiesFromProject project, callback
+
 	getAllEntitiesFromProject: (project, callback) ->
 		logger.log project:project, "getting all entities for project"
 		self._getAllFoldersFromProject project, (err, folders = {}) ->

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

@@ -4,6 +4,7 @@ logger = require('logger-sharelatex')
 EditorController = require('../Editor/EditorController')
 FileTypeManager = require('../Uploads/FileTypeManager')
 FileWriter = require('../../infrastructure/FileWriter')
+ProjectEntityHandler = require('../Project/ProjectEntityHandler')
 
 module.exports = UpdateMerger =
 	mergeUpdate: (user_id, project_id, path, updateRequest, source, callback = (error) ->)->
@@ -16,13 +17,30 @@ module.exports = UpdateMerger =
 						logger.err project_id:project_id, fsPath:fsPath, "error deleting file"
 					callback mergeErr
 
-	_mergeUpdate: (user_id, project_id, path, fsPath, source, callback = (error) ->)->
-		FileTypeManager.isBinary path, fsPath, (err, isFile)->
+	_determineFileType: (project_id, path, fsPath, callback = (err, fileType) ->) ->
+		ProjectEntityHandler.getAllEntities project_id, (err, docs, files) ->
 			return callback(err) if err?
-			if isFile
+			if _.some(files, (f) -> f.path is path)
+				return callback(null, "existing-file")
+			if _.some(docs, (d) -> d.path is path)
+				return callback(null, "existing-doc") 
+			# existing file not found in project, fall back to extension check
+			FileTypeManager.isBinary path, fsPath, (err, isFile)->
+				return callback(err) if err?
+				if isFile
+					callback(null, "new-file") # extension was not text
+				else
+					callback(null, "new-doc")
+
+	_mergeUpdate: (user_id, project_id, path, fsPath, source, callback = (error) ->)->
+		UpdateMerger._determineFileType project_id, path, fsPath, (err, fileType)->
+			return callback(err) if err?
+			if fileType in ["existing-file", "new-file"]
 				UpdateMerger.p.processFile project_id, fsPath, path, source, user_id, callback
-			else
+			else if fileType in ["existing-doc", "new-doc"]
 				UpdateMerger.p.processDoc project_id, user_id, fsPath, path, source, callback
+			else
+				callback new Error("unrecognized file")
 
 	deleteUpdate: (user_id, project_id, path, source, callback = () ->)->
 		EditorController.deleteEntityWithPath project_id, path, source, user_id, () ->

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

@@ -16,28 +16,47 @@ describe 'UpdateMerger :', ->
 			'../Editor/EditorController': @EditorController = {}
 			'../Uploads/FileTypeManager':@FileTypeManager = {}
 			'../../infrastructure/FileWriter': @FileWriter = {}
+			'../Project/ProjectEntityHandler': @ProjectEntityHandler = {}
 			'settings-sharelatex':{path:{dumpPath:"dump_here"}}
 		@project_id = "project_id_here"
 		@user_id = "mock-user-id"
 
-		@docPath = "/folder/doc.tex"
-		@filePath = "/folder/file.png"
+		@docPath = @newDocPath = "/folder/doc.tex"
+		@filePath = @newFilePath = "/folder/file.png"
+
+		@existingDocPath = '/folder/other.tex'
+		@existingFilePath = '/folder/fig1.pdf'
+		
 		@linkedFileData = {provider: 'url'}
 
-		@fsPath = "/tmp/file/path"
+		@existingDocs = [ 
+			{path: '/main.tex'}
+			{path: '/folder/other.tex'}
+		]
+		@existingFiles = [
+			{path: '/figure.pdf'}
+			{path: '/folder/fig1.pdf'}
+		]
+		@ProjectEntityHandler.getAllEntities = sinon.stub().callsArgWith(1, null, @existingDocs, @existingFiles)
 
+		@fsPath = "/tmp/file/path"
 		@source = "dropbox"
 		@updateRequest = new BufferedStream()
 		@FileWriter.writeStreamToDisk = sinon.stub().yields(null, @fsPath)
 		@callback = sinon.stub()
 
 	describe 'mergeUpdate', ->
-		describe "doc updates", ->
+	
+		describe "doc updates for a new doc", ->
+
 			beforeEach ->
 				@FileTypeManager.isBinary = sinon.stub().yields(null, false)
 				@updateMerger.p.processDoc = sinon.stub().yields()
 				@updateMerger.mergeUpdate @user_id, @project_id, @docPath, @updateRequest, @source, @callback
 
+			it 'should look at the file contents', ->
+				@FileTypeManager.isBinary.called.should.equal true
+
 			it 'should process update as doc', ->
 				@updateMerger.p.processDoc
 					.calledWith(@project_id, @user_id, @fsPath, @docPath, @source)
@@ -46,12 +65,15 @@ describe 'UpdateMerger :', ->
 			it 'removes the temp file from disk', ->
 				@fs.unlink.calledWith(@fsPath).should.equal true
 
-		describe "file updates", ->
+		describe "file updates for a new file ", ->
 			beforeEach ->
 				@FileTypeManager.isBinary = sinon.stub().yields(null, true)
 				@updateMerger.p.processFile = sinon.stub().yields()
 				@updateMerger.mergeUpdate @user_id, @project_id, @filePath, @updateRequest, @source, @callback
 
+			it 'should look at the file contents', ->
+				@FileTypeManager.isBinary.called.should.equal true
+
 			it 'should process update as file', ->
 				@updateMerger.p.processFile
 					.calledWith(@project_id, @fsPath, @filePath, @source, @user_id)
@@ -60,6 +82,40 @@ describe 'UpdateMerger :', ->
 			it 'removes the temp file from disk', ->
 				@fs.unlink.calledWith(@fsPath).should.equal true
 
+		describe "doc updates for an existing doc", ->
+			beforeEach ->
+				@FileTypeManager.isBinary = sinon.stub()
+				@updateMerger.p.processDoc = sinon.stub().yields()
+				@updateMerger.mergeUpdate @user_id, @project_id, @existingDocPath, @updateRequest, @source, @callback
+
+			it 'should not look at the file contents', ->
+				@FileTypeManager.isBinary.called.should.equal false
+
+			it 'should process update as doc', ->
+				@updateMerger.p.processDoc
+					.calledWith(@project_id, @user_id, @fsPath, @existingDocPath, @source)
+					.should.equal true
+
+			it 'removes the temp file from disk', ->
+				@fs.unlink.calledWith(@fsPath).should.equal true
+
+		describe "file updates for an existing file", ->
+			beforeEach ->
+				@FileTypeManager.isBinary = sinon.stub()
+				@updateMerger.p.processFile = sinon.stub().yields()
+				@updateMerger.mergeUpdate @user_id, @project_id, @existingFilePath, @updateRequest, @source, @callback
+
+			it 'should not look at the file contents', ->
+				@FileTypeManager.isBinary.called.should.equal false
+
+			it 'should process update as file', ->
+				@updateMerger.p.processFile
+					.calledWith(@project_id, @fsPath, @existingFilePath, @source, @user_id)
+					.should.equal true
+
+			it 'removes the temp file from disk', ->
+				@fs.unlink.calledWith(@fsPath).should.equal true
+
 	describe 'deleteUpdate', ->
 		beforeEach ->
 			@EditorController.deleteEntityWithPath = sinon.stub().yields()