diff --git a/services/web/app/src/Features/Templates/TemplatesManager.js b/services/web/app/src/Features/Templates/TemplatesManager.js index baa840b2e2..f105b6ae85 100644 --- a/services/web/app/src/Features/Templates/TemplatesManager.js +++ b/services/web/app/src/Features/Templates/TemplatesManager.js @@ -1,22 +1,25 @@ const { Project } = require('../../models/Project') -const OError = require('@overleaf/o-error') const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') -const ProjectOptionsHandler = require('../Project/ProjectOptionsHandler') -const ProjectRootDocManager = require('../Project/ProjectRootDocManager') +const ProjectOptionsHandler = + require('../Project/ProjectOptionsHandler').promises +const ProjectRootDocManager = + require('../Project/ProjectRootDocManager').promises const ProjectUploadManager = require('../Uploads/ProjectUploadManager') -const async = require('async') const fs = require('fs') const util = require('util') const logger = require('@overleaf/logger') -const { fetchJson, RequestFailedError } = require('@overleaf/fetch-utils') -const request = require('request') +const { + fetchJson, + fetchStreamWithResponse, + RequestFailedError, +} = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') const crypto = require('crypto') const Errors = require('../Errors/Errors') -const _ = require('lodash') +const { pipeline } = require('stream/promises') const TemplatesManager = { - createProjectFromV1Template( + async createProjectFromV1Template( brandVariationId, compiler, mainFile, @@ -24,153 +27,113 @@ const TemplatesManager = { templateName, templateVersionId, userId, - imageName, - _callback + imageName ) { - const callback = _.once(_callback) const zipUrl = `${settings.apis.v1.url}/api/v1/overleaf/templates/${templateVersionId}` - const zipReq = request(zipUrl, { - auth: { + const zipReq = await fetchStreamWithResponse(zipUrl, { + basicAuth: { user: settings.apis.v1.user, - pass: settings.apis.v1.pass, + password: settings.apis.v1.pass, }, - timeout: settings.apis.v1.timeout, - }) - zipReq.on('error', function (err) { - logger.warn({ err }, 'error getting zip from template API') - return callback(err) + signal: AbortSignal.timeout(settings.apis.v1.timeout), }) const projectName = ProjectDetailsHandler.fixProjectName(templateName) const dumpPath = `${settings.path.dumpFolder}/${crypto.randomUUID()}` const writeStream = fs.createWriteStream(dumpPath) - const attributes = { - fromV1TemplateId: templateId, - fromV1TemplateVersionId: templateVersionId, - } - writeStream.on('close', function () { - if (zipReq.response.statusCode !== 200) { + try { + const attributes = { + fromV1TemplateId: templateId, + fromV1TemplateVersionId: templateVersionId, + } + await pipeline(zipReq.stream, writeStream) + + if (zipReq.response.status !== 200) { logger.warn( - { uri: zipUrl, statusCode: zipReq.response.statusCode }, + { uri: zipUrl, statusCode: zipReq.response.status }, 'non-success code getting zip from template API' ) - return callback(new Error('get zip failed')) + throw new Error(`get zip failed: ${zipReq.response.status}`) } - ProjectUploadManager.createProjectFromZipArchiveWithName( - userId, - projectName, - dumpPath, - attributes, - function (err, project) { - if (err) { - OError.tag(err, 'problem building project from zip', { - zipReq, - }) - 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 - ), - ], - function (err) { - if (err) { - return callback(err) - } - fs.unlink(dumpPath, function (err) { - if (err) { - return logger.err({ err }, 'error unlinking template zip') - } - }) - const update = { - fromV1TemplateId: templateId, - fromV1TemplateVersionId: templateVersionId, - } - Project.updateOne( - { _id: project._id }, - update, - {}, - function (err) { - if (err) { - return callback(err) - } - callback(null, project) - } - ) - } - ) - } - ) - }) - zipReq.pipe(writeStream) - }, + const project = + await ProjectUploadManager.promises.createProjectFromZipArchiveWithName( + userId, + projectName, + dumpPath, + attributes + ) - _setCompiler(projectId, compiler, callback) { - if (compiler == null) { - return callback() + await TemplatesManager._setCompiler(project._id, compiler) + await TemplatesManager._setImage(project._id, imageName) + await TemplatesManager._setMainFile(project._id, mainFile) + await TemplatesManager._setBrandVariationId(project._id, brandVariationId) + + const update = { + fromV1TemplateId: templateId, + fromV1TemplateVersionId: templateVersionId, + } + await Project.updateOne({ _id: project._id }, update, {}) + + return project + } finally { + await fs.promises.unlink(dumpPath) } - ProjectOptionsHandler.setCompiler(projectId, compiler, callback) }, - _setImage(projectId, imageName, callback) { + async _setCompiler(projectId, compiler) { + if (compiler == null) { + return + } + await ProjectOptionsHandler.setCompiler(projectId, compiler) + }, + + async _setImage(projectId, imageName) { if (!imageName) { imageName = 'wl_texlive:2018.1' } - ProjectOptionsHandler.setImageName(projectId, imageName, callback) + + await ProjectOptionsHandler.setImageName(projectId, imageName) }, - _setMainFile(projectId, mainFile, callback) { + async _setMainFile(projectId, mainFile) { if (mainFile == null) { - return callback() + return } - ProjectRootDocManager.setRootDocFromName(projectId, mainFile, callback) + await ProjectRootDocManager.setRootDocFromName(projectId, mainFile) }, - _setBrandVariationId(projectId, brandVariationId, callback) { + async _setBrandVariationId(projectId, brandVariationId) { if (brandVariationId == null) { - return callback() + return } - ProjectOptionsHandler.setBrandVariationId( - projectId, - brandVariationId, - callback - ) + await ProjectOptionsHandler.setBrandVariationId(projectId, brandVariationId) }, - promises: { - async fetchFromV1(templateId) { - const url = new URL( - `/api/v2/templates/${templateId}`, - settings.apis.v1.url - ) + async fetchFromV1(templateId) { + const url = new URL(`/api/v2/templates/${templateId}`, settings.apis.v1.url) - try { - return await fetchJson(url, { - basicAuth: { - user: settings.apis.v1.user, - password: settings.apis.v1.pass, - }, - signal: AbortSignal.timeout(settings.apis.v1.timeout), - }) - } catch (err) { - if (err instanceof RequestFailedError && err.response.status === 404) { - throw new Errors.NotFoundError() - } else { - throw err - } + try { + return await fetchJson(url, { + basicAuth: { + user: settings.apis.v1.user, + password: settings.apis.v1.pass, + }, + signal: AbortSignal.timeout(settings.apis.v1.timeout), + }) + } catch (err) { + if (err instanceof RequestFailedError && err.response.status === 404) { + throw new Errors.NotFoundError() + } else { + throw err } - }, + } }, } -TemplatesManager.fetchFromV1 = util.callbackify( - TemplatesManager.promises.fetchFromV1 -) -module.exports = TemplatesManager +module.exports = { + promises: TemplatesManager, + createProjectFromV1Template: util.callbackify( + TemplatesManager.createProjectFromV1Template + ), + fetchFromV1: util.callbackify(TemplatesManager.fetchFromV1), +} diff --git a/services/web/test/unit/src/Templates/TemplatesManagerTests.js b/services/web/test/unit/src/Templates/TemplatesManagerTests.js index 38a5933615..9e01f73e27 100644 --- a/services/web/test/unit/src/Templates/TemplatesManagerTests.js +++ b/services/web/test/unit/src/Templates/TemplatesManagerTests.js @@ -11,9 +11,9 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const SandboxedModule = require('sandboxed-module') -const assert = require('assert') const sinon = require('sinon') const { RequestFailedError } = require('@overleaf/fetch-utils') +const { ReadableString } = require('@overleaf/stream-utils') const modulePath = '../../../../app/src/Features/Templates/TemplatesManager' @@ -30,6 +30,9 @@ describe('TemplatesManager', function () { this.user_id = 'user-id' this.dumpPath = `${this.dumpFolder}/${this.uuid}` this.callback = sinon.stub() + this.pipeline = sinon.stub().callsFake(async (stream, res) => { + if (res.callback) res.callback() + }) this.request = sinon.stub().returns({ pipe() {}, on() {}, @@ -38,31 +41,50 @@ describe('TemplatesManager', function () { }, }) this.fs = { + promises: { unlink: sinon.stub() }, unlink: sinon.stub(), createWriteStream: sinon.stub().returns({ on: sinon.stub().yields() }), } this.ProjectUploadManager = { - createProjectFromZipArchiveWithName: sinon - .stub() - .callsArgWith(4, null, { _id: this.project_id }), + promises: { + createProjectFromZipArchiveWithName: sinon + .stub() + .resolves({ _id: this.project_id }), + }, } this.dumpFolder = 'dump/path' this.ProjectOptionsHandler = { - setCompiler: sinon.stub().callsArgWith(2), - setImageName: sinon.stub().callsArgWith(2), - setBrandVariationId: sinon.stub().callsArgWith(2), + promises: { + setCompiler: sinon.stub().resolves(), + setImageName: sinon.stub().resolves(), + setBrandVariationId: sinon.stub().resolves(), + }, } this.uuid = '1234' this.ProjectRootDocManager = { - setRootDocFromName: sinon.stub().callsArgWith(2), + promises: { + setRootDocFromName: sinon.stub().resolves(), + }, } this.ProjectDetailsHandler = { getProjectDescription: sinon.stub(), fixProjectName: sinon.stub().returns(this.templateName), } - this.Project = { updateOne: sinon.stub().callsArgWith(3, null) } + this.Project = { updateOne: sinon.stub().resolves() } + this.mockStream = new ReadableString('{}') + this.mockResponse = { + status: 200, + headers: new Headers({ + 'Content-Length': '2', + 'Content-Type': 'application/json', + }), + } this.FetchUtils = { fetchJson: sinon.stub(), + fetchStreamWithResponse: sinon.stub().resolves({ + stream: this.mockStream, + response: this.mockResponse, + }), RequestFailedError, } this.TemplatesManager = SandboxedModule.require(modulePath, { @@ -85,6 +107,7 @@ describe('TemplatesManager', function () { url: (this.v1Url = 'http://overleaf.com'), user: 'overleaf', pass: 'password', + timeout: 10, }, }, overleaf: { @@ -97,8 +120,9 @@ describe('TemplatesManager', function () { request: this.request, fs: this.fs, '../../models/Project': { Project: this.Project }, + 'stream/promises': { pipeline: this.pipeline }, }, - }) + }).promises return (this.zipUrl = '%2Ftemplates%2F52fb86a81ae1e566597a25f6%2Fv%2F4%2Fzip&templateName=Moderncv%20Banking&compiler=pdflatex') }) @@ -114,13 +138,12 @@ describe('TemplatesManager', function () { this.templateName, this.templateVersionId, this.user_id, - this.imageName, - this.callback + this.imageName ) }) it('should fetch zip from v1 based on template id', function () { - return this.request.should.have.been.calledWith( + return this.FetchUtils.fetchStreamWithResponse.should.have.been.calledWith( `${this.v1Url}/api/v1/overleaf/templates/${this.templateVersionId}` ) }) @@ -132,7 +155,7 @@ describe('TemplatesManager', function () { }) it('should create project', function () { - return this.ProjectUploadManager.createProjectFromZipArchiveWithName.should.have.been.calledWithMatch( + return this.ProjectUploadManager.promises.createProjectFromZipArchiveWithName.should.have.been.calledWithMatch( this.user_id, this.templateName, this.dumpPath, @@ -144,23 +167,25 @@ describe('TemplatesManager', function () { }) it('should unlink file', function () { - return this.fs.unlink.should.have.been.calledWith(this.dumpPath) + return this.fs.promises.unlink.should.have.been.calledWith( + this.dumpPath + ) }) it('should set project options when passed', function () { - this.ProjectOptionsHandler.setCompiler.should.have.been.calledWithMatch( + this.ProjectOptionsHandler.promises.setCompiler.should.have.been.calledWithMatch( this.project_id, this.compiler ) - this.ProjectOptionsHandler.setImageName.should.have.been.calledWithMatch( + this.ProjectOptionsHandler.promises.setImageName.should.have.been.calledWithMatch( this.project_id, this.imageName ) - this.ProjectRootDocManager.setRootDocFromName.should.have.been.calledWithMatch( + this.ProjectRootDocManager.promises.setRootDocFromName.should.have.been.calledWithMatch( this.project_id, this.mainFile ) - return this.ProjectOptionsHandler.setBrandVariationId.should.have.been.calledWithMatch( + return this.ProjectOptionsHandler.promises.setBrandVariationId.should.have.been.calledWithMatch( this.project_id, this.brandVariationId ) @@ -187,18 +212,21 @@ describe('TemplatesManager', function () { this.templateName, this.templateVersionId, this.user_id, - null, - this.callback + null ) }) it('should not set missing project options', function () { - this.ProjectOptionsHandler.setCompiler.called.should.equal(false) - this.ProjectRootDocManager.setRootDocFromName.called.should.equal(false) - this.ProjectOptionsHandler.setBrandVariationId.called.should.equal( + this.ProjectOptionsHandler.promises.setCompiler.called.should.equal( false ) - return this.ProjectOptionsHandler.setImageName.should.have.been.calledWithMatch( + this.ProjectRootDocManager.promises.setRootDocFromName.called.should.equal( + false + ) + this.ProjectOptionsHandler.promises.setBrandVariationId.called.should.equal( + false + ) + return this.ProjectOptionsHandler.promises.setImageName.should.have.been.calledWithMatch( this.project_id, 'wl_texlive:2018.1' )