From 5a3318f5b344faa1460f2fafa5240955fe04cbc1 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 4 Jul 2022 11:12:01 +0100 Subject: [PATCH] Merge pull request #8701 from overleaf/bg-simple-iterable-paths simple iterable paths GitOrigin-RevId: f6906016888ccfc95c88858bdac4d2633fc1c5f4 --- .../app/src/Features/Project/IterablePath.js | 15 +++++++++++++++ .../Features/Project/ProjectEntityHandler.js | 15 ++++++++------- .../Project/ProjectEntityMongoUpdateHandler.js | 3 ++- .../Project/ProjectEntityUpdateHandler.js | 7 ++++--- .../app/src/Features/Project/ProjectLocator.js | 7 ++++--- .../web/scripts/delete_dangling_file_refs.js | 3 ++- .../test/unit/src/Project/IterablePathTests.js | 18 ++++++++++++++++++ 7 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 services/web/app/src/Features/Project/IterablePath.js create mode 100644 services/web/test/unit/src/Project/IterablePathTests.js diff --git a/services/web/app/src/Features/Project/IterablePath.js b/services/web/app/src/Features/Project/IterablePath.js new file mode 100644 index 0000000000..52cf068c42 --- /dev/null +++ b/services/web/app/src/Features/Project/IterablePath.js @@ -0,0 +1,15 @@ +/** + * Handles malformed filetrees - when fields such as `folder.docs`, + * `folder.folders` or `folder.fileRefs` are missing it returns an + * empty array. + */ +function iterablePaths(folder, field) { + if (!folder) { + return [] + } + return folder[field] || [] +} + +module.exports = { + iterablePaths, +} diff --git a/services/web/app/src/Features/Project/ProjectEntityHandler.js b/services/web/app/src/Features/Project/ProjectEntityHandler.js index a1ff3086b3..2d76e27d48 100644 --- a/services/web/app/src/Features/Project/ProjectEntityHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityHandler.js @@ -4,6 +4,7 @@ const Errors = require('../Errors/Errors') const ProjectGetter = require('./ProjectGetter') const { promisifyAll } = require('../../util/promises') const OError = require('@overleaf/o-error') +const { iterablePaths } = require('./IterablePath') const ProjectEntityHandler = { getAllDocs(projectId, callback) { @@ -26,7 +27,7 @@ const ProjectEntityHandler = { } const docs = {} for (const { path: folderPath, folder } of folders) { - for (const doc of folder.docs || []) { + for (const doc of iterablePaths(folder, 'docs')) { const content = docContents[doc._id.toString()] if (content != null) { docs[path.join(folderPath, doc.name)] = { @@ -51,7 +52,7 @@ const ProjectEntityHandler = { } const files = {} for (const { path: folderPath, folder } of folders) { - for (const file of folder.fileRefs || []) { + for (const file of iterablePaths(folder, 'fileRefs')) { if (file != null) { files[path.join(folderPath, file.name)] = file } @@ -80,12 +81,12 @@ const ProjectEntityHandler = { const docs = [] const files = [] for (const { path: folderPath, folder } of folders) { - for (const doc of folder.docs || []) { + for (const doc of iterablePaths(folder, 'docs')) { if (doc != null) { docs.push({ path: path.join(folderPath, doc.name), doc }) } } - for (const file of folder.fileRefs || []) { + for (const file of iterablePaths(folder, 'fileRefs')) { if (file != null) { files.push({ path: path.join(folderPath, file.name), file }) } @@ -111,7 +112,7 @@ const ProjectEntityHandler = { const folders = ProjectEntityHandler._getAllFoldersFromProject(project) const docPath = {} for (const { path: folderPath, folder } of folders) { - for (const doc of folder.docs || []) { + for (const doc of iterablePaths(folder, 'docs')) { docPath[doc._id] = path.join(folderPath, doc.name) } } @@ -171,7 +172,7 @@ const ProjectEntityHandler = { return path.join(basePath, docInCurrentFolder.name) } else { let docPath, childFolder - for (childFolder of folder.folders || []) { + for (childFolder of iterablePaths(folder, 'folders')) { docPath = recursivelyFindDocInFolder( path.join(basePath, childFolder.name), docId, @@ -211,7 +212,7 @@ const ProjectEntityHandler = { const processFolder = (basePath, folder) => { folders.push({ path: basePath, folder }) if (folder.folders) { - for (const childFolder of folder.folders) { + for (const childFolder of iterablePaths(folder, 'folders')) { if (childFolder.name != null) { const childPath = path.join(basePath, childFolder.name) processFolder(childPath, childFolder) diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index 89800c7b2c..99a21234da 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -17,6 +17,7 @@ const ProjectLocator = require('./ProjectLocator') const FolderStructureBuilder = require('./FolderStructureBuilder') const SafePath = require('./SafePath') const { DeletedFile } = require('../../models/DeletedFile') +const { iterablePaths } = require('./IterablePath') const LOCK_NAMESPACE = 'mongoTransaction' const ENTITY_TYPE_TO_MONGO_PATH_SEGMENT = { @@ -494,7 +495,7 @@ function _countElements(project) { let total = 0 if (folder.folders) { total += folder.folders.length - for (const subfolder of folder.folders) { + for (const subfolder of iterablePaths(folder, 'folders')) { total += countFolder(subfolder) } } diff --git a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js index 9f2889b3db..b9476fa92d 100644 --- a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js @@ -22,6 +22,7 @@ const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') const FileWriter = require('../../infrastructure/FileWriter') const EditorRealTimeController = require('../Editor/EditorRealTimeController') const { promisifyAll } = require('../../util/promises') +const { iterablePaths } = require('./IterablePath') const LOCK_NAMESPACE = 'sequentialProjectStructureUpdateLock' const VALID_ROOT_DOC_EXTENSIONS = Settings.validRootDocExtensions @@ -1608,16 +1609,16 @@ const ProjectEntityUpdateHandler = { } else if (entityType.indexOf('folder') !== -1) { changes = { oldDocs: [], oldFiles: [] } const _recurseFolder = (folder, folderPath) => { - for (const doc of folder.docs) { + for (const doc of iterablePaths(folder, 'docs')) { changes.oldDocs.push({ doc, path: Path.join(folderPath, doc.name) }) } - for (const file of folder.fileRefs) { + for (const file of iterablePaths(folder, 'fileRefs')) { changes.oldFiles.push({ file, path: Path.join(folderPath, file.name), }) } - for (const childFolder of folder.folders) { + for (const childFolder of iterablePaths(folder, 'folders')) { _recurseFolder(childFolder, Path.join(folderPath, childFolder.name)) } } diff --git a/services/web/app/src/Features/Project/ProjectLocator.js b/services/web/app/src/Features/Project/ProjectLocator.js index 7d672fab75..237ab79dc7 100644 --- a/services/web/app/src/Features/Project/ProjectLocator.js +++ b/services/web/app/src/Features/Project/ProjectLocator.js @@ -4,6 +4,7 @@ const async = require('async') const ProjectGetter = require('./ProjectGetter') const Errors = require('../Errors/Errors') const { promisifyMultiResult } = require('../../util/promises') +const { iterablePaths } = require('./IterablePath') function findElement(options, _callback) { // The search algorithm below potentially invokes the callback multiple @@ -203,19 +204,19 @@ function _findElementByPathWithProject( if (entityName == null) { return cb(null, folder, 'folder') } - for (const file of folder.fileRefs || []) { + for (const file of iterablePaths(folder, 'fileRefs')) { if (matchFn(file != null ? file.name : undefined, entityName)) { result = file type = 'file' } } - for (const doc of folder.docs || []) { + for (const doc of iterablePaths(folder, 'docs')) { if (matchFn(doc != null ? doc.name : undefined, entityName)) { result = doc type = 'doc' } } - for (const childFolder of folder.folders || []) { + for (const childFolder of iterablePaths(folder, 'folders')) { if ( matchFn(childFolder != null ? childFolder.name : undefined, entityName) ) { diff --git a/services/web/scripts/delete_dangling_file_refs.js b/services/web/scripts/delete_dangling_file_refs.js index 0c441a442a..521add216c 100644 --- a/services/web/scripts/delete_dangling_file_refs.js +++ b/services/web/scripts/delete_dangling_file_refs.js @@ -9,6 +9,7 @@ const { db, waitForDb } = require('../app/src/infrastructure/mongodb') const Errors = require('../app/src/Features/Errors/Errors') const FileStoreHandler = require('../app/src/Features/FileStore/FileStoreHandler') const ProjectEntityMongoUpdateHandler = require('../app/src/Features/Project/ProjectEntityMongoUpdateHandler') +const { iterablePaths } = require('../app/src/Features/Project/IterablePath') const OPTIONS = parseArgs() @@ -70,7 +71,7 @@ async function processProject(project) { function findRefsInFolder(folder) { let docIds = folder.docs.map(doc => doc._id) let fileIds = folder.fileRefs.map(file => file._id) - for (const subfolder of folder.folders) { + for (const subfolder of iterablePaths(folder, 'folders')) { const subrefs = findRefsInFolder(subfolder) docIds = docIds.concat(subrefs.docIds) fileIds = fileIds.concat(subrefs.fileIds) diff --git a/services/web/test/unit/src/Project/IterablePathTests.js b/services/web/test/unit/src/Project/IterablePathTests.js new file mode 100644 index 0000000000..e00607eb69 --- /dev/null +++ b/services/web/test/unit/src/Project/IterablePathTests.js @@ -0,0 +1,18 @@ +const { expect } = require('chai') +const { + iterablePaths, +} = require('../../../../app/src/Features/Project/IterablePath') + +describe('iterablePaths', function () { + it('returns an empty array for empty folders', function () { + expect(iterablePaths(null, 'docs')).to.deep.equal([]) + expect(iterablePaths({}, 'docs')).to.deep.equal([]) + }) + + it('returns the `docs` object when it is iterable', function () { + const folder = { + docs: [{ _id: 1 }, { _id: 2 }], + } + expect(iterablePaths(folder, 'docs')).to.equal(folder.docs) + }) +})