From 3d352b35cb09fd7b5112c8f43dc57fb25f10a2c7 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Wed, 7 Jan 2026 12:35:15 +0100 Subject: [PATCH] Merge pull request #30394 from overleaf/revert-30391-dp-test-revert-1 [web] Reapply: Promisify ProjectLocator (#30319) GitOrigin-RevId: 0cde095b81ea61211881b6b29fa4dd58d952a162 --- .../src/Features/Project/ProjectLocator.mjs | 240 +++++++----------- .../unit/src/Project/ProjectLocator.test.mjs | 13 +- 2 files changed, 106 insertions(+), 147 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectLocator.mjs b/services/web/app/src/Features/Project/ProjectLocator.mjs index abc80b9dcb..88ec8ed5f2 100644 --- a/services/web/app/src/Features/Project/ProjectLocator.mjs +++ b/services/web/app/src/Features/Project/ProjectLocator.mjs @@ -1,17 +1,11 @@ import _ from 'lodash' import logger from '@overleaf/logger' import OError from '@overleaf/o-error' -import async from 'async' import ProjectGetter from './ProjectGetter.mjs' import Errors from '../Errors/Errors.js' -import { promisifyMultiResult } from '@overleaf/promise-utils' +import { callbackifyMultiResult } from '@overleaf/promise-utils' import { iterablePaths } from './IterablePath.mjs' -/** - * @param project - * @param predicate - * @returns {{path: string, value: *}} - */ function findDeep(project, predicate) { function find(value, path) { if (predicate(value)) { @@ -29,17 +23,8 @@ function findDeep(project, predicate) { return find(project.rootFolder, ['rootFolder']) } -function findElement(options, _callback) { - // The search algorithm below potentially invokes the callback multiple - // times. - const callback = _.once(_callback) - - const { - project, - project_id: projectId, - element_id: elementId, - type, - } = options +async function findElement(options) { + const { project_id: projectId, element_id: elementId, type } = options const elementType = sanitizeTypeOfElement(type) let count = 0 @@ -50,10 +35,9 @@ function findElement(options, _callback) { projectId || project._id }` ) - callback(new Errors.NotFoundError('entity not found')) + throw new Errors.NotFoundError('entity not found') } } - function search(searchFolder, path) { count++ const element = _.find( @@ -65,9 +49,9 @@ function findElement(options, _callback) { searchFolder.folders != null && searchFolder.folders.length !== 0 ) { - _.forEach(searchFolder.folders, (folder, index) => { + for (const [index, folder] of searchFolder.folders.entries()) { if (folder == null) { - return + continue } const newPath = {} for (const key of Object.keys(path)) { @@ -76,8 +60,11 @@ function findElement(options, _callback) { } // make a value copy of the string newPath.fileSystem += `/${folder.name}` newPath.mongo += `.folders.${index}` - search(folder, newPath) - }) + const result = search(folder, newPath) + if (result) { + return result + } + } endOfBranch() } else if (element != null) { const elementPlaceInArray = getIndexOf( @@ -86,8 +73,9 @@ function findElement(options, _callback) { ) path.fileSystem += `/${element.name}` path.mongo += `.${elementType}.${elementPlaceInArray}` - callback(null, element, path, searchFolder) - } else if (element == null) { + return { element, path, folder: searchFolder } + } + if (element == null) { endOfBranch() } } @@ -99,97 +87,70 @@ function findElement(options, _callback) { elementId + '' === project.rootFolder[0]._id + '' && elementType === 'folders' ) { - callback(null, project.rootFolder[0], path, null) - } else { - search(project.rootFolder[0], path) + return { element: project.rootFolder[0], path, folder: null } } + return search(project.rootFolder[0], path) } - if (project != null) { - startSearch(project) - } else { - ProjectGetter.getProject( - projectId, - { rootFolder: true, rootDoc_id: true }, - (err, project) => { - if (err != null) { - return callback(err) - } - if (project == null) { - return callback(new Errors.NotFoundError('project not found')) - } - startSearch(project) - } - ) + const project = + options.project || + (await ProjectGetter.promises.getProject(projectId, { + rootFolder: true, + rootDoc_id: true, + })) + + if (project == null) { + throw new Errors.NotFoundError('project not found') } + return startSearch(project) } -function findRootDoc(opts, callback) { - const getRootDoc = project => { - if (project.rootDoc_id != null) { - findElement( - { project, element_id: project.rootDoc_id, type: 'docs' }, - (error, ...args) => { - if (error != null) { - if (error instanceof Errors.NotFoundError) { - return callback(null, null) - } else { - return callback(error) - } - } - callback(null, ...args) - } - ) - } else { - callback(null, null) +async function findRootDoc(opts) { + const getRootDoc = async project => { + if (project.rootDoc_id == null) { + return { element: null, path: null, folder: null } + } + try { + return await findElement({ + project, + element_id: project.rootDoc_id, + type: 'docs', + }) + } catch (err) { + if (err instanceof Errors.NotFoundError) { + return { element: null, path: null, folder: null } + } + throw err } } - const { project, project_id: projectId } = opts - if (project != null) { - getRootDoc(project) - } else { - ProjectGetter.getProject( - projectId, - { rootFolder: true, rootDoc_id: true }, - (err, project) => { - if (err != null) { - logger.warn({ err }, 'error getting project') - callback(err) - } else { - getRootDoc(project) - } - } - ) - } + const { project_id: projectId } = opts + const project = + opts.project || + (await ProjectGetter.promises.getProject(projectId, { + rootFolder: true, + rootDoc_id: true, + })) + return await getRootDoc(project) } -function findElementByPath(options, callback) { - const { project, project_id: projectId, path, exactCaseMatch } = options +async function findElementByPath(options) { + const { project_id: projectId, path, exactCaseMatch } = options if (path == null) { - return new Error('no path provided for findElementByPath') - } - - if (project != null) { - _findElementByPathWithProject(project, path, exactCaseMatch, callback) - } else { - ProjectGetter.getProject( - projectId, - { rootFolder: true, rootDoc_id: true }, - (err, project) => { - if (err != null) { - return callback(err) - } - _findElementByPathWithProject(project, path, exactCaseMatch, callback) - } - ) + throw new Error('no path provided for findElementByPath') } + const project = + options.project || + (await ProjectGetter.promises.getProject(projectId, { + rootFolder: true, + rootDoc_id: true, + })) + return await _findElementByPathWithProject(project, path, exactCaseMatch) } -function _findElementByPathWithProject( +async function _findElementByPathWithProject( project, needlePath, - exactCaseMatch, - callback + exactCaseMatch ) { let matchFn if (exactCaseMatch) { @@ -200,9 +161,9 @@ function _findElementByPathWithProject( (b != null ? b.toLowerCase() : undefined) } - function getParentFolder(haystackFolder, foldersList, level, cb) { + function getParentFolder(haystackFolder, foldersList, level) { if (foldersList.length === 0) { - return cb(null, haystackFolder) + return haystackFolder } const needleFolderName = foldersList[level] let found = false @@ -210,25 +171,22 @@ function _findElementByPathWithProject( if (matchFn(folder.name, needleFolderName)) { found = true if (level === foldersList.length - 1) { - return cb(null, folder) - } else { - return getParentFolder(folder, foldersList, level + 1, cb) + return folder } + return getParentFolder(folder, foldersList, level + 1) } } if (!found) { - cb( - new Error( - `not found project: ${project._id} search path: ${needlePath}, folder ${foldersList[level]} could not be found` - ) + throw new Error( + `not found project: ${project._id} search path: ${needlePath}, folder ${foldersList[level]} could not be found` ) } } - function getEntity(folder, entityName, cb) { + function getEntity(folder, entityName) { let result, type if (entityName == null) { - return cb(null, folder, 'folder', null) + return { element: folder, type: 'folder', folder: null } } for (const file of iterablePaths(folder, 'fileRefs')) { if (matchFn(file != null ? file.name : undefined, entityName)) { @@ -252,21 +210,18 @@ function _findElementByPathWithProject( } if (result != null) { - cb(null, result, type, folder) - } else { - cb( - new Error( - `not found project: ${project._id} search path: ${needlePath}, entity ${entityName} could not be found` - ) - ) + return { element: result, type, folder } } + throw new Error( + `not found project: ${project._id} search path: ${needlePath}, entity ${entityName} could not be found` + ) } if (project == null) { - return callback(new Error('Tried to find an element for a null project')) + throw new Error('Tried to find an element for a null project') } if (needlePath === '' || needlePath === '/') { - return callback(null, project.rootFolder[0], 'folder', null) + return { element: project.rootFolder[0], type: 'folder', folder: null } } if (needlePath.indexOf('/') === 0) { @@ -275,11 +230,8 @@ function _findElementByPathWithProject( const foldersList = needlePath.split('/') const needleName = foldersList.pop() const rootFolder = project.rootFolder[0] - - const jobs = [] - jobs.push(cb => getParentFolder(rootFolder, foldersList, 0, cb)) - jobs.push((folder, cb) => getEntity(folder, needleName, cb)) - async.waterfall(jobs, callback) + const parentFolder = getParentFolder(rootFolder, foldersList, 0) + return getEntity(parentFolder, needleName) } function sanitizeTypeOfElement(elementType) { @@ -329,26 +281,26 @@ function findElementByMongoPath(project, mongoPath) { } export default { - findElement, - findElementByPath, - findRootDoc, + findElement: callbackifyMultiResult(findElement, [ + 'element', + 'path', + 'folder', + ]), + findElementByPath: callbackifyMultiResult(findElementByPath, [ + 'element', + 'type', + 'folder', + ]), + findRootDoc: callbackifyMultiResult(findRootDoc, [ + 'element', + 'path', + 'folder', + ]), findElementByMongoPath, findDeep, promises: { - findElement: promisifyMultiResult(findElement, [ - 'element', - 'path', - 'folder', - ]), - findElementByPath: promisifyMultiResult(findElementByPath, [ - 'element', - 'type', - 'folder', - ]), - findRootDoc: promisifyMultiResult(findRootDoc, [ - 'element', - 'path', - 'folder', - ]), + findElement, + findElementByPath, + findRootDoc, }, } diff --git a/services/web/test/unit/src/Project/ProjectLocator.test.mjs b/services/web/test/unit/src/Project/ProjectLocator.test.mjs index 60092b17a4..2f0a788112 100644 --- a/services/web/test/unit/src/Project/ProjectLocator.test.mjs +++ b/services/web/test/unit/src/Project/ProjectLocator.test.mjs @@ -1,6 +1,7 @@ import { vi, expect } from 'vitest' import sinon from 'sinon' import Errors from '../../../../app/src/Features/Errors/Errors.js' + const modulePath = '../../../../app/src/Features/Project/ProjectLocator' vi.mock('../../../../app/src/Features/Errors/Errors.js', () => @@ -43,7 +44,9 @@ project.rootDoc_id = rootDoc._id describe('ProjectLocator', function () { beforeEach(async function (ctx) { ctx.ProjectGetter = { - getProject: sinon.stub().callsArgWith(2, null, project), + promises: { + getProject: sinon.stub().resolves(project), + }, } ctx.ProjectHelper = { isArchived: sinon.stub(), @@ -481,7 +484,11 @@ describe('ProjectLocator', function () { describe('with a null project', function () { beforeEach(function (ctx) { - ctx.ProjectGetter = { getProject: sinon.stub().callsArg(2) } + ctx.ProjectGetter = { + promises: { + getProject: sinon.stub().resolves(null), + }, + } }) it('should not crash with a null', async function (ctx) { @@ -502,7 +509,7 @@ describe('ProjectLocator', function () { project_id: project._id, path, }) - ctx.ProjectGetter.getProject + ctx.ProjectGetter.promises.getProject .calledWith(project._id, { rootFolder: true, rootDoc_id: true }) .should.equal(true) element.should.deep.equal(doc1)