From 5aee58c6f4ad476fe3c84f6e28b59e84931126e5 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Tue, 23 Apr 2024 10:16:30 +0100 Subject: [PATCH] Merge pull request #17956 from overleaf/mj-web-promisify-restore-manager [web] Promisify RestoreManager GitOrigin-RevId: cd3395f8cb7b90e19828297e1c89c1d3850877a6 --- .../src/Features/History/RestoreManager.js | 153 +++++++----------- .../unit/src/History/RestoreManagerTests.js | 132 +++++++-------- 2 files changed, 113 insertions(+), 172 deletions(-) diff --git a/services/web/app/src/Features/History/RestoreManager.js b/services/web/app/src/Features/History/RestoreManager.js index 3fc74d0ea0..551b58ce38 100644 --- a/services/web/app/src/Features/History/RestoreManager.js +++ b/services/web/app/src/Features/History/RestoreManager.js @@ -1,122 +1,79 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let RestoreManager const Settings = require('@overleaf/settings') const Path = require('path') const FileWriter = require('../../infrastructure/FileWriter') const FileSystemImportManager = require('../Uploads/FileSystemImportManager') -const ProjectEntityHandler = require('../Project/ProjectEntityHandler') const EditorController = require('../Editor/EditorController') const Errors = require('../Errors/Errors') const moment = require('moment') +const { callbackifyAll } = require('@overleaf/promise-utils') -module.exports = RestoreManager = { - restoreFileFromV2(userId, projectId, version, pathname, callback) { - if (callback == null) { - callback = function () {} - } - return RestoreManager._writeFileVersionToDisk( +const RestoreManager = { + async restoreFileFromV2(userId, projectId, version, pathname) { + const fsPath = await RestoreManager._writeFileVersionToDisk( projectId, version, - pathname, - function (error, fsPath) { - if (error != null) { - return callback(error) - } - const basename = Path.basename(pathname) - let dirname = Path.dirname(pathname) - if (dirname === '.') { - // no directory - dirname = '' - } - return RestoreManager._findOrCreateFolder( - projectId, - dirname, - function (error, parentFolderId) { - if (error != null) { - return callback(error) - } - const addEntityWithName = (name, callback) => - FileSystemImportManager.addEntity( - userId, - projectId, - parentFolderId, - name, - fsPath, - false, - callback - ) - return RestoreManager._addEntityWithUniqueName( - addEntityWithName, - basename, - callback - ) - } - ) - } + pathname ) - }, - - _findOrCreateFolder(projectId, dirname, callback) { - if (callback == null) { - callback = function () {} + const basename = Path.basename(pathname) + let dirname = Path.dirname(pathname) + if (dirname === '.') { + // no directory + dirname = '' } - return EditorController.mkdirp( + const parentFolderId = await RestoreManager._findOrCreateFolder( projectId, - dirname, - function (error, newFolders, lastFolder) { - if (error != null) { - return callback(error) - } - return callback(null, lastFolder != null ? lastFolder._id : undefined) - } + dirname + ) + const addEntityWithName = async name => + await FileSystemImportManager.promises.addEntity( + userId, + projectId, + parentFolderId, + name, + fsPath, + false + ) + return await RestoreManager._addEntityWithUniqueName( + addEntityWithName, + basename ) }, - _addEntityWithUniqueName(addEntityWithName, basename, callback) { - if (callback == null) { - callback = function () {} - } - return addEntityWithName(basename, function (error, entity) { - if (error != null) { - if (error instanceof Errors.InvalidNameError) { - // likely a duplicate name, so try with a prefix - const date = moment(new Date()).format('Do MMM YY H:mm:ss') - // Move extension to the end so the file type is preserved - const extension = Path.extname(basename) - basename = Path.basename(basename, extension) - basename = `${basename} (Restored on ${date})` - if (extension !== '') { - basename = `${basename}${extension}` - } - return addEntityWithName(basename, callback) - } else { - return callback(error) - } - } else { - return callback(null, entity) - } - }) + async _findOrCreateFolder(projectId, dirname) { + const { lastFolder } = await EditorController.promises.mkdirp( + projectId, + dirname + ) + return lastFolder?._id }, - _writeFileVersionToDisk(projectId, version, pathname, callback) { - if (callback == null) { - callback = function () {} + async _addEntityWithUniqueName(addEntityWithName, basename) { + try { + return await addEntityWithName(basename) + } catch (error) { + if (error instanceof Errors.InvalidNameError) { + // likely a duplicate name, so try with a prefix + const date = moment(new Date()).format('Do MMM YY H:mm:ss') + // Move extension to the end so the file type is preserved + const extension = Path.extname(basename) + basename = Path.basename(basename, extension) + basename = `${basename} (Restored on ${date})` + if (extension !== '') { + basename = `${basename}${extension}` + } + return await addEntityWithName(basename) + } else { + throw error + } } + }, + + async _writeFileVersionToDisk(projectId, version, pathname) { const url = `${ Settings.apis.project_history.url }/project/${projectId}/version/${version}/${encodeURIComponent(pathname)}` - return FileWriter.writeUrlToDisk(projectId, url, callback) + return await FileWriter.promises.writeUrlToDisk(projectId, url) }, } + +module.exports = { ...callbackifyAll(RestoreManager), promises: RestoreManager } diff --git a/services/web/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js index 0f8884fc81..c0cf681484 100644 --- a/services/web/test/unit/src/History/RestoreManagerTests.js +++ b/services/web/test/unit/src/History/RestoreManagerTests.js @@ -1,18 +1,4 @@ -/* eslint-disable - max-len, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') -const assert = require('assert') -const { expect } = require('chai') const sinon = require('sinon') const modulePath = require('path').join( __dirname, @@ -21,6 +7,7 @@ const modulePath = require('path').join( const Errors = require('../../../../app/src/Features/Errors/Errors') const tk = require('timekeeper') const moment = require('moment') +const { expect } = require('chai') describe('RestoreManager', function () { beforeEach(function () { @@ -28,62 +15,62 @@ describe('RestoreManager', function () { this.RestoreManager = SandboxedModule.require(modulePath, { requires: { '@overleaf/settings': {}, - '../../infrastructure/FileWriter': (this.FileWriter = {}), - '../Uploads/FileSystemImportManager': (this.FileSystemImportManager = - {}), - '../Project/ProjectEntityHandler': (this.ProjectEntityHandler = {}), - '../Editor/EditorController': (this.EditorController = {}), + '../../infrastructure/FileWriter': (this.FileWriter = { promises: {} }), + '../Uploads/FileSystemImportManager': (this.FileSystemImportManager = { + promises: {}, + }), + '../Editor/EditorController': (this.EditorController = { + promises: {}, + }), }, }) this.user_id = 'mock-user-id' this.project_id = 'mock-project-id' this.version = 42 - return (this.callback = sinon.stub()) }) afterEach(function () { - return tk.reset() + tk.reset() }) describe('restoreFileFromV2', function () { beforeEach(function () { - this.RestoreManager._writeFileVersionToDisk = sinon + this.RestoreManager.promises._writeFileVersionToDisk = sinon .stub() - .yields(null, (this.fsPath = '/tmp/path/on/disk')) - this.RestoreManager._findOrCreateFolder = sinon + .resolves((this.fsPath = '/tmp/path/on/disk')) + this.RestoreManager.promises._findOrCreateFolder = sinon .stub() - .yields(null, (this.folder_id = 'mock-folder-id')) - return (this.FileSystemImportManager.addEntity = sinon + .resolves((this.folder_id = 'mock-folder-id')) + this.FileSystemImportManager.promises.addEntity = sinon .stub() - .yields(null, (this.entity = 'mock-entity'))) + .resolves((this.entity = 'mock-entity')) }) describe('with a file not in a folder', function () { - beforeEach(function () { + beforeEach(async function () { this.pathname = 'foo.tex' - return this.RestoreManager.restoreFileFromV2( + this.result = await this.RestoreManager.promises.restoreFileFromV2( this.user_id, this.project_id, this.version, - this.pathname, - this.callback + this.pathname ) }) it('should write the file version to disk', function () { - return this.RestoreManager._writeFileVersionToDisk + this.RestoreManager.promises._writeFileVersionToDisk .calledWith(this.project_id, this.version, this.pathname) .should.equal(true) }) it('should find the root folder', function () { - return this.RestoreManager._findOrCreateFolder + this.RestoreManager.promises._findOrCreateFolder .calledWith(this.project_id, '') .should.equal(true) }) it('should add the entity', function () { - return this.FileSystemImportManager.addEntity + this.FileSystemImportManager.promises.addEntity .calledWith( this.user_id, this.project_id, @@ -95,31 +82,30 @@ describe('RestoreManager', function () { .should.equal(true) }) - it('should call the callback with the entity', function () { - return this.callback.calledWith(null, this.entity).should.equal(true) + it('should return the entity', function () { + expect(this.result).to.equal(this.entity) }) }) describe('with a file in a folder', function () { - beforeEach(function () { + beforeEach(async function () { this.pathname = 'foo/bar.tex' - return this.RestoreManager.restoreFileFromV2( + await this.RestoreManager.promises.restoreFileFromV2( this.user_id, this.project_id, this.version, - this.pathname, - this.callback + this.pathname ) }) it('should find the folder', function () { - return this.RestoreManager._findOrCreateFolder + this.RestoreManager.promises._findOrCreateFolder .calledWith(this.project_id, 'foo') .should.equal(true) }) it('should add the entity by its basename', function () { - return this.FileSystemImportManager.addEntity + this.FileSystemImportManager.promises.addEntity .calledWith( this.user_id, this.project_id, @@ -134,81 +120,79 @@ describe('RestoreManager', function () { }) describe('_findOrCreateFolder', function () { - beforeEach(function () { - this.EditorController.mkdirp = sinon - .stub() - .yields(null, [], { _id: (this.folder_id = 'mock-folder-id') }) - return this.RestoreManager._findOrCreateFolder( + beforeEach(async function () { + this.EditorController.promises.mkdirp = sinon.stub().resolves({ + newFolders: [], + lastFolder: { _id: (this.folder_id = 'mock-folder-id') }, + }) + this.result = await this.RestoreManager.promises._findOrCreateFolder( this.project_id, - 'folder/name', - this.callback + 'folder/name' ) }) it('should look up or create the folder', function () { - return this.EditorController.mkdirp + this.EditorController.promises.mkdirp .calledWith(this.project_id, 'folder/name') .should.equal(true) }) it('should return the folder_id', function () { - return this.callback.calledWith(null, this.folder_id).should.equal(true) + expect(this.result).to.equal(this.folder_id) }) }) describe('_addEntityWithUniqueName', function () { beforeEach(function () { this.addEntityWithName = sinon.stub() - return (this.name = 'foo.tex') + this.name = 'foo.tex' }) describe('with a valid name', function () { - beforeEach(function () { - this.addEntityWithName.yields(null, (this.entity = 'mock-entity')) - return this.RestoreManager._addEntityWithUniqueName( - this.addEntityWithName, - this.name, - this.callback - ) + beforeEach(async function () { + this.addEntityWithName.resolves((this.entity = 'mock-entity')) + this.result = + await this.RestoreManager.promises._addEntityWithUniqueName( + this.addEntityWithName, + this.name + ) }) it('should add the entity', function () { - return this.addEntityWithName.calledWith(this.name).should.equal(true) + this.addEntityWithName.calledWith(this.name).should.equal(true) }) it('should return the entity', function () { - return this.callback.calledWith(null, this.entity).should.equal(true) + expect(this.result).to.equal(this.entity) }) }) describe('with an invalid name', function () { - beforeEach(function () { - this.addEntityWithName - .onFirstCall() - .yields(new Errors.InvalidNameError()) + beforeEach(async function () { + this.addEntityWithName.rejects(new Errors.InvalidNameError()) this.addEntityWithName .onSecondCall() - .yields(null, (this.entity = 'mock-entity')) - return this.RestoreManager._addEntityWithUniqueName( - this.addEntityWithName, - this.name, - this.callback - ) + .resolves((this.entity = 'mock-entity')) + this.result = + await this.RestoreManager.promises._addEntityWithUniqueName( + this.addEntityWithName, + this.name + ) }) it('should try to add the entity with its original name', function () { - return this.addEntityWithName.calledWith('foo.tex').should.equal(true) + this.addEntityWithName.calledWith('foo.tex').should.equal(true) }) it('should try to add the entity with a unique name', function () { const date = moment(new Date()).format('Do MMM YY H:mm:ss') - return this.addEntityWithName + this.addEntityWithName .calledWith(`foo (Restored on ${date}).tex`) .should.equal(true) }) it('should return the entity', function () { - return this.callback.calledWith(null, this.entity).should.equal(true) + expect(this.result).to.equal(this.entity) }) }) })