From 155831c3e73d4ef731db9c1828ac6f7294e3e9d1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 24 Feb 2016 17:21:20 +0000 Subject: [PATCH] Skip top-level directory if it exists in zip upload --- .../Features/Uploads/ArchiveManager.coffee | 16 ++++++ .../Uploads/ProjectUploadManager.coffee | 6 ++- .../coffee/Uploads/ArchiveManagerTests.coffee | 54 +++++++++++++++++++ .../Uploads/ProjectUploadManagerTests.coffee | 6 ++- 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee index e666b83f24..325df9369f 100644 --- a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee @@ -1,6 +1,8 @@ child = require "child_process" logger = require "logger-sharelatex" metrics = require "../../infrastructure/Metrics" +fs = require "fs" +Path = require "path" module.exports = ArchiveManager = extractZipArchive: (source, destination, _callback = (err) ->) -> @@ -34,4 +36,18 @@ module.exports = ArchiveManager = error = new Error(error) logger.error err:error, source: source, destination: destination, "error unzipping file" callback(error) + + findTopLevelDirectory: (directory, callback = (error, topLevelDir) ->) -> + fs.readdir directory, (error, files) -> + return callback(error) if error? + if files.length == 1 + childPath = Path.join(directory, files[0]) + fs.stat childPath, (error, stat) -> + return callback(error) if error? + if stat.isDirectory() + return callback(null, childPath) + else + return callback(null, directory) + else + return callback(null, directory) diff --git a/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee b/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee index e4535231dd..127c6bc94f 100644 --- a/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee @@ -19,9 +19,11 @@ module.exports = ProjectUploadHandler = destination = @_getDestinationDirectory path ArchiveManager.extractZipArchive path, destination, (error) -> return callback(error) if error? - FileSystemImportManager.addFolderContents owner_id, project_id, folder_id, destination, false, (error) -> + ArchiveManager.findTopLevelDirectory destination, (error, topLevelDestination) -> return callback(error) if error? - rimraf(destination, callback) + FileSystemImportManager.addFolderContents owner_id, project_id, folder_id, topLevelDestination, false, (error) -> + return callback(error) if error? + rimraf(destination, callback) _getDestinationDirectory: (source) -> return path.join(path.dirname(source), "#{path.basename(source, ".zip")}-#{Date.now()}") diff --git a/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee b/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee index d1f72a780e..079ab6d099 100644 --- a/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee @@ -22,6 +22,7 @@ describe "ArchiveManager", -> "child_process": @child "logger-sharelatex": @logger "../../infrastructure/Metrics": @metrics + "fs": @fs = {} describe "extractZipArchive", -> beforeEach -> @@ -69,4 +70,57 @@ describe "ArchiveManager", -> it "should log out the error", -> @logger.error.called.should.equal true + + describe "findTopLevelDirectory", -> + beforeEach -> + @fs.readdir = sinon.stub() + @fs.stat = sinon.stub() + @directory = "test/directory" + + describe "with multiple files", -> + beforeEach -> + @fs.readdir.callsArgWith(1, null, ["multiple", "files"]) + @ArchiveManager.findTopLevelDirectory(@directory, @callback) + + it "should find the files in the directory", -> + @fs.readdir + .calledWith(@directory) + .should.equal true + + it "should return the original directory", -> + @callback + .calledWith(null, @directory) + .should.equal true + + describe "with a single file (not folder)", -> + beforeEach -> + @fs.readdir.callsArgWith(1, null, ["foo.tex"]) + @fs.stat.callsArgWith(1, null, { isDirectory: () -> false }) + @ArchiveManager.findTopLevelDirectory(@directory, @callback) + + it "should check if the file is a directory", -> + @fs.stat + .calledWith(@directory + "/foo.tex") + .should.equal true + + it "should return the original directory", -> + @callback + .calledWith(null, @directory) + .should.equal true + + describe "with a single top-level folder", -> + beforeEach -> + @fs.readdir.callsArgWith(1, null, ["folder"]) + @fs.stat.callsArgWith(1, null, { isDirectory: () -> true }) + @ArchiveManager.findTopLevelDirectory(@directory, @callback) + + it "should check if the file is a directory", -> + @fs.stat + .calledWith(@directory + "/folder") + .should.equal true + + it "should return the child directory", -> + @callback + .calledWith(null, @directory + "/folder") + .should.equal true diff --git a/services/web/test/UnitTests/coffee/Uploads/ProjectUploadManagerTests.coffee b/services/web/test/UnitTests/coffee/Uploads/ProjectUploadManagerTests.coffee index 6cd0a875b3..d4571f0474 100644 --- a/services/web/test/UnitTests/coffee/Uploads/ProjectUploadManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Uploads/ProjectUploadManagerTests.coffee @@ -64,6 +64,7 @@ describe "ProjectUploadManager", -> @destination = "/path/to/zile/file-extracted" @ProjectUploadManager._getDestinationDirectory = sinon.stub().returns @destination @ArchiveManager.extractZipArchive = sinon.stub().callsArg(2) + @ArchiveManager.findTopLevelDirectory = sinon.stub().callsArgWith(1, null, @topLevelDestination = "/path/to/zip/file-extracted/nested") @FileSystemImportManager.addFolderContents = sinon.stub().callsArg(5) @ProjectUploadManager.insertZipArchiveIntoFolder @owner_id, @project_id, @folder_id, @source, @callback @@ -74,8 +75,11 @@ describe "ProjectUploadManager", -> it "should extract the archive", -> @ArchiveManager.extractZipArchive.calledWith(@source, @destination).should.equal true + it "should find the top level directory", -> + @ArchiveManager.findTopLevelDirectory.calledWith(@destination).should.equal true + it "should insert the extracted archive into the folder", -> - @FileSystemImportManager.addFolderContents.calledWith(@owner_id, @project_id, @folder_id, @destination, false) + @FileSystemImportManager.addFolderContents.calledWith(@owner_id, @project_id, @folder_id, @topLevelDestination, false) .should.equal true it "should return the callback", ->