diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index f747ef792c..39b3e090e0 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -1,22 +1,3 @@ -/* eslint-disable - camelcase, - handle-callback-err, - max-len, - one-var, - standard/no-callback-literal, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS103: Rewrite code to no longer use __guard__ - * DS201: Simplify complex destructure assignments - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let ProjectEntityMongoUpdateHandler, self const _ = require('underscore') const async = require('async') const logger = require('logger-sharelatex') @@ -34,19 +15,19 @@ const SafePath = require('./SafePath') const LOCK_NAMESPACE = 'mongoTransaction' -const wrapWithLock = function(methodWithoutLock) { +function wrapWithLock(methodWithoutLock) { // This lock is used whenever we read or write to an existing project's // structure. Some operations to project structure cannot be done atomically // in mongo, this lock is used to prevent reading the structure between two // parts of a staged update. - const methodWithLock = function(project_id, ...rest) { - const adjustedLength = Math.max(rest.length, 1), - args = rest.slice(0, adjustedLength - 1), - callback = rest[adjustedLength - 1] - return LockManager.runWithLock( + function methodWithLock(projectId, ...rest) { + const adjustedLength = Math.max(rest.length, 1) + const args = rest.slice(0, adjustedLength - 1) + const callback = rest[adjustedLength - 1] + LockManager.runWithLock( LOCK_NAMESPACE, - project_id, - cb => methodWithoutLock(project_id, ...Array.from(args), cb), + projectId, + cb => methodWithoutLock(projectId, ...args, cb), callback ) } @@ -54,233 +35,245 @@ const wrapWithLock = function(methodWithoutLock) { return methodWithLock } -module.exports = ProjectEntityMongoUpdateHandler = self = { +const ProjectEntityMongoUpdateHandler = { LOCK_NAMESPACE, - addDoc: wrapWithLock(function(project_id, folder_id, doc, callback) { - if (callback == null) { - callback = function(err, result) {} - } - return ProjectGetter.getProjectWithoutLock( - project_id, + addDoc: wrapWithLock(function(projectId, folderId, doc, callback) { + ProjectGetter.getProjectWithoutLock( + projectId, { rootFolder: true, name: true, overleaf: true }, - function(err, project) { + (err, project) => { if (err != null) { - logger.warn({ project_id, err }, 'error getting project for add doc') + logger.warn({ projectId, err }, 'error getting project for add doc') return callback(err) } logger.log( - { project_id, folder_id, doc_name: doc.name }, + { projectId, folderId, doc_name: doc.name }, 'adding doc to project with project' ) - return self._confirmFolder(project, folder_id, folder_id => { - return self._putElement(project, folder_id, doc, 'doc', callback) - }) + ProjectEntityMongoUpdateHandler._confirmFolder( + project, + folderId, + folderId => { + ProjectEntityMongoUpdateHandler._putElement( + project, + folderId, + doc, + 'doc', + callback + ) + } + ) } ) }), - addFile: wrapWithLock(function(project_id, folder_id, fileRef, callback) { - if (callback == null) { - callback = function(error, result, project) {} - } - return ProjectGetter.getProjectWithoutLock( - project_id, + addFile: wrapWithLock(function(projectId, folderId, fileRef, callback) { + ProjectGetter.getProjectWithoutLock( + projectId, { rootFolder: true, name: true, overleaf: true }, - function(err, project) { + (err, project) => { if (err != null) { - logger.warn({ project_id, err }, 'error getting project for add file') + logger.warn({ projectId, err }, 'error getting project for add file') return callback(err) } logger.log( - { project_id: project._id, folder_id, file_name: fileRef.name }, + { projectId: project._id, folderId, file_name: fileRef.name }, 'adding file' ) - return self._confirmFolder(project, folder_id, folder_id => - self._putElement(project, folder_id, fileRef, 'file', callback) + ProjectEntityMongoUpdateHandler._confirmFolder( + project, + folderId, + folderId => + ProjectEntityMongoUpdateHandler._putElement( + project, + folderId, + fileRef, + 'file', + callback + ) ) } ) }), - replaceFileWithNew: wrapWithLock( - (project_id, file_id, newFileRef, callback) => - ProjectGetter.getProjectWithoutLock( - project_id, - { rootFolder: true, name: true, overleaf: true }, - function(err, project) { - if (err != null) { - return callback(err) - } - return ProjectLocator.findElement( - { project, element_id: file_id, type: 'file' }, - (err, fileRef, path) => { - if (err != null) { - return callback(err) - } - return ProjectEntityMongoUpdateHandler._insertDeletedFileReference( - project_id, - fileRef, - function(err) { - if (err != null) { - return callback(err) - } - const conditions = { _id: project._id } - const inc = {} - // increment the project structure version as we are adding a new file here - inc['version'] = 1 - const set = {} - set[`${path.mongo}._id`] = newFileRef._id - set[`${path.mongo}.created`] = new Date() - set[`${path.mongo}.linkedFileData`] = - newFileRef.linkedFileData - inc[`${path.mongo}.rev`] = 1 - set[`${path.mongo}.hash`] = newFileRef.hash - const update = { - $inc: inc, - $set: set - } - // Note: Mongoose uses new:true to return the modified document - // https://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate - // but Mongo uses returnNewDocument:true instead - // https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/ - // We are using Mongoose here, but if we ever switch to a direct mongo call - // the next line will need to be updated. - return Project.findOneAndUpdate( - conditions, - update, - { new: true }, - function(err, newProject) { - if (err != null) { - return callback(err) - } - return callback(null, fileRef, project, path, newProject) - } - ) - } - ) - } - ) + replaceFileWithNew: wrapWithLock((projectId, fileId, newFileRef, callback) => + ProjectGetter.getProjectWithoutLock( + projectId, + { rootFolder: true, name: true, overleaf: true }, + (err, project) => { + if (err != null) { + return callback(err) } - ) + ProjectLocator.findElement( + { project, element_id: fileId, type: 'file' }, + (err, fileRef, path) => { + if (err != null) { + return callback(err) + } + ProjectEntityMongoUpdateHandler._insertDeletedFileReference( + projectId, + fileRef, + err => { + if (err != null) { + return callback(err) + } + const conditions = { _id: project._id } + const inc = {} + // increment the project structure version as we are adding a new file here + inc['version'] = 1 + const set = {} + set[`${path.mongo}._id`] = newFileRef._id + set[`${path.mongo}.created`] = new Date() + set[`${path.mongo}.linkedFileData`] = newFileRef.linkedFileData + inc[`${path.mongo}.rev`] = 1 + set[`${path.mongo}.hash`] = newFileRef.hash + const update = { + $inc: inc, + $set: set + } + // Note: Mongoose uses new:true to return the modified document + // https://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate + // but Mongo uses returnNewDocument:true instead + // https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/ + // We are using Mongoose here, but if we ever switch to a direct mongo call + // the next line will need to be updated. + Project.findOneAndUpdate( + conditions, + update, + { new: true }, + (err, newProject) => { + if (err != null) { + return callback(err) + } + callback(null, fileRef, project, path, newProject) + } + ) + } + ) + } + ) + } + ) ), - mkdirp: wrapWithLock(function(project_id, path, options, callback) { + mkdirp: wrapWithLock(function(projectId, path, options, callback) { // defaults to case insensitive paths, use options {exactCaseMatch:true} // to make matching case-sensitive let folders = path.split('/') folders = _.select(folders, folder => folder.length !== 0) - return ProjectGetter.getProjectWithOnlyFolders( - project_id, - (err, project) => { - if (path === '/') { - logger.log( - { project_id: project._id }, - 'mkdir is only trying to make path of / so sending back root folder' - ) - return callback(null, [], project.rootFolder[0]) - } - logger.log({ project_id: project._id, path, folders }, 'running mkdirp') - - let builtUpPath = '' - const procesFolder = (previousFolders, folderName, callback) => { - let parentFolder_id - previousFolders = previousFolders || [] - const parentFolder = previousFolders[previousFolders.length - 1] - if (parentFolder != null) { - parentFolder_id = parentFolder._id - } - builtUpPath = `${builtUpPath}/${folderName}` - return ProjectLocator.findElementByPath( - { - project, - path: builtUpPath, - exactCaseMatch: - options != null ? options.exactCaseMatch : undefined - }, - (err, foundFolder) => { - if (foundFolder == null) { - logger.log( - { path, project_id: project._id, folderName }, - 'making folder from mkdirp' - ) - return self.addFolder.withoutLock( - project_id, - parentFolder_id, - folderName, - function(err, newFolder, parentFolder_id) { - if (err != null) { - return callback(err) - } - newFolder.parentFolder_id = parentFolder_id - previousFolders.push(newFolder) - return callback(null, previousFolders) - } - ) - } else { - foundFolder.filterOut = true - previousFolders.push(foundFolder) - return callback(null, previousFolders) - } - } - ) - } - - return async.reduce(folders, [], procesFolder, function(err, folders) { - if (err != null) { - return callback(err) - } - const lastFolder = folders[folders.length - 1] - folders = _.select(folders, folder => !folder.filterOut) - return callback(null, folders, lastFolder) - }) + ProjectGetter.getProjectWithOnlyFolders(projectId, (err, project) => { + if (err != null) { + return callback(err) } - ) + if (path === '/') { + logger.log( + { projectId: project._id }, + 'mkdir is only trying to make path of / so sending back root folder' + ) + return callback(null, [], project.rootFolder[0]) + } + logger.log({ projectId: project._id, path, folders }, 'running mkdirp') + + let builtUpPath = '' + const procesFolder = (previousFolders, folderName, callback) => { + let parentFolderId + previousFolders = previousFolders || [] + const parentFolder = previousFolders[previousFolders.length - 1] + if (parentFolder != null) { + parentFolderId = parentFolder._id + } + builtUpPath = `${builtUpPath}/${folderName}` + ProjectLocator.findElementByPath( + { + project, + path: builtUpPath, + exactCaseMatch: options != null ? options.exactCaseMatch : undefined + }, + (err, foundFolder) => { + if (err != null) { + logger.log( + { path, projectId: project._id, folderName }, + 'making folder from mkdirp' + ) + ProjectEntityMongoUpdateHandler.addFolder.withoutLock( + projectId, + parentFolderId, + folderName, + (err, newFolder, parentFolderId) => { + if (err != null) { + return callback(err) + } + newFolder.parentFolder_id = parentFolderId + previousFolders.push(newFolder) + callback(null, previousFolders) + } + ) + } else { + foundFolder.filterOut = true + previousFolders.push(foundFolder) + callback(null, previousFolders) + } + } + ) + } + + async.reduce(folders, [], procesFolder, (err, folders) => { + if (err != null) { + return callback(err) + } + const lastFolder = folders[folders.length - 1] + folders = _.select(folders, folder => !folder.filterOut) + callback(null, folders, lastFolder) + }) + }) }), moveEntity: wrapWithLock(function( - project_id, - entity_id, + projectId, + entityId, destFolderId, entityType, callback ) { - if (callback == null) { - callback = function(error) {} - } - return ProjectGetter.getProjectWithoutLock( - project_id, + ProjectGetter.getProjectWithoutLock( + projectId, { rootFolder: true, name: true, overleaf: true }, - function(err, project) { + (err, project) => { if (err != null) { return callback(err) } - return ProjectLocator.findElement( - { project, element_id: entity_id, type: entityType }, - function(err, entity, entityPath) { + ProjectLocator.findElement( + { project, element_id: entityId, type: entityType }, + (err, entity, entityPath) => { if (err != null) { return callback(err) } // Prevent top-level docs/files with reserved names (to match v1 behaviour) - if (self._blockedFilename(entityPath, entityType)) { + if ( + ProjectEntityMongoUpdateHandler._blockedFilename( + entityPath, + entityType + ) + ) { return callback( new Errors.InvalidNameError('blocked element name') ) } - return self._checkValidMove( + ProjectEntityMongoUpdateHandler._checkValidMove( project, entityType, entity, entityPath, destFolderId, - function(error) { + error => { if (error != null) { return callback(error) } - return ProjectEntityHandler.getAllEntitiesFromProject( + ProjectEntityHandler.getAllEntitiesFromProject( project, - function(error, oldDocs, oldFiles) { + (error, oldDocs, oldFiles) => { if (error != null) { return callback(error) } @@ -290,12 +283,12 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { // will cause some breakage but is better than being // lost, which is what happens if this is done in the // opposite order. - return self._putElement( + ProjectEntityMongoUpdateHandler._putElement( project, destFolderId, entity, entityType, - function(err, result) { + (err, result) => { if (err != null) { return callback(err) } @@ -307,18 +300,18 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { // have not moved a folder subfolder of itself (which // is done by _checkValidMove above) because that // would lead to it being deleted. - return self._removeElementFromMongoArray( + ProjectEntityMongoUpdateHandler._removeElementFromMongoArray( Project, - project_id, + projectId, entityPath.mongo, - entity_id, - function(err, newProject) { + entityId, + (err, newProject) => { if (err != null) { return callback(err) } - return ProjectEntityHandler.getAllEntitiesFromProject( + ProjectEntityHandler.getAllEntitiesFromProject( newProject, - function(err, newDocs, newFiles) { + (err, newDocs, newFiles) => { if (err != null) { return callback(err) } @@ -338,7 +331,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) { logger.warn( { - project_id, + projectId, oldDocs: oldDocs.length, newDocs: newDocs.length, oldFiles: oldFiles.length, @@ -354,7 +347,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) ) } - return callback( + callback( null, project, startPath, @@ -379,30 +372,30 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) }), - deleteEntity: wrapWithLock((project_id, entity_id, entityType, callback) => + deleteEntity: wrapWithLock((projectId, entityId, entityType, callback) => ProjectGetter.getProjectWithoutLock( - project_id, + projectId, { name: true, rootFolder: true, overleaf: true }, - function(error, project) { + (error, project) => { if (error != null) { return callback(error) } - return ProjectLocator.findElement( - { project, element_id: entity_id, type: entityType }, - function(error, entity, path) { + ProjectLocator.findElement( + { project, element_id: entityId, type: entityType }, + (error, entity, path) => { if (error != null) { return callback(error) } - return self._removeElementFromMongoArray( + ProjectEntityMongoUpdateHandler._removeElementFromMongoArray( Project, - project_id, + projectId, path.mongo, - entity_id, - function(error, newProject) { + entityId, + (error, newProject) => { if (error != null) { return callback(error) } - return callback(null, entity, path, project, newProject) + callback(null, entity, path, project, newProject) } ) } @@ -412,22 +405,22 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ), renameEntity: wrapWithLock( - (project_id, entity_id, entityType, newName, callback) => + (projectId, entityId, entityType, newName, callback) => ProjectGetter.getProjectWithoutLock( - project_id, + projectId, { rootFolder: true, name: true, overleaf: true }, (error, project) => { if (error != null) { return callback(error) } - return ProjectEntityHandler.getAllEntitiesFromProject( + ProjectEntityHandler.getAllEntitiesFromProject( project, (error, oldDocs, oldFiles) => { if (error != null) { return callback(error) } - return ProjectLocator.findElement( - { project, element_id: entity_id, type: entityType }, + ProjectLocator.findElement( + { project, element_id: entityId, type: entityType }, (error, entity, entPath, parentFolder) => { if (error != null) { return callback(error) @@ -438,35 +431,38 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) // Prevent top-level docs/files with reserved names (to match v1 behaviour) if ( - self._blockedFilename({ fileSystem: endPath }, entityType) + ProjectEntityMongoUpdateHandler._blockedFilename( + { fileSystem: endPath }, + entityType + ) ) { return callback( new Errors.InvalidNameError('blocked element name') ) } // check if the new name already exists in the current folder - return self._checkValidElementName( + ProjectEntityMongoUpdateHandler._checkValidElementName( parentFolder, newName, error => { if (error != null) { return callback(error) } - const conditions = { _id: project_id } + const conditions = { _id: projectId } const update = { $set: {}, $inc: {} } const namePath = entPath.mongo + '.name' update['$set'][namePath] = newName // we need to increment the project version number for any structure change update['$inc']['version'] = 1 - return Project.findOneAndUpdate( + Project.findOneAndUpdate( conditions, update, { new: true }, - function(error, newProject) { + (error, newProject) => { if (error != null) { return callback(error) } - return ProjectEntityHandler.getAllEntitiesFromProject( + ProjectEntityHandler.getAllEntitiesFromProject( newProject, (error, newDocs, newFiles) => { if (error != null) { @@ -480,7 +476,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { newFiles, newProject } - return callback( + callback( null, project, startPath, @@ -503,41 +499,41 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) ), - addFolder: wrapWithLock((project_id, parentFolder_id, folderName, callback) => + addFolder: wrapWithLock((projectId, parentFolderId, folderName, callback) => ProjectGetter.getProjectWithoutLock( - project_id, + projectId, { rootFolder: true, name: true, overleaf: true }, - function(err, project) { + (err, project) => { if (err != null) { logger.warn( - { project_id, err }, + { projectId, err }, 'error getting project for add folder' ) return callback(err) } - return self._confirmFolder( + ProjectEntityMongoUpdateHandler._confirmFolder( project, - parentFolder_id, - parentFolder_id => { + parentFolderId, + parentFolderId => { const folder = new Folder({ name: folderName }) logger.log( - { project: project._id, parentFolder_id, folderName }, + { project: project._id, parentFolderId, folderName }, 'adding new folder' ) - return self._putElement( + ProjectEntityMongoUpdateHandler._putElement( project, - parentFolder_id, + parentFolderId, folder, 'folder', err => { if (err != null) { logger.warn( - { err, project_id: project._id }, + { err, projectId: project._id }, 'error adding folder to project' ) return callback(err) } - return callback(null, folder, parentFolder_id) + callback(null, folder, parentFolderId) } ) } @@ -546,54 +542,42 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) ), - _removeElementFromMongoArray(model, model_id, path, element_id, callback) { - if (callback == null) { - callback = function(err, project) {} - } - const conditions = { _id: model_id } + _removeElementFromMongoArray(model, modelId, path, elementId, callback) { + const conditions = { _id: modelId } const pullUpdate = { $pull: {}, $inc: {} } const nonArrayPath = path.slice(0, path.lastIndexOf('.')) // remove specific element from array by id - pullUpdate['$pull'][nonArrayPath] = { _id: element_id } + pullUpdate['$pull'][nonArrayPath] = { _id: elementId } // we need to increment the project version number for any structure change pullUpdate['$inc']['version'] = 1 - return model.findOneAndUpdate( - conditions, - pullUpdate, - { new: true }, - callback - ) + model.findOneAndUpdate(conditions, pullUpdate, { new: true }, callback) }, _countElements(project) { - var countFolder = function(folder) { + function countFolder(folder) { let total = 0 - for (let subfolder of Array.from( - (folder != null ? folder.folders : undefined) || [] - )) { + for (let subfolder of (folder != null ? folder.folders : undefined) || + []) { total += countFolder(subfolder) } if ( - __guard__(folder != null ? folder.folders : undefined, x => x.length) != - null + folder != null && + folder.folders != null && + folder.folders.length > 0 ) { total += folder.folders.length } - if ( - __guard__(folder != null ? folder.docs : undefined, x1 => x1.length) != - null - ) { + if (folder != null && folder.docs != null && folder.docs.length > 0) { total += folder.docs.length } if ( - __guard__( - folder != null ? folder.fileRefs : undefined, - x2 => x2.length - ) != null + folder != null && + folder.fileRefs != null && + folder.fileRefs.length > 0 ) { total += folder.fileRefs.length } @@ -604,12 +588,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { return countFolder(project.rootFolder[0]) }, - _putElement(project, folder_id, element, type, callback) { - let e - if (callback == null) { - callback = function(err, path, project) {} - } - const sanitizeTypeOfElement = function(elementType) { + _putElement(project, folderId, element, type, callback) { + function sanitizeTypeOfElement(elementType) { const lastChar = elementType.slice(-1) if (lastChar !== 's') { elementType += 's' @@ -621,45 +601,46 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { } if (element == null || element._id == null) { - e = new Error('no element passed to be inserted') logger.warn( - { project_id: project._id, folder_id, element, type }, + { projectId: project._id, folderId, element, type }, 'failed trying to insert element as it was null' ) - return callback(e) + return callback(new Error('no element passed to be inserted')) } type = sanitizeTypeOfElement(type) // original check path.resolve("/", element.name) isnt "/#{element.name}" or element.name.match("/") // check if name is allowed if (!SafePath.isCleanFilename(element.name)) { - e = new Errors.InvalidNameError('invalid element name') logger.warn( - { project_id: project._id, folder_id, element, type }, + { projectId: project._id, folderId, element, type }, 'failed trying to insert element as name was invalid' ) - return callback(e) + return callback(new Errors.InvalidNameError('invalid element name')) } - if (folder_id == null) { - folder_id = project.rootFolder[0]._id + if (folderId == null) { + folderId = project.rootFolder[0]._id } - if (self._countElements(project) > settings.maxEntitiesPerProject) { + if ( + ProjectEntityMongoUpdateHandler._countElements(project) > + settings.maxEntitiesPerProject + ) { logger.warn( - { project_id: project._id }, + { projectId: project._id }, 'project too big, stopping insertions' ) CooldownManager.putProjectOnCooldown(project._id) return callback(new Error('project_has_to_many_files')) } - return ProjectLocator.findElement( - { project, element_id: folder_id, type: 'folders' }, + ProjectLocator.findElement( + { project, element_id: folderId, type: 'folders' }, (err, folder, path) => { if (err != null) { logger.warn( - { err, project_id: project._id, folder_id, type, element }, + { err, projectId: project._id, folderId, type, element }, 'error finding folder for _putElement' ) return callback(err) @@ -673,49 +654,53 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { return callback(new Errors.InvalidNameError('path too long')) } // Prevent top-level docs/files with reserved names (to match v1 behaviour) - if (self._blockedFilename(newPath, type)) { + if (ProjectEntityMongoUpdateHandler._blockedFilename(newPath, type)) { return callback(new Errors.InvalidNameError('blocked element name')) } - return self._checkValidElementName(folder, element.name, err => { - if (err != null) { - return callback(err) - } - const id = element._id + '' - element._id = require('mongoose').Types.ObjectId(id) - const conditions = { _id: project._id } - const mongopath = `${path.mongo}.${type}` - const update = { $push: {}, $inc: {} } - update['$push'][mongopath] = element - // we need to increment the project version number for any structure change - update['$inc']['version'] = 1 // increment project version number - logger.log( - { - project_id: project._id, - element_id: element._id, - fileType: type, - folder_id, - mongopath - }, - 'adding element to project' - ) - // We are using Mongoose here, but if we ever switch to a direct mongo call - // the next line will need to be updated to {returnNewDocument:true} - return Project.findOneAndUpdate( - conditions, - update, - { new: true }, - function(err, newProject) { - if (err != null) { - logger.warn( - { err, project_id: project._id }, - 'error saving in putElement project' - ) - return callback(err) - } - return callback(err, { path: newPath }, newProject) + ProjectEntityMongoUpdateHandler._checkValidElementName( + folder, + element.name, + err => { + if (err != null) { + return callback(err) } - ) - }) + const id = element._id + '' + element._id = require('mongoose').Types.ObjectId(id) + const conditions = { _id: project._id } + const mongopath = `${path.mongo}.${type}` + const update = { $push: {}, $inc: {} } + update['$push'][mongopath] = element + // we need to increment the project version number for any structure change + update['$inc']['version'] = 1 // increment project version number + logger.log( + { + projectId: project._id, + element_id: element._id, + fileType: type, + folderId, + mongopath + }, + 'adding element to project' + ) + // We are using Mongoose here, but if we ever switch to a direct mongo call + // the next line will need to be updated to {returnNewDocument:true} + Project.findOneAndUpdate( + conditions, + update, + { new: true }, + (err, newProject) => { + if (err != null) { + logger.warn( + { err, projectId: project._id }, + 'error saving in putElement project' + ) + return callback(err) + } + callback(err, { path: newPath }, newProject) + } + ) + } + ) } ) }, @@ -725,10 +710,10 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { // javascript reserved names are forbidden for docs and files // at the top-level (but folders with reserved names are allowed). const isFolder = ['folder', 'folders'].includes(entityType) - const [dir, file] = Array.from([ + const [dir, file] = [ path.dirname(entityPath.fileSystem), path.basename(entityPath.fileSystem) - ]) + ] const isTopLevel = dir === '/' if (isTopLevel && !isFolder && SafePath.isBlockedFilename(file)) { return true @@ -740,45 +725,36 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { _checkValidElementName(folder, name, callback) { // check if the name is already taken by a doc, file or // folder. If so, return an error "file already exists". - if (callback == null) { - callback = function(err) {} - } const err = new Errors.InvalidNameError('file already exists') - for (let doc of Array.from( - (folder != null ? folder.docs : undefined) || [] - )) { + for (let doc of (folder != null ? folder.docs : undefined) || []) { if (doc.name === name) { return callback(err) } } - for (let file of Array.from( - (folder != null ? folder.fileRefs : undefined) || [] - )) { + for (let file of (folder != null ? folder.fileRefs : undefined) || []) { if (file.name === name) { return callback(err) } } - for (folder of Array.from( - (folder != null ? folder.folders : undefined) || [] - )) { + for (folder of (folder != null ? folder.folders : undefined) || []) { if (folder.name === name) { return callback(err) } } - return callback() + callback() }, - _confirmFolder(project, folder_id, callback) { + _confirmFolder(project, folderId, callback) { logger.log( - { folder_id, project_id: project._id }, + { folderId, projectId: project._id }, 'confirming folder in project' ) - if (folder_id + '' === 'undefined') { - return callback(project.rootFolder[0]._id) - } else if (folder_id !== null) { - return callback(folder_id) + if (folderId + '' === 'undefined') { + callback(project.rootFolder[0]._id) + } else if (folderId !== null) { + callback(folderId) } else { - return callback(project.rootFolder[0]._id) + callback(project.rootFolder[0]._id) } }, @@ -790,57 +766,53 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { destFolderId, callback ) { - if (callback == null) { - callback = function(error) {} - } - return ProjectLocator.findElement( + ProjectLocator.findElement( { project, element_id: destFolderId, type: 'folder' }, - function(err, destEntity, destFolderPath) { + (err, destEntity, destFolderPath) => { if (err != null) { return callback(err) } // check if there is already a doc/file/folder with the same name // in the destination folder - return self._checkValidElementName(destEntity, entity.name, function( - err - ) { - if (err != null) { - return callback(err) - } - if (/folder/.test(entityType)) { - logger.log( - { - destFolderPath: destFolderPath.fileSystem, - folderPath: entityPath.fileSystem - }, - 'checking folder is not moving into child folder' - ) - const isNestedFolder = - destFolderPath.fileSystem.slice( - 0, - entityPath.fileSystem.length - ) === entityPath.fileSystem - if (isNestedFolder) { - return callback( - new Errors.InvalidNameError( - 'destination folder is a child folder of me' - ) - ) + ProjectEntityMongoUpdateHandler._checkValidElementName( + destEntity, + entity.name, + err => { + if (err != null) { + return callback(err) } + if (/folder/.test(entityType)) { + logger.log( + { + destFolderPath: destFolderPath.fileSystem, + folderPath: entityPath.fileSystem + }, + 'checking folder is not moving into child folder' + ) + const isNestedFolder = + destFolderPath.fileSystem.slice( + 0, + entityPath.fileSystem.length + ) === entityPath.fileSystem + if (isNestedFolder) { + return callback( + new Errors.InvalidNameError( + 'destination folder is a child folder of me' + ) + ) + } + } + callback() } - return callback() - }) + ) } ) }, - _insertDeletedDocReference(project_id, doc, callback) { - if (callback == null) { - callback = function(error) {} - } - return Project.update( + _insertDeletedDocReference(projectId, doc, callback) { + Project.update( { - _id: project_id + _id: projectId }, { $push: { @@ -856,13 +828,10 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { ) }, - _insertDeletedFileReference(project_id, fileRef, callback) { - if (callback == null) { - callback = function(error) {} - } - return Project.update( + _insertDeletedFileReference(projectId, fileRef, callback) { + Project.update( { - _id: project_id + _id: projectId }, { $push: { @@ -881,8 +850,4 @@ module.exports = ProjectEntityMongoUpdateHandler = self = { } } -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} +module.exports = ProjectEntityMongoUpdateHandler diff --git a/services/web/app/src/Features/Project/ProjectLocator.js b/services/web/app/src/Features/Project/ProjectLocator.js index bf5694bd25..c71f695514 100644 --- a/services/web/app/src/Features/Project/ProjectLocator.js +++ b/services/web/app/src/Features/Project/ProjectLocator.js @@ -1,25 +1,3 @@ -/* eslint-disable - camelcase, - handle-callback-err, - max-len, - no-array-constructor, - no-return-assign, - no-undef, - no-unused-vars, - standard/no-callback-literal, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS203: Remove `|| {}` from converted for-own loops - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let ProjectLocator -const { Project } = require('../../models/Project') const ProjectGetter = require('./ProjectGetter') const ProjectHelper = require('./ProjectHelper') const Errors = require('../Errors/Errors') @@ -27,92 +5,93 @@ const _ = require('underscore') const logger = require('logger-sharelatex') const async = require('async') -module.exports = ProjectLocator = { +const ProjectLocator = { findElement(options, _callback) { - if (_callback == null) { - _callback = function(err, element, path, parentFolder) {} - } - const callback = function(...args) { - _callback(...Array.from(args || [])) - return (_callback = function() {}) - } + // The search algorithm below potentially invokes the callback multiple + // times. + const callback = _.once(_callback) - const { project, project_id, element_id, type } = options + const { + project, + project_id: projectId, + element_id: elementId, + type + } = options const elementType = sanitizeTypeOfElement(type) let count = 0 const endOfBranch = function() { if (--count === 0) { logger.warn( - `element ${element_id} could not be found for project ${project_id || + `element ${elementId} could not be found for project ${projectId || project._id}` ) - return callback(new Errors.NotFoundError('entity not found')) + callback(new Errors.NotFoundError('entity not found')) } } - var search = function(searchFolder, path) { + function search(searchFolder, path) { count++ const element = _.find( searchFolder[elementType], - el => (el != null ? el._id : undefined) + '' === element_id + '' + el => (el != null ? el._id : undefined) + '' === elementId + '' ) // need to ToString both id's for robustness if ( element == null && searchFolder.folders != null && searchFolder.folders.length !== 0 ) { - _.each(searchFolder.folders, function(folder, index) { + _.each(searchFolder.folders, (folder, index) => { if (folder == null) { return } const newPath = {} - for (let key of Object.keys(path || {})) { + for (let key of Object.keys(path)) { const value = path[key] newPath[key] = value } // make a value copy of the string newPath.fileSystem += `/${folder.name}` newPath.mongo += `.folders.${index}` - return search(folder, newPath) + search(folder, newPath) }) endOfBranch() } else if (element != null) { const elementPlaceInArray = getIndexOf( searchFolder[elementType], - element_id + elementId ) path.fileSystem += `/${element.name}` path.mongo += `.${elementType}.${elementPlaceInArray}` - return callback(null, element, path, searchFolder) + callback(null, element, path, searchFolder) } else if (element == null) { - return endOfBranch() + endOfBranch() } } const path = { fileSystem: '', mongo: 'rootFolder.0' } - const startSearch = function(project) { - if (element_id + '' === project.rootFolder[0]._id + '') { - return callback(null, project.rootFolder[0], path, null) + const startSearch = project => { + if (elementId + '' === project.rootFolder[0]._id + '') { + callback(null, project.rootFolder[0], path, null) } else { - return search(project.rootFolder[0], path) + search(project.rootFolder[0], path) } } if (project != null) { - return startSearch(project) + startSearch(project) } else { - return ProjectGetter.getProject( - project_id, + ProjectGetter.getProject( + projectId, { rootFolder: true, rootDoc_id: true }, - function(err, project) { + (err, project) => { if (err != null) { return callback(err) } if (project == null) { return callback(new Errors.NotFoundError('project not found')) } - return startSearch(project) + startSearch(project) } ) } @@ -121,9 +100,9 @@ module.exports = ProjectLocator = { findRootDoc(opts, callback) { const getRootDoc = project => { if (project.rootDoc_id != null) { - return this.findElement( + this.findElement( { project, element_id: project.rootDoc_id, type: 'docs' }, - function(error, ...args) { + (error, ...args) => { if (error != null) { if (error instanceof Errors.NotFoundError) { return callback(null, null) @@ -131,26 +110,26 @@ module.exports = ProjectLocator = { return callback(error) } } - return callback(null, ...Array.from(args)) + callback(null, ...args) } ) } else { - return callback(null, null) + callback(null, null) } } - const { project, project_id } = opts + const { project, project_id: projectId } = opts if (project != null) { - return getRootDoc(project) + getRootDoc(project) } else { - return ProjectGetter.getProject( - project_id, + ProjectGetter.getProject( + projectId, { rootFolder: true, rootDoc_id: true }, - function(err, project) { + (err, project) => { if (err != null) { logger.warn({ err }, 'error getting project') - return callback(err) + callback(err) } else { - return getRootDoc(project) + getRootDoc(project) } } ) @@ -158,30 +137,27 @@ module.exports = ProjectLocator = { }, findElementByPath(options, callback) { - if (callback == null) { - callback = function(err, foundEntity, type) {} - } - const { project, project_id, path, exactCaseMatch } = options + const { project, project_id: projectId, path, exactCaseMatch } = options if (path == null) { return new Error('no path provided for findElementByPath') } if (project != null) { - return ProjectLocator._findElementByPathWithProject( + ProjectLocator._findElementByPathWithProject( project, path, exactCaseMatch, callback ) } else { - return ProjectGetter.getProject( - project_id, + ProjectGetter.getProject( + projectId, { rootFolder: true, rootDoc_id: true }, - function(err, project) { + (err, project) => { if (err != null) { return callback(err) } - return ProjectLocator._findElementByPathWithProject( + ProjectLocator._findElementByPathWithProject( project, path, exactCaseMatch, @@ -194,9 +170,6 @@ module.exports = ProjectLocator = { _findElementByPathWithProject(project, needlePath, exactCaseMatch, callback) { let matchFn - if (callback == null) { - callback = function(err, foundEntity, type) {} - } if (exactCaseMatch) { matchFn = (a, b) => a === b } else { @@ -205,13 +178,13 @@ module.exports = ProjectLocator = { (b != null ? b.toLowerCase() : undefined) } - var getParentFolder = function(haystackFolder, foldersList, level, cb) { + function getParentFolder(haystackFolder, foldersList, level, cb) { if (foldersList.length === 0) { return cb(null, haystackFolder) } const needleFolderName = foldersList[level] let found = false - for (let folder of Array.from(haystackFolder.folders)) { + for (let folder of haystackFolder.folders) { if (matchFn(folder.name, needleFolderName)) { found = true if (level === foldersList.length - 1) { @@ -222,34 +195,36 @@ module.exports = ProjectLocator = { } } if (!found) { - return cb( - `not found project: ${ - project._id - } search path: ${needlePath}, folder ${ - foldersList[level] - } could not be found` + cb( + new Error( + `not found project: ${ + project._id + } search path: ${needlePath}, folder ${ + foldersList[level] + } could not be found` + ) ) } } - const getEntity = function(folder, entityName, cb) { + function getEntity(folder, entityName, cb) { let result, type if (entityName == null) { return cb(null, folder, 'folder') } - for (let file of Array.from(folder.fileRefs || [])) { + for (let file of folder.fileRefs || []) { if (matchFn(file != null ? file.name : undefined, entityName)) { result = file type = 'file' } } - for (let doc of Array.from(folder.docs || [])) { + for (let doc of folder.docs || []) { if (matchFn(doc != null ? doc.name : undefined, entityName)) { result = doc type = 'doc' } } - for (let childFolder of Array.from(folder.folders || [])) { + for (let childFolder of folder.folders || []) { if ( matchFn( childFolder != null ? childFolder.name : undefined, @@ -262,25 +237,20 @@ module.exports = ProjectLocator = { } if (result != null) { - return cb(null, result, type) + cb(null, result, type) } else { - return cb( - `not found project: ${ - project._id - } search path: ${needlePath}, entity ${entityName} could not be found` + cb( + new Error( + `not found project: ${ + project._id + } search path: ${needlePath}, entity ${entityName} could not be found` + ) ) } } - if (typeof err !== 'undefined' && err !== null) { - logger.warn( - { err, project_id: project._id }, - 'error getting project for finding element' - ) - return callback(err) - } if (project == null) { - return callback('Tried to find an element for a null project') + return callback(new Error('Tried to find an element for a null project')) } if (needlePath === '' || needlePath === '/') { return callback(null, project.rootFolder[0], 'folder') @@ -294,22 +264,22 @@ module.exports = ProjectLocator = { const rootFolder = project.rootFolder[0] logger.log( - { project_id: project._id, path: needlePath, foldersList }, + { projectId: project._id, path: needlePath, foldersList }, 'looking for element by path' ) - const jobs = new Array() + const jobs = [] jobs.push(cb => getParentFolder(rootFolder, foldersList, 0, cb)) jobs.push((folder, cb) => getEntity(folder, needleName, cb)) - return async.waterfall(jobs, callback) + async.waterfall(jobs, callback) }, - findUsersProjectByName(user_id, projectName, callback) { - return ProjectGetter.findAllUsersProjects( - user_id, + findUsersProjectByName(userId, projectName, callback) { + ProjectGetter.findAllUsersProjects( + userId, 'name archived trashed', - function(err, allProjects) { - if (typeof error !== 'undefined' && error !== null) { - return callback(error) + (err, allProjects) => { + if (err != null) { + return callback(err) } const { owned, readAndWrite } = allProjects const projects = owned.concat(readAndWrite) @@ -318,19 +288,19 @@ module.exports = ProjectLocator = { projects, project => project.name.toLowerCase() === projectName && - !ProjectHelper.isArchivedOrTrashed(project, user_id) + !ProjectHelper.isArchivedOrTrashed(project, userId) ) logger.log( - { user_id, projectName, totalProjects: projects.length, project }, + { userId, projectName, totalProjects: projects.length, project }, 'looking for project by name' ) - return callback(null, project) + callback(null, project) } ) } } -var sanitizeTypeOfElement = function(elementType) { +function sanitizeTypeOfElement(elementType) { const lastChar = elementType.slice(-1) if (lastChar !== 's') { elementType += 's' @@ -341,7 +311,7 @@ var sanitizeTypeOfElement = function(elementType) { return elementType } -var getIndexOf = function(searchEntity, id) { +function getIndexOf(searchEntity, id) { const { length } = searchEntity let count = 0 while (count < length) { @@ -355,3 +325,5 @@ var getIndexOf = function(searchEntity, id) { count++ } } + +module.exports = ProjectLocator diff --git a/services/web/app/src/infrastructure/LockManager.js b/services/web/app/src/infrastructure/LockManager.js index 31a032c814..7c4a05bfa8 100644 --- a/services/web/app/src/infrastructure/LockManager.js +++ b/services/web/app/src/infrastructure/LockManager.js @@ -1,20 +1,4 @@ -/* eslint-disable - 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: - * DS101: Remove unnecessary use of Array.from - * 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 LockManager const metrics = require('metrics-sharelatex') -const Settings = require('settings-sharelatex') const RedisWrapper = require('./RedisWrapper') const rclient = RedisWrapper.client('lock') const logger = require('logger-sharelatex') @@ -29,7 +13,7 @@ let COUNT = 0 const LOCK_QUEUES = new Map() // queue lock requests for each name/id so they get the lock on a first-come first-served basis -module.exports = LockManager = { +const LockManager = { LOCK_TEST_INTERVAL: 50, // 50ms between each test of the lock MAX_TEST_INTERVAL: 1000, // back off to 1s between each test of the lock MAX_LOCK_WAIT_TIME: 10000, // 10s maximum time to spend trying to get the lock @@ -51,23 +35,20 @@ module.exports = LockManager = { // runner must be a function accepting a callback, e.g. runner = (cb) -> // This error is defined here so we get a useful stacktrace - if (callback == null) { - callback = function(error) {} - } const slowExecutionError = new Error('slow execution during lock') const timer = new metrics.Timer(`lock.${namespace}`) const key = `lock:web:${namespace}:${id}` - LockManager._getLock(key, namespace, function(error, lockValue) { + LockManager._getLock(key, namespace, (error, lockValue) => { if (error != null) { return callback(error) } - // The lock can expire in redis but the process carry on. This setTimout call + // The lock can expire in redis but the process carry on. This setTimeout call // is designed to log if this happens. - const countIfExceededLockTimeout = function() { + function countIfExceededLockTimeout() { metrics.inc(`lock.${namespace}.exceeded_lock_timeout`) - return logger.log('exceeded lock timeout', { + logger.log('exceeded lock timeout', { namespace, id, slowExecutionError @@ -78,8 +59,8 @@ module.exports = LockManager = { LockManager.REDIS_LOCK_EXPIRY * 1000 ) - return runner((error1, ...values) => - LockManager._releaseLock(key, lockValue, function(error2) { + runner((error1, ...values) => + LockManager._releaseLock(key, lockValue, error2 => { clearTimeout(exceededLockTimeout) const timeTaken = new Date() - timer.start @@ -97,16 +78,13 @@ module.exports = LockManager = { if (error != null) { return callback(error) } - return callback(null, ...Array.from(values)) + callback(null, ...values) }) ) }) }, _tryLock(key, namespace, callback) { - if (callback == null) { - callback = function(err, isFree, lockValue) {} - } const lockValue = LockManager.randomLock() rclient.set( key, @@ -114,17 +92,17 @@ module.exports = LockManager = { 'EX', LockManager.REDIS_LOCK_EXPIRY, 'NX', - function(err, gotLock) { + (err, gotLock) => { if (err != null) { return callback(err) } if (gotLock === 'OK') { metrics.inc(`lock.${namespace}.try.success`) - return callback(err, true, lockValue) + callback(err, true, lockValue) } else { metrics.inc(`lock.${namespace}.try.failed`) logger.log({ key, redis_response: gotLock }, 'lock is locked') - return callback(err, false) + callback(err, false) } } ) @@ -133,15 +111,12 @@ module.exports = LockManager = { // it's sufficient to serialize within a process because that is where the parallel operations occur _getLock(key, namespace, callback) { // this is what we need to do for each lock we want to request - if (callback == null) { - callback = function(error, lockValue) {} - } const task = next => - LockManager._getLockByPolling(key, namespace, function(error, lockValue) { + LockManager._getLockByPolling(key, namespace, (error, lockValue) => { // tell the queue to start trying to get the next lock (if any) next() // we have got a lock result, so we can continue with our own execution - return callback(error, lockValue) + callback(error, lockValue) }) // create a queue for this key if needed const queueName = `${key}:${namespace}` @@ -154,53 +129,43 @@ module.exports = LockManager = { // remove the queue object when queue is empty queue.drain = () => LOCK_QUEUES.delete(queueName) // store the queue in our global map - return LOCK_QUEUES.set(queueName, queue) + LOCK_QUEUES.set(queueName, queue) } else { // queue the request to get the lock - return queue.push(task) + queue.push(task) } }, _getLockByPolling(key, namespace, callback) { - let attempt - if (callback == null) { - callback = function(error, lockValue) {} - } const startTime = Date.now() const testInterval = LockManager.LOCK_TEST_INTERVAL let attempts = 0 - return (attempt = function() { + function attempt() { if (Date.now() - startTime > LockManager.MAX_LOCK_WAIT_TIME) { metrics.inc(`lock.${namespace}.get.failed`) return callback(new Error('Timeout')) } attempts += 1 - return LockManager._tryLock(key, namespace, function( - error, - gotLock, - lockValue - ) { + LockManager._tryLock(key, namespace, (error, gotLock, lockValue) => { if (error != null) { return callback(error) } if (gotLock) { metrics.gauge(`lock.${namespace}.get.success.tries`, attempts) - return callback(null, lockValue) + callback(null, lockValue) } else { - return setTimeout(attempt, testInterval) + setTimeout(attempt, testInterval) } }) - })() + } + attempt() }, _releaseLock(key, lockValue, callback) { - rclient.eval(LockManager.unlockScript, 1, key, lockValue, function( - err, - result - ) { + rclient.eval(LockManager.unlockScript, 1, key, lockValue, (err, result) => { if (err != null) { - return callback(err) + callback(err) } else if (result != null && result !== 1) { // successful unlock should release exactly one key logger.warn( @@ -208,10 +173,12 @@ module.exports = LockManager = { 'unlocking error' ) metrics.inc('unlock-error') - return callback(new Error('tried to release timed out lock')) + callback(new Error('tried to release timed out lock')) } else { - return callback(null, result) + callback(null, result) } }) } } + +module.exports = LockManager diff --git a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js index 7188446ea3..575d2ffce1 100644 --- a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js @@ -1,39 +1,23 @@ -/* eslint-disable - camelcase, - handle-callback-err, - max-len, - no-return-assign, - no-unused-vars, - standard/no-callback-literal, -*/ -// 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 chai = require('chai') const { expect } = chai const { assert } = require('chai') -const should = chai.should() const sinon = require('sinon') const tk = require('timekeeper') -const modulePath = - '../../../../app/src/Features/Project/ProjectEntityMongoUpdateHandler' const Errors = require('../../../../app/src/Features/Errors/Errors') const { ObjectId } = require('mongoose').Types const SandboxedModule = require('sandboxed-module') +const MODULE_PATH = + '../../../../app/src/Features/Project/ProjectEntityMongoUpdateHandler' + describe('ProjectEntityMongoUpdateHandler', function() { - const project_id = '4eecb1c1bffa66588e0000a1' - const doc_id = '4eecb1c1bffa66588e0000a2' - const file_id = '4eecb1c1bffa66588e0000a3' - const folder_id = '4eecaffcbffa66588e000008' + const projectId = '4eecb1c1bffa66588e0000a1' + const docId = '4eecb1c1bffa66588e0000a2' + const fileId = '4eecb1c1bffa66588e0000a3' + const folderId = '4eecaffcbffa66588e000008' beforeEach(function() { - let Folder - this.FolderModel = Folder = class Folder { + this.FolderModel = class Folder { constructor(options) { ;({ name: this.name } = options) this._id = 'folder_id' @@ -42,12 +26,12 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.docName = 'doc-name' this.fileName = 'something.jpg' - this.project = { _id: project_id, name: 'project name' } + this.project = { _id: projectId, name: 'project name' } this.callback = sinon.stub() tk.freeze(Date.now()) - return (this.subject = SandboxedModule.require(modulePath, { + this.subject = SandboxedModule.require(MODULE_PATH, { globals: { console: console }, @@ -79,25 +63,25 @@ describe('ProjectEntityMongoUpdateHandler', function() { getProjectWithoutLock: sinon.stub().yields(null, this.project) }) } - })) + }) }) afterEach(function() { - return tk.reset() + tk.reset() }) describe('addDoc', function() { beforeEach(function() { - this.subject._confirmFolder = sinon.stub().yields(folder_id) + this.subject._confirmFolder = sinon.stub().yields(folderId) this.subject._putElement = sinon.stub() - this.doc = { _id: doc_id } - return this.subject.addDoc(project_id, folder_id, this.doc, this.callback) + this.doc = { _id: docId } + this.subject.addDoc(projectId, folderId, this.doc, this.callback) }) it('gets the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -106,35 +90,30 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('checks the folder exists', function() { - return this.subject._confirmFolder - .calledWith(this.project, folder_id) + this.subject._confirmFolder + .calledWith(this.project, folderId) .should.equal(true) }) it('puts the element in mongo', function() { - return this.subject._putElement - .calledWith(this.project, folder_id, this.doc, 'doc', this.callback) + this.subject._putElement + .calledWith(this.project, folderId, this.doc, 'doc', this.callback) .should.equal(true) }) }) describe('addFile', function() { beforeEach(function() { - this.subject._confirmFolder = sinon.stub().yields(folder_id) + this.subject._confirmFolder = sinon.stub().yields(folderId) this.subject._putElement = sinon.stub() - this.file = { _id: file_id } - return this.subject.addFile( - project_id, - folder_id, - this.file, - this.callback - ) + this.file = { _id: fileId } + this.subject.addFile(projectId, folderId, this.file, this.callback) }) it('gets the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -143,21 +122,21 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('checks the folder exists', function() { - return this.subject._confirmFolder - .calledWith(this.project, folder_id) + this.subject._confirmFolder + .calledWith(this.project, folderId) .should.equal(true) }) it('puts the element in mongo', function() { - return this.subject._putElement - .calledWith(this.project, folder_id, this.file, 'file', this.callback) + this.subject._putElement + .calledWith(this.project, folderId, this.file, 'file', this.callback) .should.equal(true) }) }) describe('replaceFileWithNew', function() { beforeEach(function() { - this.file = { _id: file_id } + this.file = { _id: fileId } this.path = { mongo: 'file.png' } this.newFile = { _id: 'new-file-id' } this.newFile.linkedFileData = this.linkedFileData = { provider: 'url' } @@ -170,17 +149,17 @@ describe('ProjectEntityMongoUpdateHandler', function() { .yields(null, this.newProject) this.ProjectModel.update = sinon.stub().yields() - return this.subject.replaceFileWithNew( - project_id, - file_id, + this.subject.replaceFileWithNew( + projectId, + fileId, this.newFile, this.callback ) }) it('gets the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -189,23 +168,23 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('finds the existing element', function() { - return this.ProjectLocator.findElement + this.ProjectLocator.findElement .calledWith({ project: this.project, - element_id: file_id, + element_id: fileId, type: 'file' }) .should.equal(true) }) it('inserts a deletedFile reference for the old file', function() { - return this.ProjectModel.update + this.ProjectModel.update .calledWith( - { _id: project_id }, + { _id: projectId }, { $push: { deletedFiles: { - _id: file_id, + _id: fileId, name: this.file.name, linkedFileData: this.file.linkedFileData, hash: this.file.hash, @@ -218,9 +197,9 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('increments the project version and sets the rev and created_at', function() { - return this.ProjectModel.findOneAndUpdate + this.ProjectModel.findOneAndUpdate .calledWith( - { _id: project_id }, + { _id: projectId }, { $inc: { version: 1, 'file.png.rev': 1 }, $set: { @@ -236,7 +215,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('calls the callback', function() { - return this.callback + this.callback .calledWith(null, this.file, this.project, this.path, this.newProject) .should.equal(true) }) @@ -244,12 +223,12 @@ describe('ProjectEntityMongoUpdateHandler', function() { describe('mkdirp', function() { beforeEach(function() { - this.parentFolder_id = '1jnjknjk' + this.parentFolderId = '1jnjknjk' this.newFolder = { _id: 'newFolder_id_here' } this.lastFolder = { _id: '123das', folders: [] } this.rootFolder = { _id: 'rootFolderId' } - this.project = { _id: project_id, rootFolder: [this.rootFolder] } + this.project = { _id: projectId, rootFolder: [this.rootFolder] } this.ProjectGetter.getProjectWithOnlyFolders = sinon .stub() @@ -260,124 +239,115 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.parentFolder = { _id: 'parentFolder_id_here' } const lastFolder = path.substring(path.lastIndexOf('/')) if (lastFolder.indexOf('level1') === -1) { - return cb('level1 is not the last foler ') + cb(new Error('level1 is not the last folder')) } else { - return cb(null, this.parentFolder) + cb(null, this.parentFolder) } }) - return (this.subject.addFolder = { - withoutLock: (project_id, parentFolder_id, folderName, callback) => { - return callback(null, { name: folderName }, this.parentFolder_id) + this.subject.addFolder = { + withoutLock: (projectId, parentFolderId, folderName, callback) => { + return callback(null, { name: folderName }, this.parentFolderId) } - }) + } }) it('should return the root folder if the path is just a slash', function(done) { const path = '/' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - lastFolder.should.deep.equal(this.rootFolder) - assert.equal(lastFolder.parentFolder_id, undefined) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + lastFolder.should.deep.equal(this.rootFolder) + assert.equal(lastFolder.parentFolder_id, undefined) + done() + }) }) it('should make just one folder', function(done) { const path = '/differentFolder/' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - folders.length.should.equal(1) - lastFolder.name.should.equal('differentFolder') - lastFolder.parentFolder_id.should.equal(this.parentFolder_id) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + folders.length.should.equal(1) + lastFolder.name.should.equal('differentFolder') + lastFolder.parentFolder_id.should.equal(this.parentFolderId) + done() + }) }) it('should make the final folder in path if it doesnt exist with one level', function(done) { const path = 'level1/level2' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - folders.length.should.equal(1) - lastFolder.name.should.equal('level2') - lastFolder.parentFolder_id.should.equal(this.parentFolder_id) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + folders.length.should.equal(1) + lastFolder.name.should.equal('level2') + lastFolder.parentFolder_id.should.equal(this.parentFolderId) + done() + }) }) it('should make the final folder in path if it doesnt exist with mutliple levels', function(done) { const path = 'level1/level2/level3' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - folders.length.should.equal(2) - folders[0].name.should.equal('level2') - folders[0].parentFolder_id.should.equal(this.parentFolder_id) - lastFolder.name.should.equal('level3') - lastFolder.parentFolder_id.should.equal(this.parentFolder_id) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + folders.length.should.equal(2) + folders[0].name.should.equal('level2') + folders[0].parentFolder_id.should.equal(this.parentFolderId) + lastFolder.name.should.equal('level3') + lastFolder.parentFolder_id.should.equal(this.parentFolderId) + done() + }) }) it('should work with slashes either side', function(done) { const path = '/level1/level2/level3/' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - folders.length.should.equal(2) - folders[0].name.should.equal('level2') - folders[0].parentFolder_id.should.equal(this.parentFolder_id) - lastFolder.name.should.equal('level3') - lastFolder.parentFolder_id.should.equal(this.parentFolder_id) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + folders.length.should.equal(2) + folders[0].name.should.equal('level2') + folders[0].parentFolder_id.should.equal(this.parentFolderId) + lastFolder.name.should.equal('level3') + lastFolder.parentFolder_id.should.equal(this.parentFolderId) + done() + }) }) it('should use a case-insensitive match by default', function(done) { const path = '/differentFolder/' - return this.subject.mkdirp( - project_id, - path, - {}, - (err, folders, lastFolder) => { - this.ProjectLocator.findElementByPath - .calledWithMatch({ exactCaseMatch: undefined }) - .should.equal(true) - return done() + this.subject.mkdirp(projectId, path, {}, (err, folders, lastFolder) => { + if (err != null) { + return done(err) } - ) + this.ProjectLocator.findElementByPath + .calledWithMatch({ exactCaseMatch: undefined }) + .should.equal(true) + done() + }) }) it('should use a case-sensitive match if exactCaseMatch option is set', function(done) { const path = '/differentFolder/' - return this.subject.mkdirp( - project_id, + this.subject.mkdirp( + projectId, path, { exactCaseMatch: true }, (err, folders, lastFolder) => { + if (err != null) { + return done(err) + } this.ProjectLocator.findElementByPath .calledWithMatch({ exactCaseMatch: true }) .should.equal(true) - return done() + done() } ) }) @@ -429,18 +399,12 @@ describe('ProjectEntityMongoUpdateHandler', function() { .stub() .yields(null, { path: this.pathAfterMove }, this.newProject) - return this.subject.moveEntity( - project_id, - doc_id, - folder_id, - 'docs', - this.callback - ) + this.subject.moveEntity(projectId, docId, folderId, 'docs', this.callback) }) it('should get the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -449,31 +413,31 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should find the doc to move', function() { - return this.ProjectLocator.findElement - .calledWith({ element_id: doc_id, type: 'docs', project: this.project }) + this.ProjectLocator.findElement + .calledWith({ element_id: docId, type: 'docs', project: this.project }) .should.equal(true) }) it('should check this is a valid move', function() { - return this.subject._checkValidMove - .calledWith(this.project, 'docs', this.doc, this.path, folder_id) + this.subject._checkValidMove + .calledWith(this.project, 'docs', this.doc, this.path, folderId) .should.equal(true) }) it('should put the element in the new folder', function() { - return this.subject._putElement - .calledWith(this.project, folder_id, this.doc, 'docs') + this.subject._putElement + .calledWith(this.project, folderId, this.doc, 'docs') .should.equal(true) }) it('should remove the element from its current position', function() { - return this.subject._removeElementFromMongoArray - .calledWith(this.ProjectModel, project_id, this.path.mongo, doc_id) + this.subject._removeElementFromMongoArray + .calledWith(this.ProjectModel, projectId, this.path.mongo, docId) .should.equal(true) }) it('should remove the element from its current position after putting the element in the new folder', function() { - return this.subject._removeElementFromMongoArray + this.subject._removeElementFromMongoArray .calledAfter(this.subject._putElement) .should.equal(true) }) @@ -486,7 +450,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { newFiles: this.newFiles, newProject: this.newProject } - return this.callback + this.callback .calledWith( null, this.project, @@ -530,18 +494,12 @@ describe('ProjectEntityMongoUpdateHandler', function() { .stub() .yields(null, { path: this.pathAfterMove }, this.newProject) - return this.subject.moveEntity( - project_id, - doc_id, - folder_id, - 'docs', - this.callback - ) + this.subject.moveEntity(projectId, docId, folderId, 'docs', this.callback) }) it('should get the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -550,46 +508,44 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should find the doc to move', function() { - return this.ProjectLocator.findElement - .calledWith({ element_id: doc_id, type: 'docs', project: this.project }) + this.ProjectLocator.findElement + .calledWith({ element_id: docId, type: 'docs', project: this.project }) .should.equal(true) }) it('should check this is an invalid move', function() { - return this.subject._checkValidMove - .calledWith(this.project, 'docs', this.doc, this.path, folder_id) + this.subject._checkValidMove + .calledWith(this.project, 'docs', this.doc, this.path, folderId) .should.equal(true) }) it('should not put the element in the new folder', function() { - return this.subject._putElement.called.should.equal(false) + this.subject._putElement.called.should.equal(false) }) it('should not remove the element from its current position', function() { - return this.subject._removeElementFromMongoArray.called.should.equal( - false - ) + this.subject._removeElementFromMongoArray.called.should.equal(false) }) it('calls the callback with an error', function() { - return this.callback.calledWith(new Error()).should.equal(true) + this.callback.calledWith(new Error()).should.equal(true) }) }) describe('deleteEntity', function() { beforeEach(function() { this.path = { mongo: 'mongo.path', fileSystem: '/file/system/path' } - this.doc = { _id: doc_id } + this.doc = { _id: docId } this.ProjectLocator.findElement = sinon .stub() .callsArgWith(1, null, this.doc, this.path) this.subject._removeElementFromMongoArray = sinon.stub().yields() - return this.subject.deleteEntity(project_id, doc_id, 'doc', this.callback) + this.subject.deleteEntity(projectId, docId, 'doc', this.callback) }) it('should get the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -598,7 +554,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should find the element', function() { - return this.ProjectLocator.findElement + this.ProjectLocator.findElement .calledWith({ project: this.project, element_id: this.doc._id, @@ -608,18 +564,13 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should remove the element from the database', function() { - return this.subject._removeElementFromMongoArray - .calledWith( - this.ProjectModel, - project_id, - this.path.mongo, - this.doc._id - ) + this.subject._removeElementFromMongoArray + .calledWith(this.ProjectModel, projectId, this.path.mongo, this.doc._id) .should.equal(true) }) it('calls the callbck', function() { - return this.callback + this.callback .calledWith(null, this.doc, this.path, this.project) .should.equal(true) }) @@ -631,11 +582,11 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.path = { mongo: 'mongo.path', fileSystem: '/old.tex' } this.project = { - _id: ObjectId(project_id), + _id: ObjectId(projectId), rootFolder: [{ _id: ObjectId() }] } - this.doc = { _id: doc_id, name: 'old.tex', rev: 1 } - this.folder = { _id: folder_id } + this.doc = { _id: docId, name: 'old.tex', rev: 1 } + this.folder = { _id: folderId } this.newProject = 'new-project' this.ProjectGetter.getProjectWithoutLock = sinon @@ -666,9 +617,9 @@ describe('ProjectEntityMongoUpdateHandler', function() { .stub() .callsArgWith(3, null, this.newProject) - return this.subject.renameEntity( - project_id, - doc_id, + this.subject.renameEntity( + projectId, + docId, 'doc', this.newName, this.callback @@ -676,8 +627,8 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should get the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -686,21 +637,21 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should find the doc', function() { - return this.ProjectLocator.findElement - .calledWith({ element_id: doc_id, type: 'doc', project: this.project }) + this.ProjectLocator.findElement + .calledWith({ element_id: docId, type: 'doc', project: this.project }) .should.equal(true) }) it('should check the new name is valid', function() { - return this.subject._checkValidElementName + this.subject._checkValidElementName .calledWith(this.folder, this.newName) .should.equal(true) }) it('should update the doc name', function() { - return this.ProjectModel.findOneAndUpdate + this.ProjectModel.findOneAndUpdate .calledWith( - { _id: project_id }, + { _id: projectId }, { $set: { 'mongo.path.name': this.newName }, $inc: { version: 1 } }, { new: true } ) @@ -715,7 +666,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { newFiles: this.newFiles, newProject: this.newProject } - return this.callback + this.callback .calledWith( null, this.project, @@ -734,20 +685,20 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.ProjectGetter.getProjectWithOnlyFolders = sinon .stub() .callsArgWith(1, null, this.project) - this.subject._confirmFolder = sinon.stub().yields(folder_id) + this.subject._confirmFolder = sinon.stub().yields(folderId) this.subject._putElement = sinon.stub().yields() - return this.subject.addFolder( - project_id, - folder_id, + this.subject.addFolder( + projectId, + folderId, this.folderName, this.callback ) }) it('gets the project', function() { - return this.ProjectGetter.getProjectWithoutLock - .calledWith(project_id, { + this.ProjectGetter.getProjectWithoutLock + .calledWith(projectId, { rootFolder: true, name: true, overleaf: true @@ -756,28 +707,28 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('checks the parent folder exists', function() { - return this.subject._confirmFolder - .calledWith(this.project, folder_id) + this.subject._confirmFolder + .calledWith(this.project, folderId) .should.equal(true) }) it('puts the element in mongo', function() { - const folderMatcher = sinon.match(folder => { - return folder.name === this.folderName - }) + const folderMatcher = sinon.match( + folder => folder.name === this.folderName + ) - return this.subject._putElement - .calledWithMatch(this.project, folder_id, folderMatcher, 'folder') + this.subject._putElement + .calledWithMatch(this.project, folderId, folderMatcher, 'folder') .should.equal(true) }) it('calls the callback', function() { - const folderMatcher = sinon.match(folder => { - return folder.name === this.folderName - }) + const folderMatcher = sinon.match( + folder => folder.name === this.folderName + ) - return this.callback - .calledWithMatch(null, folderMatcher, folder_id) + this.callback + .calledWithMatch(null, folderMatcher, folderId) .should.equal(true) }) }) @@ -791,7 +742,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.ProjectModel.findOneAndUpdate = sinon .stub() .yields(null, this.project) - return this.subject._removeElementFromMongoArray( + this.subject._removeElementFromMongoArray( this.ProjectModel, this.id, this.mongoPath, @@ -801,7 +752,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should pull', function() { - return this.ProjectModel.findOneAndUpdate + this.ProjectModel.findOneAndUpdate .calledWith( { _id: this.id }, { @@ -814,14 +765,14 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should call the callback', function() { - return this.callback.calledWith(null, this.project).should.equal(true) + this.callback.calledWith(null, this.project).should.equal(true) }) }) describe('_countElements', function() { beforeEach(function() { - return (this.project = { - _id: project_id, + this.project = { + _id: projectId, rootFolder: [ { docs: [{ _id: 123 }, { _id: 345 }], @@ -862,33 +813,33 @@ describe('ProjectEntityMongoUpdateHandler', function() { ] } ] - }) + } }) it('should return the correct number', function() { - return expect(this.subject._countElements(this.project)).to.equal(26) + expect(this.subject._countElements(this.project)).to.equal(26) }) it('should deal with null folders', function() { this.project.rootFolder[0].folders[0].folders = undefined - return expect(this.subject._countElements(this.project)).to.equal(17) + expect(this.subject._countElements(this.project)).to.equal(17) }) it('should deal with null docs', function() { this.project.rootFolder[0].folders[0].docs = undefined - return expect(this.subject._countElements(this.project)).to.equal(23) + expect(this.subject._countElements(this.project)).to.equal(23) }) it('should deal with null fileRefs', function() { this.project.rootFolder[0].folders[0].folders[0].fileRefs = undefined - return expect(this.subject._countElements(this.project)).to.equal(23) + expect(this.subject._countElements(this.project)).to.equal(23) }) }) describe('_putElement', function() { beforeEach(function() { this.project = { - _id: project_id, + _id: projectId, rootFolder: [{ _id: ObjectId() }] } this.folder = { @@ -906,19 +857,22 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.ProjectLocator.findElement = sinon .stub() .yields(null, this.folder, this.path) - return (this.ProjectModel.findOneAndUpdate = sinon + this.ProjectModel.findOneAndUpdate = sinon .stub() - .yields(null, this.project)) + .yields(null, this.project) }) describe('updating the project', function() { it('should use the correct mongo path', function(done) { - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, this.doc, 'docs', err => { + if (err != null) { + return done(err) + } this.ProjectModel.findOneAndUpdate.args[0][0]._id.should.equal( this.project._id ) @@ -928,80 +882,85 @@ describe('ProjectEntityMongoUpdateHandler', function() { ], this.doc ) - return done() + done() } ) }) it('should return the project in the callback', function(done) { - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, this.doc, 'docs', (err, path, project) => { + if (err != null) { + return done(err) + } assert.equal(project, this.project) - return done() + done() } ) }) it('should add an s onto the type if not included', function(done) { - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, this.doc, 'doc', err => { + if (err != null) { + return done(err) + } assert.deepEqual( this.ProjectModel.findOneAndUpdate.args[0][1].$push[ this.path.mongo + '.docs' ], this.doc ) - return done() + done() } ) }) it('should not call update if element is null', function(done) { - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, null, 'doc', err => { + expect(err).to.exist this.ProjectModel.findOneAndUpdate.called.should.equal(false) - return done() + done() } ) }) it('should default to root folder insert', function(done) { - return this.subject._putElement( - this.project, - null, - this.doc, - 'doc', - err => { - this.ProjectLocator.findElement.args[0][0].element_id.should.equal( - this.project.rootFolder[0]._id - ) - return done() + this.subject._putElement(this.project, null, this.doc, 'doc', err => { + if (err != null) { + return done(err) } - ) + this.ProjectLocator.findElement.args[0][0].element_id.should.equal( + this.project.rootFolder[0]._id + ) + done() + }) }) it('should error if the element has no _id', function(done) { const doc = { name: 'something' } - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, doc, 'doc', err => { + expect(err).to.exist this.ProjectModel.findOneAndUpdate.called.should.equal(false) - return done() + done() } ) }) @@ -1011,7 +970,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: 'something*bad' } - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, doc, @@ -1021,7 +980,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { err.should.deep.equal( new Errors.InvalidNameError('invalid element name') ) - return done() + done() } ) }) @@ -1031,7 +990,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: new Array(200).join('long-') + 'something' } - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, doc, @@ -1041,7 +1000,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { err.should.deep.equal( new Errors.InvalidNameError('invalid element name') ) - return done() + done() } ) }) @@ -1061,7 +1020,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: 'something' } - return this.subject._putElement( + this.subject._putElement( this.project, this.folder._id, doc, @@ -1069,7 +1028,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { err => { this.ProjectModel.findOneAndUpdate.called.should.equal(false) err.should.deep.equal(new Errors.InvalidNameError('path too long')) - return done() + done() } ) }) @@ -1079,19 +1038,13 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: 'another-doc.tex' } - return this.subject._putElement( - this.project, - this.folder, - doc, - 'doc', - err => { - this.ProjectModel.findOneAndUpdate.called.should.equal(false) - err.should.deep.equal( - new Errors.InvalidNameError('file already exists') - ) - return done() - } - ) + this.subject._putElement(this.project, this.folder, doc, 'doc', err => { + this.ProjectModel.findOneAndUpdate.called.should.equal(false) + err.should.deep.equal( + new Errors.InvalidNameError('file already exists') + ) + done() + }) }) it('should error if a file already exists with the same name', function(done) { @@ -1099,19 +1052,13 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: 'another-file.tex' } - return this.subject._putElement( - this.project, - this.folder, - doc, - 'doc', - err => { - this.ProjectModel.findOneAndUpdate.called.should.equal(false) - err.should.deep.equal( - new Errors.InvalidNameError('file already exists') - ) - return done() - } - ) + this.subject._putElement(this.project, this.folder, doc, 'doc', err => { + this.ProjectModel.findOneAndUpdate.called.should.equal(false) + err.should.deep.equal( + new Errors.InvalidNameError('file already exists') + ) + done() + }) }) it('should error if a folder already exists with the same name', function(done) { @@ -1119,34 +1066,28 @@ describe('ProjectEntityMongoUpdateHandler', function() { _id: ObjectId(), name: 'another-folder' } - return this.subject._putElement( - this.project, - this.folder, - doc, - 'doc', - err => { - this.ProjectModel.findOneAndUpdate.called.should.equal(false) - err.should.deep.equal( - new Errors.InvalidNameError('file already exists') - ) - return done() - } - ) + this.subject._putElement(this.project, this.folder, doc, 'doc', err => { + this.ProjectModel.findOneAndUpdate.called.should.equal(false) + err.should.deep.equal( + new Errors.InvalidNameError('file already exists') + ) + done() + }) }) }) }) describe('_checkValidElementName', function() { beforeEach(function() { - return (this.folder = { + this.folder = { docs: [{ name: 'doc_name' }], fileRefs: [{ name: 'file_name' }], folders: [{ name: 'folder_name' }] - }) + } }) it('returns an error if name matches any doc name', function() { - return this.subject._checkValidElementName(this.folder, 'doc_name', err => + this.subject._checkValidElementName(this.folder, 'doc_name', err => expect(err).to.deep.equal( new Errors.InvalidNameError('file already exists') ) @@ -1154,29 +1095,23 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('returns an error if name matches any file name', function() { - return this.subject._checkValidElementName( - this.folder, - 'file_name', - err => - expect(err).to.deep.equal( - new Errors.InvalidNameError('file already exists') - ) + this.subject._checkValidElementName(this.folder, 'file_name', err => + expect(err).to.deep.equal( + new Errors.InvalidNameError('file already exists') + ) ) }) it('returns an error if name matches any folder name', function() { - return this.subject._checkValidElementName( - this.folder, - 'folder_name', - err => - expect(err).to.deep.equal( - new Errors.InvalidNameError('file already exists') - ) + this.subject._checkValidElementName(this.folder, 'folder_name', err => + expect(err).to.deep.equal( + new Errors.InvalidNameError('file already exists') + ) ) }) it('returns nothing if name is valid', function() { - return this.subject._checkValidElementName( + this.subject._checkValidElementName( this.folder, 'unique_name', err => expect(err).to.be.undefined @@ -1186,17 +1121,17 @@ describe('ProjectEntityMongoUpdateHandler', function() { describe('_checkValidMove', function() { beforeEach(function() { - this.destFolder = { _id: folder_id } + this.destFolder = { _id: folderId } this.destFolderPath = { fileSystem: '/foo/bar' } this.ProjectLocator.findElement = sinon .stub() .yields(null, this.destFolder, this.destFolderPath) - return (this.subject._checkValidElementName = sinon.stub().yields()) + this.subject._checkValidElementName = sinon.stub().yields() }) it('checks the element name is valid', function() { - this.doc = { _id: doc_id, name: 'doc_name' } - return this.subject._checkValidMove( + this.doc = { _id: docId, name: 'doc_name' } + this.subject._checkValidMove( this.project, 'doc', this.doc, @@ -1204,7 +1139,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { this.destFolder._id, err => { expect(err).to.be.undefined - return this.subject._checkValidElementName + this.subject._checkValidElementName .calledWith(this.destFolder, this.doc.name) .should.equal(true) } @@ -1213,14 +1148,14 @@ describe('ProjectEntityMongoUpdateHandler', function() { it('returns an error if trying to move a folder inside itself', function() { const folder = { name: 'folder_name' } - return this.subject._checkValidMove( + this.subject._checkValidMove( this.project, 'folder', folder, { fileSystem: '/foo' }, this.destFolder._id, err => { - return expect(err).to.deep.equal( + expect(err).to.deep.equal( new Errors.InvalidNameError( 'destination folder is a child folder of me' ) @@ -1238,18 +1173,18 @@ describe('ProjectEntityMongoUpdateHandler', function() { } this.callback = sinon.stub() this.ProjectModel.update = sinon.stub().yields() - return this.subject._insertDeletedDocReference( - project_id, + this.subject._insertDeletedDocReference( + projectId, this.doc, this.callback ) }) it('should insert the doc into deletedDocs', function() { - return this.ProjectModel.update + this.ProjectModel.update .calledWith( { - _id: project_id + _id: projectId }, { $push: { @@ -1265,7 +1200,7 @@ describe('ProjectEntityMongoUpdateHandler', function() { }) it('should call the callback', function() { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) })