Merge pull request #1738 from sharelatex/spd-dumpfolder

Ensure dump folder exists before opening templates

GitOrigin-RevId: 3c7269fbcdff45a06e505154a35ce6d2bd4b7ae1

services/web/app/coffee/Features/Templates/TemplatesManager.coffee

@@ -3,6 +3,7 @@ ProjectDetailsHandler = require "../../../js/Features/Project/ProjectDetailsHand
 ProjectOptionsHandler = require "../../../js/Features/Project/ProjectOptionsHandler"
 ProjectRootDocManager = require "../../../js/Features/Project/ProjectRootDocManager"
 ProjectUploadManager = require "../../../js/Features/Uploads/ProjectUploadManager"
+FileWriter = require "../../infrastructure/FileWriter"
 async = require "async"
 fs = require "fs"
 logger = require "logger-sharelatex"
@@ -21,33 +22,36 @@ module.exports = TemplatesManager =
 		zipReq.on "error", (err) ->
 			logger.error { err }, "error getting zip from template API"
 			callback err
-		projectName = ProjectDetailsHandler.fixProjectName templateName
-		dumpPath = "#{settings.path.dumpFolder}/#{uuid.v4()}"
-		writeStream = fs.createWriteStream dumpPath
-		writeStream.on "close", ->
-			if zipReq.response.statusCode != 200
-				logger.err { uri: zipUrl, statusCode: zipReq.response.statusCode }, "non-success code getting zip from template API"
-				return callback new Error("get zip failed")
-			ProjectUploadManager.createProjectFromZipArchiveWithName user_id, projectName, dumpPath, (err, project) ->
-				if err?
-					logger.err { err, zipReq }, "problem building project from zip"
-					return callback err
-				async.series [
-					(cb) -> TemplatesManager._setCompiler project._id, compiler, cb
-					(cb) -> TemplatesManager._setImage project._id, imageName, cb
-					(cb) -> TemplatesManager._setMainFile project._id, mainFile, cb
-					(cb) -> TemplatesManager._setBrandVariationId project._id, brandVariationId, cb
-				], (err) ->
-					return callback err if err?
-					fs.unlink dumpPath, (err) ->
-						logger.err {err}, "error unlinking template zip" if err?
-					update =
-						fromV1TemplateId: templateId,
-						fromV1TemplateVersionId: templateVersionId
-					Project.update { _id: project._id }, update, {}, (err) ->
+		FileWriter.ensureDumpFolderExists (err) ->
+			return callback(err) if err?
+
+			projectName = ProjectDetailsHandler.fixProjectName templateName
+			dumpPath = "#{settings.path.dumpFolder}/#{uuid.v4()}"
+			writeStream = fs.createWriteStream dumpPath
+			writeStream.on "close", ->
+				if zipReq.response.statusCode != 200
+					logger.err { uri: zipUrl, statusCode: zipReq.response.statusCode }, "non-success code getting zip from template API"
+					return callback new Error("get zip failed")
+				ProjectUploadManager.createProjectFromZipArchiveWithName user_id, projectName, dumpPath, (err, project) ->
+					if err?
+						logger.err { err, zipReq }, "problem building project from zip"
+						return callback err
+					async.series [
+						(cb) -> TemplatesManager._setCompiler project._id, compiler, cb
+						(cb) -> TemplatesManager._setImage project._id, imageName, cb
+						(cb) -> TemplatesManager._setMainFile project._id, mainFile, cb
+						(cb) -> TemplatesManager._setBrandVariationId project._id, brandVariationId, cb
+					], (err) ->
 						return callback err if err?
-						callback null, project
-		zipReq.pipe(writeStream)
+						fs.unlink dumpPath, (err) ->
+							logger.err {err}, "error unlinking template zip" if err?
+						update =
+							fromV1TemplateId: templateId,
+							fromV1TemplateVersionId: templateVersionId
+						Project.update { _id: project._id }, update, {}, (err) ->
+							return callback err if err?
+							callback null, project
+			zipReq.pipe(writeStream)
 
 	_setCompiler: (project_id, compiler, callback) ->
 		return callback() unless compiler?

services/web/app/coffee/infrastructure/FileWriter.coffee

@@ -7,7 +7,7 @@ request = require 'request'
 
 module.exports = FileWriter =
 
-	_ensureDumpFolderExists: (callback=(error)->) ->
+	ensureDumpFolderExists: (callback=(error)->) ->
 		fs.mkdir Settings.path.dumpFolder, (error) ->
 			if error? and error.code != 'EEXIST'
 				# Ignore error about already existing
@@ -20,7 +20,7 @@ module.exports = FileWriter =
 	writeContentToDisk: (identifier, content, callback = (error, fsPath)->) ->
 		callback = _.once(callback)
 		fsPath = "#{Settings.path.dumpFolder}/#{identifier}_#{uuid.v4()}"
-		FileWriter._ensureDumpFolderExists (error) ->
+		FileWriter.ensureDumpFolderExists (error) ->
 			return callback(error) if error?
 			fs.writeFile fsPath, content, (error) ->
 				return callback(error) if error?
@@ -31,7 +31,7 @@ module.exports = FileWriter =
 		fsPath = "#{Settings.path.dumpFolder}/#{identifier}_#{uuid.v4()}"
 
 		stream.pause()
-		FileWriter._ensureDumpFolderExists (error) ->
+		FileWriter.ensureDumpFolderExists (error) ->
 			return callback(error) if error?
 			stream.resume()
 

services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee

@@ -48,12 +48,15 @@ describe 'TemplatesManager', ->
 			fixProjectName: sinon.stub().returns(@templateName)
 		@Project =
 			update: sinon.stub().callsArgWith(3, null)
+		@FileWriter =
+			ensureDumpFolderExists: sinon.stub().callsArg(0)
 		@TemplatesManager = SandboxedModule.require modulePath, requires:
 			'../../../js/Features/Uploads/ProjectUploadManager':@ProjectUploadManager
 			'../../../js/Features/Project/ProjectOptionsHandler':@ProjectOptionsHandler
 			'../../../js/Features/Project/ProjectRootDocManager':@ProjectRootDocManager
 			'../../../js/Features/Project/ProjectDetailsHandler':@ProjectDetailsHandler
 			'../../../js/Features/Authentication/AuthenticationController': @AuthenticationController = {getLoggedInUserId: sinon.stub()}
+			'../../infrastructure/FileWriter': @FileWriter
 			'./TemplatesPublisher':@TemplatesPublisher
 			"logger-sharelatex":
 				log:->
@@ -102,6 +105,9 @@ describe 'TemplatesManager', ->
 			it "should update project", ->
 				@Project.update.should.have.been.calledWithMatch { _id: @project_id }, { fromV1TemplateId: @templateId, fromV1TemplateVersionId: @templateVersionId }
 
+			it "should ensure that the dump folder exists", ->
+				sinon.assert.called(@FileWriter.ensureDumpFolderExists)
+
 		describe "when some options not set", ->
 			beforeEach ->
 				@TemplatesManager.createProjectFromV1Template null, null, null, @templateId, @templateName, @templateVersionId, @user_id, null, @callback