diff --git a/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs b/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs index c1fce8f934..e0ee4e2ffe 100644 --- a/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs +++ b/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs @@ -30,38 +30,44 @@ export default { async function createProjectFromZipArchive(ownerId, defaultName, zipPath) { const contentsPath = await _extractZip(zipPath) - const { path, content } = - await ProjectRootDocManager.promises.findRootDocFileFromDirectory( - contentsPath - ) - - const projectName = - DocumentHelper.getTitleFromTexContent(content || '') || defaultName - const uniqueName = await _generateUniqueName(ownerId, projectName) - const project = await ProjectCreationHandler.promises.createBlankProject( - ownerId, - uniqueName - ) try { - await _initializeProjectWithZipContents(ownerId, project, contentsPath) - - if (path) { - await ProjectRootDocManager.promises.setRootDocFromName(project._id, path) - } - } catch (err) { - // no need to wait for the cleanup here - ProjectDeleter.promises - .deleteProject(project._id) - .catch(err => - logger.error( - { err, projectId: project._id }, - 'there was an error cleaning up project after importing a zip failed' - ) + const { path, content } = + await ProjectRootDocManager.promises.findRootDocFileFromDirectory( + contentsPath ) - throw err + + const projectName = + DocumentHelper.getTitleFromTexContent(content || '') || defaultName + const uniqueName = await _generateUniqueName(ownerId, projectName) + const project = await ProjectCreationHandler.promises.createBlankProject( + ownerId, + uniqueName + ) + try { + await _initializeProjectWithZipContents(ownerId, project, contentsPath) + + if (path) { + await ProjectRootDocManager.promises.setRootDocFromName( + project._id, + path + ) + } + } catch (err) { + // no need to wait for the cleanup here + ProjectDeleter.promises + .deleteProject(project._id) + .catch(err => + logger.error( + { err, projectId: project._id }, + 'there was an error cleaning up project after importing a zip failed' + ) + ) + throw err + } + return project + } finally { + await fs.promises.rm(contentsPath, { recursive: true, force: true }) } - await fs.promises.rm(contentsPath, { recursive: true, force: true }) - return project } async function createProjectFromZipArchiveWithName( @@ -71,30 +77,33 @@ async function createProjectFromZipArchiveWithName( attributes = {} ) { const contentsPath = await _extractZip(zipPath) - const uniqueName = await _generateUniqueName(ownerId, proposedName) - const project = await ProjectCreationHandler.promises.createBlankProject( - ownerId, - uniqueName, - attributes - ) - try { - await _initializeProjectWithZipContents(ownerId, project, contentsPath) - await ProjectRootDocManager.promises.setRootDocAutomatically(project._id) - } catch (err) { - // no need to wait for the cleanup here - ProjectDeleter.promises - .deleteProject(project._id) - .catch(err => - logger.error( - { err, projectId: project._id }, - 'there was an error cleaning up project after importing a zip failed' + const uniqueName = await _generateUniqueName(ownerId, proposedName) + const project = await ProjectCreationHandler.promises.createBlankProject( + ownerId, + uniqueName, + attributes + ) + + try { + await _initializeProjectWithZipContents(ownerId, project, contentsPath) + await ProjectRootDocManager.promises.setRootDocAutomatically(project._id) + } catch (err) { + // no need to wait for the cleanup here + ProjectDeleter.promises + .deleteProject(project._id) + .catch(err => + logger.error( + { err, projectId: project._id }, + 'there was an error cleaning up project after importing a zip failed' + ) ) - ) - throw err + throw err + } + return project + } finally { + await fs.promises.rm(contentsPath, { recursive: true, force: true }) } - await fs.promises.rm(contentsPath, { recursive: true, force: true }) - return project } async function _extractZip(zipPath) { @@ -102,7 +111,13 @@ async function _extractZip(zipPath) { Path.dirname(zipPath), `${Path.basename(zipPath, '.zip')}-${Date.now()}` ) - await ArchiveManager.promises.extractZipArchive(zipPath, destination) + try { + await ArchiveManager.promises.extractZipArchive(zipPath, destination) + } catch (error) { + logger.debug({ zipPath, error }, 'error extracting from zip archive') + await fs.promises.rm(destination, { recursive: true, force: true }) + throw error + } return destination } diff --git a/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs b/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs index 8bc28e798b..696b629405 100644 --- a/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs +++ b/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs @@ -323,6 +323,48 @@ describe('ProjectUploadManager', function () { .been.called }) }) + + describe('when extraction fails', function () { + beforeEach(async function (ctx) { + ctx.ArchiveManager.promises.extractZipArchive.rejects(new Error('oops')) + await expect( + ctx.ProjectUploadManager.promises.createProjectFromZipArchive( + ctx.ownerId, + ctx.projectName, + ctx.zipPath + ) + ).to.be.rejectedWith('oops') + }) + + it('should remove the destination directory', function (ctx) { + ctx.fs.promises.rm.should.have.been.calledWith(ctx.extractedZipPath, { + recursive: true, + force: true, + }) + }) + }) + + describe('when project creation fails', function () { + beforeEach(async function (ctx) { + ctx.ProjectCreationHandler.promises.createBlankProject.rejects( + new Error('oops') + ) + await expect( + ctx.ProjectUploadManager.promises.createProjectFromZipArchive( + ctx.ownerId, + ctx.projectName, + ctx.zipPath + ) + ).to.be.rejectedWith('oops') + }) + + it('should remove the destination directory', function (ctx) { + ctx.fs.promises.rm.should.have.been.calledWith(ctx.extractedZipPath, { + recursive: true, + force: true, + }) + }) + }) }) describe('createProjectFromZipArchiveWithName', function () { @@ -406,6 +448,13 @@ describe('ProjectUploadManager', function () { ctx.project._id ) }) + + it('should remove the destination directory', function (ctx) { + ctx.fs.promises.rm.should.have.been.calledWith(ctx.extractedZipPath, { + recursive: true, + force: true, + }) + }) }) describe('when setting automatically the root doc fails', function () { @@ -425,6 +474,33 @@ describe('ProjectUploadManager', function () { ctx.project._id ) }) + + it('should remove the destination directory', function (ctx) { + ctx.fs.promises.rm.should.have.been.calledWith(ctx.extractedZipPath, { + recursive: true, + force: true, + }) + }) + }) + + describe('when extraction fails', function () { + beforeEach(async function (ctx) { + ctx.ArchiveManager.promises.extractZipArchive.rejects(new Error('oops')) + await expect( + ctx.ProjectUploadManager.promises.createProjectFromZipArchiveWithName( + ctx.ownerId, + ctx.projectName, + ctx.zipPath + ) + ).to.be.rejectedWith('oops') + }) + + it('should remove the destination directory', function (ctx) { + ctx.fs.promises.rm.should.have.been.calledWith(ctx.extractedZipPath, { + recursive: true, + force: true, + }) + }) }) }) })