From 2c30ae0f8ab2975f8e6348ddccd93e37a6b9c8fe Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Wed, 1 May 2019 10:23:35 +0100 Subject: [PATCH] Merge pull request #1738 from sharelatex/spd-dumpfolder Ensure dump folder exists before opening templates GitOrigin-RevId: 3c7269fbcdff45a06e505154a35ce6d2bd4b7ae1 --- .../Templates/TemplatesManager.coffee | 56 ++++++++++--------- .../coffee/infrastructure/FileWriter.coffee | 6 +- .../Templates/TemplatesManagerTests.coffee | 6 ++ 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/services/web/app/coffee/Features/Templates/TemplatesManager.coffee b/services/web/app/coffee/Features/Templates/TemplatesManager.coffee index 88844ea03c..8e95649672 100644 --- a/services/web/app/coffee/Features/Templates/TemplatesManager.coffee +++ b/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? diff --git a/services/web/app/coffee/infrastructure/FileWriter.coffee b/services/web/app/coffee/infrastructure/FileWriter.coffee index 4e37273338..e8b9de8fea 100644 --- a/services/web/app/coffee/infrastructure/FileWriter.coffee +++ b/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() diff --git a/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee b/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee index 58900711f7..2b9d636640 100644 --- a/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee +++ b/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