From 36f0a3e01acd574e2ea473595efdabc2ab5e3c85 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Wed, 29 May 2024 14:19:10 +0200 Subject: [PATCH] [web] Promisify ProjectController (#18477) * Create `promiseAuto` util to replace `async.auto` * Promisify `BrandVariationsHandler.getBrandVariationById` * Promisify `updateProjectSettings` * Promisify `updateProjectAdminSettings` * Promisify `newProject` * Promisify `deleteProject` * Promisify `loadEditor` * Fix brandVariation loading in promise auto * Promisify `_refreshFeatures` * Promisify `_injectProjectUsers` * Fix `no-inner-declarations` * Promisify `cloneProject` * Promisify `userProjectsJson` * Promisify `projectEntitiesJson` * Promisify `restoreProject` * Promisify `renameProject` * Additional warning fix * Update unit tests * Fixup `updateProjectSettings`: call jobs inside the Promise.all * Use `expressify(...)` instead of manually call `next(err)` https://github.com/overleaf/internal/pull/18477#discussion_r1613611987 https://github.com/overleaf/internal/pull/18477#discussion_r1613621146 https://github.com/overleaf/internal/pull/18477#discussion_r1613634000 ... * Replace Promise.all by sequencial awaits https://github.com/overleaf/internal/pull/18477#discussion_r1613852746 https://github.com/overleaf/internal/pull/18477#discussion_r1613611987 * Remove manual throws of 500. Let the generic error handler catch them. https://github.com/overleaf/internal/pull/18477#discussion_r1613623446 https://github.com/overleaf/internal/pull/18477#discussion_r1613628955 * Promisify `untrashProject` https://github.com/overleaf/internal/pull/18477#discussion_r1613627783 * Promisify `expireDeletedProjectsAfterDuration` * Promisify `archiveProject` * Promisify `unarchiveProject` * Promisify `trashProject` * Promisify `expireDeletedProject` * Use async `setTimeout` from `timers/promise` https://github.com/overleaf/internal/pull/18477#discussion_r1613843085 * Remove unused `_injectProjectUsers` https://github.com/overleaf/internal/pull/18477#discussion_r1613855766 * Add missing exec in queries (?) Not sure if that makes a real difference but it's more consistent with the rest of the code * Catch floating promises https://github.com/overleaf/internal/pull/18477#discussion_r1613868876 * Replace custom `promiseAuto` by `p-props` from NPM https://github.com/overleaf/internal/pull/18477#discussion_r1613393294 * Downgrade `p-props` to v4. Later versions require ESM * Simplify code around `splitTestAssignments` GitOrigin-RevId: 84d37f7aa9227b5b9acf9eeb5db1b78afc01b6ee --- package-lock.json | 2 + .../Authorization/AuthorizationManager.js | 1 + .../BrandVariations/BrandVariationsHandler.js | 4 + .../src/Features/Project/ProjectController.js | 1270 +++++++---------- services/web/package.json | 1 + services/web/test/unit/bootstrap.js | 1 + .../src/Project/ProjectControllerTests.js | 250 ++-- 7 files changed, 712 insertions(+), 817 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8a8f9e2bfd..0ba5e13ce3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44498,6 +44498,7 @@ "openai": "^4.36.0", "otplib": "^12.0.1", "p-limit": "^2.3.0", + "p-props": "4.0.0", "parse-data-url": "^2.0.0", "passport": "^0.6.0", "passport-google-oauth20": "^2.0.0", @@ -53119,6 +53120,7 @@ "openai": "^4.36.0", "otplib": "^12.0.1", "p-limit": "^2.3.0", + "p-props": "4.0.0", "parse-data-url": "^2.0.0", "passport": "^0.6.0", "passport-google-oauth20": "^2.0.0", diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index cd1c74a922..f4789668ee 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -70,6 +70,7 @@ async function getPublicAccessLevel(projectId) { * * @param userId - The id of the user that wants to access the project. * @param projectId - The id of the project to be accessed. + * @param {string} token * @param {Object} opts * @param {boolean} opts.ignoreSiteAdmin - Do not consider whether the user is * a site admin. diff --git a/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js b/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js index a9c30d07c9..942fa3a6ab 100644 --- a/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js +++ b/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js @@ -4,9 +4,13 @@ const settings = require('@overleaf/settings') const logger = require('@overleaf/logger') const V1Api = require('../V1/V1Api') const sanitizeHtml = require('sanitize-html') +const { promisify } = require('@overleaf/promise-utils') module.exports = { getBrandVariationById, + promises: { + getBrandVariationById: promisify(getBrandVariationById), + }, } function getBrandVariationById(brandVariationId, callback) { diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index ad7cc45934..2e2c3cebf8 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -1,8 +1,10 @@ const _ = require('lodash') const OError = require('@overleaf/o-error') const crypto = require('crypto') -const async = require('async') +const { setTimeout } = require('timers/promises') +const pProps = require('p-props') const logger = require('@overleaf/logger') +const { expressify } = require('@overleaf/promise-utils') const { ObjectId } = require('mongodb') const ProjectDeleter = require('./ProjectDeleter') const ProjectDuplicator = require('./ProjectDuplicator') @@ -25,7 +27,6 @@ const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler') const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter') const ProjectEntityHandler = require('./ProjectEntityHandler') const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher') -const UserGetter = require('../User/UserGetter') const Features = require('../../infrastructure/Features') const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandler') const UserController = require('../User/UserController') @@ -47,7 +48,7 @@ const TutorialHandler = require('../Tutorial/TutorialHandler') * @typedef {import("./types").Project} Project */ -const ProjectController = { +const _ProjectController = { _isInPercentageRollout(rolloutName, objectId, percentage) { if (Settings.bypassPercentageRollouts === true) { return true @@ -58,54 +59,39 @@ const ProjectController = { return counter % 100 < percentage }, - updateProjectSettings(req, res, next) { + async updateProjectSettings(req, res) { const projectId = req.params.Project_id - const jobs = [] - if (req.body.compiler != null) { - jobs.push(callback => - EditorController.setCompiler(projectId, req.body.compiler, callback) - ) + await EditorController.promises.setCompiler(projectId, req.body.compiler) } if (req.body.imageName != null) { - jobs.push(callback => - EditorController.setImageName(projectId, req.body.imageName, callback) + await EditorController.promises.setImageName( + projectId, + req.body.imageName ) } if (req.body.name != null) { - jobs.push(callback => - EditorController.renameProject(projectId, req.body.name, callback) - ) + await EditorController.promises.renameProject(projectId, req.body.name) } if (req.body.spellCheckLanguage != null) { - jobs.push(callback => - EditorController.setSpellCheckLanguage( - projectId, - req.body.spellCheckLanguage, - callback - ) + await EditorController.promises.setSpellCheckLanguage( + projectId, + req.body.spellCheckLanguage ) } if (req.body.rootDocId != null) { - jobs.push(callback => - EditorController.setRootDoc(projectId, req.body.rootDocId, callback) - ) + await EditorController.promises.setRootDoc(projectId, req.body.rootDocId) } - async.series(jobs, error => { - if (error != null) { - return next(error) - } - res.sendStatus(204) - }) + res.sendStatus(204) }, - updateProjectAdminSettings(req, res, next) { + async updateProjectAdminSettings(req, res) { const projectId = req.params.Project_id const user = SessionManager.getSessionUser(req.session) const publicAccessLevel = req.body.publicAccessLevel @@ -120,140 +106,81 @@ const ProjectController = { req.body.publicAccessLevel != null && publicAccessLevels.includes(publicAccessLevel) ) { - const jobs = [] - - jobs.push(callback => - EditorController.setPublicAccessLevel( - projectId, - req.body.publicAccessLevel, - callback - ) + await EditorController.promises.setPublicAccessLevel( + projectId, + req.body.publicAccessLevel ) - jobs.push(callback => - ProjectAuditLogHandler.addEntry( - projectId, - 'toggle-access-level', - user._id, - req.ip, - { publicAccessLevel: req.body.publicAccessLevel, status: 'OK' }, - callback - ) + await ProjectAuditLogHandler.promises.addEntry( + projectId, + 'toggle-access-level', + user._id, + req.ip, + { publicAccessLevel: req.body.publicAccessLevel, status: 'OK' } ) - - async.series(jobs, error => { - if (error != null) { - return next(error) - } - res.sendStatus(204) - }) + res.sendStatus(204) } else { res.sendStatus(500) } }, - deleteProject(req, res) { + async deleteProject(req, res) { const projectId = req.params.Project_id const user = SessionManager.getSessionUser(req.session) - const cb = err => { - if (err != null) { - res.sendStatus(500) - } else { - res.sendStatus(200) - } - } - ProjectDeleter.deleteProject( - projectId, - { deleterUser: user, ipAddress: req.ip }, - cb - ) + await ProjectDeleter.promises.deleteProject(projectId, { + deleterUser: user, + ipAddress: req.ip, + }) + + res.sendStatus(200) }, - archiveProject(req, res, next) { + async archiveProject(req, res) { const projectId = req.params.Project_id const userId = SessionManager.getLoggedInUserId(req.session) - - ProjectDeleter.archiveProject(projectId, userId, function (err) { - if (err != null) { - next(err) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.archiveProject(projectId, userId) + res.sendStatus(200) }, - unarchiveProject(req, res, next) { + async unarchiveProject(req, res) { const projectId = req.params.Project_id const userId = SessionManager.getLoggedInUserId(req.session) - - ProjectDeleter.unarchiveProject(projectId, userId, function (err) { - if (err != null) { - next(err) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.unarchiveProject(projectId, userId) + res.sendStatus(200) }, - trashProject(req, res, next) { + async trashProject(req, res) { const projectId = req.params.project_id const userId = SessionManager.getLoggedInUserId(req.session) - - ProjectDeleter.trashProject(projectId, userId, function (err) { - if (err != null) { - next(err) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.trashProject(projectId, userId) + res.sendStatus(200) }, - untrashProject(req, res, next) { + async untrashProject(req, res) { const projectId = req.params.project_id const userId = SessionManager.getLoggedInUserId(req.session) - - ProjectDeleter.untrashProject(projectId, userId, function (err) { - if (err != null) { - next(err) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.untrashProject(projectId, userId) + res.sendStatus(200) }, - expireDeletedProjectsAfterDuration(req, res) { - ProjectDeleter.expireDeletedProjectsAfterDuration(err => { - if (err != null) { - res.sendStatus(500) - } else { - res.sendStatus(200) - } - }) + async expireDeletedProjectsAfterDuration(_req, res) { + await ProjectDeleter.promises.expireDeletedProjectsAfterDuration() + res.sendStatus(200) }, - expireDeletedProject(req, res, next) { + async expireDeletedProject(req, res) { const { projectId } = req.params - ProjectDeleter.expireDeletedProject(projectId, err => { - if (err != null) { - next(err) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.expireDeletedProject(projectId) + res.sendStatus(200) }, - restoreProject(req, res) { + async restoreProject(req, res) { const projectId = req.params.Project_id - ProjectDeleter.restoreProject(projectId, err => { - if (err != null) { - res.sendStatus(500) - } else { - res.sendStatus(200) - } - }) + await ProjectDeleter.promises.restoreProject(projectId) + res.sendStatus(200) }, - cloneProject(req, res, next) { + async cloneProject(req, res, next) { res.setTimeout(5 * 60 * 1000) // allow extra time for the copy to complete metrics.inc('cloned-project') const projectId = req.params.Project_id @@ -264,36 +191,35 @@ const ProjectController = { } const currentUser = SessionManager.getSessionUser(req.session) const { first_name: firstName, last_name: lastName, email } = currentUser - ProjectDuplicator.duplicate( - currentUser, - projectId, - projectName, - tags, - (err, project) => { - if (err != null) { - OError.tag(err, 'error cloning project', { - projectId, - userId: currentUser._id, - }) - return next(err) - } - res.json({ - name: project.name, - lastUpdated: project.lastUpdated, - project_id: project._id, - owner_ref: project.owner_ref, - owner: { - first_name: firstName, - last_name: lastName, - email, - _id: currentUser._id, - }, - }) - } - ) + try { + const project = await ProjectDuplicator.promises.duplicate( + currentUser, + projectId, + projectName, + tags + ) + res.json({ + name: project.name, + lastUpdated: project.lastUpdated, + project_id: project._id, + owner_ref: project.owner_ref, + owner: { + first_name: firstName, + last_name: lastName, + email, + _id: currentUser._id, + }, + }) + } catch (err) { + OError.tag(err, 'error cloning project', { + projectId, + userId: currentUser._id, + }) + return next(err) + } }, - newProject(req, res, next) { + async newProject(req, res) { const currentUser = SessionManager.getSessionUser(req.session) const { first_name: firstName, @@ -305,91 +231,65 @@ const ProjectController = { req.body.projectName != null ? req.body.projectName.trim() : undefined const { template } = req.body - async.waterfall( - [ - cb => { - if (template === 'example') { - ProjectCreationHandler.createExampleProject(userId, projectName, cb) - } else { - ProjectCreationHandler.createBasicProject(userId, projectName, cb) - } - }, - ], - (err, project) => { - if (err != null) { - return next(err) - } - res.json({ - project_id: project._id, - owner_ref: project.owner_ref, - owner: { - first_name: firstName, - last_name: lastName, - email, - _id: userId, - }, - }) - } - ) + const project = await (template === 'example' + ? ProjectCreationHandler.promises.createExampleProject( + userId, + projectName + ) + : ProjectCreationHandler.promises.createBasicProject(userId, projectName)) + + res.json({ + project_id: project._id, + owner_ref: project.owner_ref, + owner: { + first_name: firstName, + last_name: lastName, + email, + _id: userId, + }, + }) }, - renameProject(req, res, next) { + async renameProject(req, res) { const projectId = req.params.Project_id const newName = req.body.newProjectName - EditorController.renameProject(projectId, newName, err => { - if (err != null) { - return next(err) - } - res.sendStatus(200) - }) + await EditorController.promises.renameProject(projectId, newName) + res.sendStatus(200) }, - userProjectsJson(req, res, next) { + async userProjectsJson(req, res) { const userId = SessionManager.getLoggedInUserId(req.session) - ProjectGetter.findAllUsersProjects( + let projects = await ProjectGetter.promises.findAllUsersProjects( userId, - 'name lastUpdated publicAccesLevel archived trashed owner_ref', - (err, projects) => { - if (err != null) { - return next(err) - } - - // _buildProjectList already converts archived/trashed to booleans so isArchivedOrTrashed should not be used here - projects = ProjectController._buildProjectList(projects, userId) - .filter(p => !(p.archived || p.trashed)) - .map(p => ({ _id: p.id, name: p.name, accessLevel: p.accessLevel })) - - res.json({ projects }) - } + 'name lastUpdated publicAccesLevel archived trashed owner_ref' ) + + // _buildProjectList already converts archived/trashed to booleans so isArchivedOrTrashed should not be used here + projects = ProjectController._buildProjectList(projects, userId) + .filter(p => !(p.archived || p.trashed)) + .map(p => ({ _id: p.id, name: p.name, accessLevel: p.accessLevel })) + + res.json({ projects }) }, - projectEntitiesJson(req, res, next) { + async projectEntitiesJson(req, res) { const projectId = req.params.Project_id - ProjectGetter.getProject(projectId, (err, project) => { - if (err != null) { - return next(err) - } - let docs, files - try { - ;({ docs, files } = - ProjectEntityHandler.getAllEntitiesFromProject(project)) - } catch (err) { - return next(err) - } - const entities = docs - .concat(files) - // Sort by path ascending - .sort((a, b) => (a.path > b.path ? 1 : a.path < b.path ? -1 : 0)) - .map(e => ({ - path: e.path, - type: e.doc != null ? 'doc' : 'file', - })) - res.json({ project_id: projectId, entities }) - }) + const project = await ProjectGetter.promises.getProject(projectId) + + const { docs, files } = + ProjectEntityHandler.getAllEntitiesFromProject(project) + const entities = docs + .concat(files) + // Sort by path ascending + .sort((a, b) => (a.path > b.path ? 1 : a.path < b.path ? -1 : 0)) + .map(e => ({ + path: e.path, + type: e.doc != null ? 'doc' : 'file', + })) + res.json({ project_id: projectId, entities }) }, - loadEditor(req, res, next) { + async loadEditor(req, res, next) { const timer = new metrics.Timer('load-editor') if (!Settings.editorIsOpen) { return res.render('general/closed', { title: 'updating_site' }) @@ -408,445 +308,391 @@ const ProjectController = { const projectId = req.params.Project_id - async.auto( - { - project(cb) { - ProjectGetter.getProject( - projectId, - { - name: 1, - lastUpdated: 1, - track_changes: 1, - owner_ref: 1, - brandVariationId: 1, - overleaf: 1, - tokens: 1, + // should not be used in place of split tests query param overrides (?my-split-test-name=my-variant) + function shouldDisplayFeature(name, variantFlag) { + if (req.query && req.query[name]) { + return req.query[name] === 'true' + } else { + return variantFlag === true + } + } + + try { + const splitTests = [ + !anonymous && 'bib-file-tpr-prompt', + 'compile-log-events', + 'ide-page', + 'null-test-share-modal', + 'paywall-cta', + 'pdf-caching-cached-url-lookup', + 'pdf-caching-mode', + 'pdf-caching-prefetch-large', + 'pdf-caching-prefetching', + 'pdfjs-40', + 'personal-access-token', + 'revert-file', + 'table-generator-promotion', + 'track-pdf-download', + !anonymous && 'writefull-oauth-promotion', + ].filter(Boolean) + + const responses = await pProps( + _.mapValues( + { + splitTestAssignments: async () => { + const assignments = {} + await Promise.all( + splitTests.map(async splitTest => { + assignments[splitTest] = + await SplitTestHandler.promises.getAssignment( + req, + res, + splitTest + ) + }) + ) + return assignments }, - (err, project) => { - if (err != null) { - return cb(err) - } - cb(null, project) - } - ) - }, - user(cb) { - if (userId == null) { - SplitTestSessionHandler.sessionMaintenance(req, null, () => {}) - cb(null, defaultSettingsForAnonymousUser(userId)) - } else { - // Ignore spurious floating promises warning until we promisify - // eslint-disable-next-line @typescript-eslint/no-floating-promises - User.updateOne( - { _id: new ObjectId(userId) }, - { $set: { lastActive: new Date() } }, - {}, - () => {} - ) - // Ignore spurious floating promises warning until we promisify - // eslint-disable-next-line @typescript-eslint/no-floating-promises - User.findById( - userId, - 'email first_name last_name referal_id signUpDate featureSwitches features featuresEpoch refProviders alphaProgram betaProgram isAdmin ace labsProgram completedTutorials writefull', - (err, user) => { + project: () => + ProjectGetter.promises.getProject(projectId, { + name: 1, + lastUpdated: 1, + track_changes: 1, + owner_ref: 1, + brandVariationId: 1, + overleaf: 1, + tokens: 1, + }), + user: async () => { + if (!userId) { + SplitTestSessionHandler.promises + .sessionMaintenance(req, null) + .catch(err => { + logger.error( + { err }, + 'failed to update split test info in session' + ) + }) + return defaultSettingsForAnonymousUser(userId) + } else { + User.updateOne( + { _id: new ObjectId(userId) }, + { $set: { lastActive: new Date() } } + ) + .exec() + .catch(err => { + logger.error( + { err, userId }, + 'failed to update lastActive for user' + ) + }) + + const user = await User.findById( + userId, + 'email first_name last_name referal_id signUpDate featureSwitches features featuresEpoch refProviders alphaProgram betaProgram isAdmin ace labsProgram completedTutorials writefull' + ).exec() // Handle case of deleted user - if (user == null) { + if (!user) { UserController.logout(req, res, next) return } - if (err) { - return cb(err) - } - logger.debug({ projectId, userId }, 'got user') - SplitTestSessionHandler.sessionMaintenance(req, user, () => {}) - if (FeaturesUpdater.featuresEpochIsCurrent(user)) { - return cb(null, user) - } - ProjectController._refreshFeatures(req, user, cb) - } - ) - } - }, - userHasInstitutionLicence(cb) { - if (!userId) { - return cb(null, false) - } - InstitutionsFeatures.hasLicence(userId, (error, hasLicence) => { - if (error) { - // Don't fail if we can't get affiliation licences - return cb(null, false) - } - cb(null, hasLicence) - }) - }, - learnedWords(cb) { - if (!userId) { - return cb(null, []) - } - SpellingHandler.getUserDictionary(userId, cb) - }, - subscription(cb) { - if (userId == null) { - return cb() - } - SubscriptionLocator.getUsersSubscription(userId, cb) - }, - userIsMemberOfGroupSubscription(cb) { - if (sessionUser == null) { - return cb(null, false) - } - LimitationsManager.userIsMemberOfGroupSubscription( - sessionUser, - (error, isMember) => { - cb(error, isMember) - } - ) - }, - activate(cb) { - InactiveProjectManager.reactivateProjectIfRequired(projectId, cb) - }, - markAsOpened(cb) { - // don't need to wait for this to complete - ProjectUpdateHandler.markAsOpened(projectId, () => {}) - cb() - }, - isTokenMember(cb) { - if (userId == null) { - return cb() - } - CollaboratorsGetter.userIsTokenMember(userId, projectId, cb) - }, - isInvitedMember(cb) { - CollaboratorsGetter.isUserInvitedMemberOfProject( - userId, - projectId, - cb - ) - }, - brandVariation: [ - 'project', - (results, cb) => { - if ( - (results.project != null - ? results.project.brandVariationId - : undefined) == null - ) { - return cb() - } - BrandVariationsHandler.getBrandVariationById( - results.project.brandVariationId, - (error, brandVariationDetails) => cb(error, brandVariationDetails) - ) - }, - ], - flushToTpds: cb => { - TpdsProjectFlusher.flushProjectToTpdsIfNeeded(projectId, cb) - }, - sharingModalNullTest(cb) { - // null test targeting logged in users, for front-end side - SplitTestHandler.getAssignment(req, res, 'null-test-share-modal', cb) - }, - pdfjsAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'pdfjs-40', cb) - }, - trackPdfDownloadAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'track-pdf-download', cb) - }, - pdfCachingModeAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'pdf-caching-mode', cb) - }, - pdfCachingPrefetchingAssignment(cb) { - SplitTestHandler.getAssignment( - req, - res, - 'pdf-caching-prefetching', - cb - ) - }, - pdfCachingPrefetchLargeAssignment(cb) { - SplitTestHandler.getAssignment( - req, - res, - 'pdf-caching-prefetch-large', - cb - ) - }, - pdfCachingCachedUrlLookupAssignment(cb) { - SplitTestHandler.getAssignment( - req, - res, - 'pdf-caching-cached-url-lookup', - cb - ) - }, - tableGeneratorPromotionAssignment(cb) { - SplitTestHandler.getAssignment( - req, - res, - 'table-generator-promotion', - cb - ) - }, - personalAccessTokenAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'personal-access-token', cb) - }, - idePageAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'ide-page', cb) - }, - writefullIntegrationAssignment(cb) { - if (anonymous) { - // Disable allocation to split test for non-logged-in users. - // The in-editor promotion is only relevant for logged-in users. - cb() - } else { - SplitTestHandler.getAssignment( - req, - res, - 'writefull-oauth-promotion', - cb - ) - } - }, - tprPromptAssignment(cb) { - if (anonymous) { - cb() - } else { - SplitTestHandler.getAssignment(req, res, 'bib-file-tpr-prompt', cb) - } - }, - compileLogEventsAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'compile-log-events', cb) - }, - paywallCtaAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'paywall-cta', cb) - }, - revertFileAssignment(cb) { - SplitTestHandler.getAssignment(req, res, 'revert-file', cb) - }, - projectTags(cb) { - if (!userId) { - return cb(null, []) - } - TagsHandler.getTagsForProject(userId, projectId, cb) - }, - }, - ( - err, - { - project, - user, - userHasInstitutionLicence, - learnedWords, - subscription, - userIsMemberOfGroupSubscription, - isTokenMember, - isInvitedMember, - brandVariation, - pdfjsAssignment, - idePageAssignment, - personalAccessTokenAssignment, - projectTags, - } - ) => { - if (err != null) { - OError.tag(err, 'error getting details for project page') - return next(err) - } - const anonRequestToken = TokenAccessHandler.getRequestToken( - req, - projectId - ) - const allowedImageNames = ProjectHelper.getAllowedImagesForUser(user) - AuthorizationManager.getPrivilegeLevelForProject( + logger.debug({ projectId, userId }, 'got user') + SplitTestSessionHandler.promises + .sessionMaintenance(req, user) + .catch(err => { + logger.error( + { err }, + 'failed to update split test info in session' + ) + }) + + if (FeaturesUpdater.featuresEpochIsCurrent(user)) { + return user + } + + return await ProjectController._refreshFeatures(req, user) + } + }, + userHasInstitutionLicence: async () => { + if (!userId) { + return false + } + try { + return await InstitutionsFeatures.promises.hasLicence(userId) + } catch { + // Don't fail if we can't get affiliation licences + return false + } + }, + learnedWords() { + if (!userId) { + return [] + } + return SpellingHandler.promises.getUserDictionary(userId) + }, + subscription() { + if (!userId) { + return + } + return SubscriptionLocator.promises.getUsersSubscription(userId) + }, + userIsMemberOfGroupSubscription() { + if (!sessionUser) { + return false + } + return LimitationsManager.promises.userIsMemberOfGroupSubscription( + sessionUser + ) + }, + activate() { + return InactiveProjectManager.promises.reactivateProjectIfRequired( + projectId + ) + }, + markAsOpened() { + // don't need to wait for this to complete + ProjectUpdateHandler.promises + .markAsOpened(projectId) + .catch(err => { + logger.error( + { err, projectId }, + 'failed to mark project as opened' + ) + }) + }, + isTokenMember() { + if (!userId) { + return + } + return CollaboratorsGetter.promises.userIsTokenMember( + userId, + projectId + ) + }, + isInvitedMember() { + return CollaboratorsGetter.promises.isUserInvitedMemberOfProject( + userId, + projectId + ) + }, + flushToTpds: () => { + return TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded( + projectId + ) + }, + projectTags() { + if (!userId) { + return [] + } + return TagsHandler.promises.getTagsForProject(userId, projectId) + }, + }, + promise => promise() + ) + ) + + const { + project, + user, + userHasInstitutionLicence, + learnedWords, + subscription, + userIsMemberOfGroupSubscription, + isTokenMember, + isInvitedMember, + splitTestAssignments, + projectTags, + } = responses + + const brandVariation = project?.brandVariationId + ? await BrandVariationsHandler.promises.getBrandVariationById( + project.brandVariationId + ) + : undefined + + const anonRequestToken = TokenAccessHandler.getRequestToken( + req, + projectId + ) + const allowedImageNames = ProjectHelper.getAllowedImagesForUser(user) + + const privilegeLevel = + await AuthorizationManager.promises.getPrivilegeLevelForProject( userId, projectId, - anonRequestToken, - (error, privilegeLevel) => { - let allowedFreeTrial = true - if (error != null) { - return next(error) - } - if ( - privilegeLevel == null || - privilegeLevel === PrivilegeLevels.NONE - ) { - return res.sendStatus(401) - } + anonRequestToken + ) - if (subscription != null) { - allowedFreeTrial = false - } + let allowedFreeTrial = true - let wsUrl = Settings.wsUrl - let metricName = 'load-editor-ws' - if (user.betaProgram && Settings.wsUrlBeta !== undefined) { - wsUrl = Settings.wsUrlBeta - metricName += '-beta' - } else if ( - Settings.wsUrlV2 && - Settings.wsUrlV2Percentage > 0 && - (new ObjectId(projectId).getTimestamp() / 1000) % 100 < - Settings.wsUrlV2Percentage - ) { - wsUrl = Settings.wsUrlV2 - metricName += '-v2' - } - if (req.query && req.query.ws === 'fallback') { - // `?ws=fallback` will connect to the bare origin, and ignore - // the custom wsUrl. Hence it must load the client side - // javascript from there too. - // Not resetting it here would possibly load a socket.io v2 - // client and connect to a v0 endpoint. - wsUrl = undefined - metricName += '-fallback' - } - metrics.inc(metricName) + if (privilegeLevel == null || privilegeLevel === PrivilegeLevels.NONE) { + return res.sendStatus(401) + } - if (userId) { - AnalyticsManager.recordEventForUserInBackground( - userId, - 'project-opened', - { - projectId: project._id, - } - ) - } + if (subscription != null) { + allowedFreeTrial = false + } - // should not be used in place of split tests query param overrides (?my-split-test-name=my-variant) - function shouldDisplayFeature(name, variantFlag) { - if (req.query && req.query[name]) { - return req.query[name] === 'true' - } else { - return variantFlag === true - } - } + let wsUrl = Settings.wsUrl + let metricName = 'load-editor-ws' + if (user.betaProgram && Settings.wsUrlBeta !== undefined) { + wsUrl = Settings.wsUrlBeta + metricName += '-beta' + } else if ( + Settings.wsUrlV2 && + Settings.wsUrlV2Percentage > 0 && + (new ObjectId(projectId).getTimestamp() / 1000) % 100 < + Settings.wsUrlV2Percentage + ) { + wsUrl = Settings.wsUrlV2 + metricName += '-v2' + } + if (req.query && req.query.ws === 'fallback') { + // `?ws=fallback` will connect to the bare origin, and ignore + // the custom wsUrl. Hence it must load the client side + // javascript from there too. + // Not resetting it here would possibly load a socket.io v2 + // client and connect to a v0 endpoint. + wsUrl = undefined + metricName += '-fallback' + } + metrics.inc(metricName) - const isAdminOrTemplateOwner = - hasAdminAccess(user) || Settings.templates?.user_id === userId - const showTemplatesServerPro = - Features.hasFeature('templates-server-pro') && - isAdminOrTemplateOwner - - const debugPdfDetach = shouldDisplayFeature('debug_pdf_detach') - - const detachRole = req.params.detachRole - - const showSymbolPalette = - !Features.hasFeature('saas') || - (user.features && user.features.symbolPalette) - - // Persistent upgrade prompts - // in header & in share project modal - const showUpgradePrompt = - Features.hasFeature('saas') && - userId && - !subscription && - !userIsMemberOfGroupSubscription && - !userHasInstitutionLicence - - const showPersonalAccessToken = - userId && - (!Features.hasFeature('saas') || - req.query?.personal_access_token === 'true') - - const optionalPersonalAccessToken = - userId && - !showPersonalAccessToken && - personalAccessTokenAssignment.variant === 'enabled' // `?personal-access-token=enabled` - - const idePageReact = idePageAssignment.variant === 'react' - - const template = - detachRole === 'detached' - ? // TODO: Create React version of detached page - 'project/editor_detached' - : idePageReact - ? 'project/ide-react' - : 'project/editor' - - res.render(template, { - title: project.name, - priority_title: true, - bodyClasses: ['editor'], - project_id: project._id, - projectName: project.name, - user: { - id: userId, - email: user.email, - first_name: user.first_name, - last_name: user.last_name, - referal_id: user.referal_id, - signUpDate: user.signUpDate, - allowedFreeTrial, - featureSwitches: user.featureSwitches, - features: user.features, - refProviders: _.mapValues(user.refProviders, Boolean), - writefull: { - enabled: Boolean(user.writefull?.enabled), - }, - alphaProgram: user.alphaProgram, - betaProgram: user.betaProgram, - labsProgram: user.labsProgram, - inactiveTutorials: TutorialHandler.getInactiveTutorials(user), - isAdmin: hasAdminAccess(user), - }, - userSettings: { - mode: user.ace.mode, - editorTheme: user.ace.theme, - fontSize: user.ace.fontSize, - autoComplete: user.ace.autoComplete, - autoPairDelimiters: user.ace.autoPairDelimiters, - pdfViewer: user.ace.pdfViewer, - syntaxValidation: user.ace.syntaxValidation, - fontFamily: user.ace.fontFamily || 'lucida', - lineHeight: user.ace.lineHeight || 'normal', - overallTheme: user.ace.overallTheme, - }, - privilegeLevel, - anonymous, - isTokenMember, - isRestrictedTokenMember: AuthorizationManager.isRestrictedUser( - userId, - privilegeLevel, - isTokenMember, - isInvitedMember - ), - languages: Settings.languages, - learnedWords, - editorThemes: THEME_LIST, - legacyEditorThemes: LEGACY_THEME_LIST, - maxDocLength: Settings.max_doc_length, - brandVariation, - allowedImageNames, - gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl, - gitBridgeEnabled: Features.hasFeature('git-bridge'), - wsUrl, - showSupport: Features.hasFeature('support'), - showTemplatesServerPro, - pdfjsVariant: pdfjsAssignment.variant, - debugPdfDetach, - showSymbolPalette, - symbolPaletteAvailable: Features.hasFeature('symbol-palette'), - detachRole, - metadata: { viewport: false }, - showUpgradePrompt, - fixedSizeDocument: true, - useOpenTelemetry: Settings.useOpenTelemetryClient, - idePageReact, - showPersonalAccessToken, - optionalPersonalAccessToken, - hasTrackChangesFeature: Features.hasFeature('track-changes'), - projectTags, - }) - timer.done() + if (userId) { + AnalyticsManager.recordEventForUserInBackground( + userId, + 'project-opened', + { + projectId: project._id, } ) } - ) + + const isAdminOrTemplateOwner = + hasAdminAccess(user) || Settings.templates?.user_id === userId + const showTemplatesServerPro = + Features.hasFeature('templates-server-pro') && isAdminOrTemplateOwner + + const debugPdfDetach = shouldDisplayFeature('debug_pdf_detach') + + const detachRole = req.params.detachRole + + const showSymbolPalette = + !Features.hasFeature('saas') || + (user.features && user.features.symbolPalette) + + // Persistent upgrade prompts + // in header & in share project modal + const showUpgradePrompt = + Features.hasFeature('saas') && + userId && + !subscription && + !userIsMemberOfGroupSubscription && + !userHasInstitutionLicence + + const showPersonalAccessToken = + userId && + (!Features.hasFeature('saas') || + req.query?.personal_access_token === 'true') + + const optionalPersonalAccessToken = + userId && + !showPersonalAccessToken && + splitTestAssignments['personal-access-token'].variant === 'enabled' // `?personal-access-token=enabled` + + const idePageReact = splitTestAssignments['ide-page'].variant === 'react' + + const template = + detachRole === 'detached' + ? // TODO: Create React version of detached page + 'project/editor_detached' + : idePageReact + ? 'project/ide-react' + : 'project/editor' + + res.render(template, { + title: project.name, + priority_title: true, + bodyClasses: ['editor'], + project_id: project._id, + projectName: project.name, + user: { + id: userId, + email: user.email, + first_name: user.first_name, + last_name: user.last_name, + referal_id: user.referal_id, + signUpDate: user.signUpDate, + allowedFreeTrial, + featureSwitches: user.featureSwitches, + features: user.features, + refProviders: _.mapValues(user.refProviders, Boolean), + writefull: { + enabled: Boolean(user.writefull?.enabled), + }, + alphaProgram: user.alphaProgram, + betaProgram: user.betaProgram, + labsProgram: user.labsProgram, + inactiveTutorials: TutorialHandler.getInactiveTutorials(user), + isAdmin: hasAdminAccess(user), + }, + userSettings: { + mode: user.ace.mode, + editorTheme: user.ace.theme, + fontSize: user.ace.fontSize, + autoComplete: user.ace.autoComplete, + autoPairDelimiters: user.ace.autoPairDelimiters, + pdfViewer: user.ace.pdfViewer, + syntaxValidation: user.ace.syntaxValidation, + fontFamily: user.ace.fontFamily || 'lucida', + lineHeight: user.ace.lineHeight || 'normal', + overallTheme: user.ace.overallTheme, + }, + privilegeLevel, + anonymous, + isTokenMember, + isRestrictedTokenMember: AuthorizationManager.isRestrictedUser( + userId, + privilegeLevel, + isTokenMember, + isInvitedMember + ), + languages: Settings.languages, + learnedWords, + editorThemes: THEME_LIST, + legacyEditorThemes: LEGACY_THEME_LIST, + maxDocLength: Settings.max_doc_length, + brandVariation, + allowedImageNames, + gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl, + gitBridgeEnabled: Features.hasFeature('git-bridge'), + wsUrl, + showSupport: Features.hasFeature('support'), + showTemplatesServerPro, + pdfjsVariant: splitTestAssignments['pdfjs-40'].variant, + debugPdfDetach, + showSymbolPalette, + symbolPaletteAvailable: Features.hasFeature('symbol-palette'), + detachRole, + metadata: { viewport: false }, + showUpgradePrompt, + fixedSizeDocument: true, + useOpenTelemetry: Settings.useOpenTelemetryClient, + idePageReact, + showPersonalAccessToken, + optionalPersonalAccessToken, + hasTrackChangesFeature: Features.hasFeature('track-changes'), + projectTags, + }) + timer.done() + } catch (err) { + OError.tag(err, 'error getting details for project page') + return next(err) + } }, - _refreshFeatures(req, user, callback) { + async _refreshFeatures(req, user) { // If the feature refresh has failed in this session, don't retry // it - require the user to log in again. if (req.session.feature_refresh_failed) { @@ -854,29 +700,41 @@ const ProjectController = { path: 'load-editor', status: 'skipped', }) - return callback(null, user) + return user } // If the refresh takes too long then return the current // features. Note that the user.features property may still be - // updated in the background after the callback is called. - callback = _.once(callback) - const refreshTimeoutHandler = setTimeout(() => { - req.session.feature_refresh_failed = { reason: 'timeout', at: new Date() } + // updated in the background after the promise is resolved. + const abortController = new AbortController() + const refreshTimeoutHandler = async () => { + await setTimeout(5000, { signal: abortController.signal }) + req.session.feature_refresh_failed = { + reason: 'timeout', + at: new Date(), + } metrics.inc('features-refresh', 1, { path: 'load-editor', status: 'timeout', }) - callback(null, user) - }, 5000) + return user + } + // try to refresh user features now const timer = new metrics.Timer('features-refresh-on-load-editor') - FeaturesUpdater.refreshFeatures( - user._id, - 'load-editor', - (err, features) => { - clearTimeout(refreshTimeoutHandler) - timer.done() - if (err) { + + return Promise.race([ + refreshTimeoutHandler(), + (async () => { + try { + user.features = await FeaturesUpdater.promises.refreshFeatures( + user._id, + 'load-editor' + ) + metrics.inc('features-refresh', 1, { + path: 'load-editor', + status: 'success', + }) + } catch (err) { // keep a record to prevent unneceary retries and leave // the original features unmodified if the refresh failed req.session.feature_refresh_failed = { @@ -887,16 +745,12 @@ const ProjectController = { path: 'load-editor', status: 'error', }) - } else { - user.features = features - metrics.inc('features-refresh', 1, { - path: 'load-editor', - status: 'success', - }) } - callback(null, user) - } - ) + abortController.abort() + timer.done() + return user + })(), + ]) }, _buildProjectList(allProjects, userId) { @@ -997,51 +851,6 @@ const ProjectController = { return model }, - _injectProjectUsers(projects, callback) { - const users = {} - for (const project of projects) { - if (project.owner_ref != null) { - users[project.owner_ref.toString()] = true - } - if (project.lastUpdatedBy != null) { - users[project.lastUpdatedBy.toString()] = true - } - } - - const userIds = Object.keys(users) - async.eachSeries( - userIds, - (userId, cb) => { - UserGetter.getUser( - userId, - { first_name: 1, last_name: 1, email: 1 }, - (error, user) => { - if (error != null) { - return cb(error) - } - users[userId] = user - cb() - } - ) - }, - error => { - if (error != null) { - return callback(error) - } - for (const project of projects) { - if (project.owner_ref != null) { - project.owner = users[project.owner_ref.toString()] - } - if (project.lastUpdatedBy != null) { - project.lastUpdatedBy = - users[project.lastUpdatedBy.toString()] || null - } - } - callback(null, projects) - } - ) - }, - _buildPortalTemplatesList(affiliations) { if (affiliations == null) { affiliations = [] @@ -1136,4 +945,33 @@ const LEGACY_THEME_LIST = [ 'xcode', ] +const ProjectController = { + archiveProject: expressify(_ProjectController.archiveProject), + cloneProject: expressify(_ProjectController.cloneProject), + deleteProject: expressify(_ProjectController.deleteProject), + expireDeletedProject: expressify(_ProjectController.expireDeletedProject), + expireDeletedProjectsAfterDuration: expressify( + _ProjectController.expireDeletedProjectsAfterDuration + ), + loadEditor: expressify(_ProjectController.loadEditor), + newProject: expressify(_ProjectController.newProject), + projectEntitiesJson: expressify(_ProjectController.projectEntitiesJson), + renameProject: expressify(_ProjectController.renameProject), + restoreProject: expressify(_ProjectController.restoreProject), + trashProject: expressify(_ProjectController.trashProject), + unarchiveProject: expressify(_ProjectController.unarchiveProject), + untrashProject: expressify(_ProjectController.untrashProject), + updateProjectAdminSettings: expressify( + _ProjectController.updateProjectAdminSettings + ), + updateProjectSettings: expressify(_ProjectController.updateProjectSettings), + userProjectsJson: expressify(_ProjectController.userProjectsJson), + + _buildProjectList: _ProjectController._buildProjectList, + _buildProjectViewModel: _ProjectController._buildProjectViewModel, + _injectProjectUsers: _ProjectController._injectProjectUsers, + _isInPercentageRollout: _ProjectController._isInPercentageRollout, + _refreshFeatures: _ProjectController._refreshFeatures, +} + module.exports = ProjectController diff --git a/services/web/package.json b/services/web/package.json index f56022ff41..622b521b52 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -142,6 +142,7 @@ "openai": "^4.36.0", "otplib": "^12.0.1", "p-limit": "^2.3.0", + "p-props": "4.0.0", "parse-data-url": "^2.0.0", "passport": "^0.6.0", "passport-google-oauth20": "^2.0.0", diff --git a/services/web/test/unit/bootstrap.js b/services/web/test/unit/bootstrap.js index 65fb238eb8..e0488c6b06 100644 --- a/services/web/test/unit/bootstrap.js +++ b/services/web/test/unit/bootstrap.js @@ -50,6 +50,7 @@ SandboxedModule.configure({ ignoreMissing: true, requires: getSandboxedModuleRequires(), globals: { + AbortController, AbortSignal, Buffer, Promise, diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index 0afd9dd5e7..bb3400aeff 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -38,48 +38,73 @@ describe('ProjectController', function () { } this.token = 'some-token' this.ProjectDeleter = { - deleteProject: sinon.stub().callsArg(2), - restoreProject: sinon.stub().callsArg(1), + promises: { + deleteProject: sinon.stub().resolves(), + restoreProject: sinon.stub().resolves(), + }, findArchivedProjects: sinon.stub(), } this.ProjectDuplicator = { - duplicate: sinon.stub().callsArgWith(4, null, { _id: this.project_id }), + promises: { + duplicate: sinon.stub().resolves({ _id: this.project_id }), + }, } this.ProjectCreationHandler = { - createExampleProject: sinon - .stub() - .callsArgWith(2, null, { _id: this.project_id }), - createBasicProject: sinon - .stub() - .callsArgWith(2, null, { _id: this.project_id }), + promises: { + createExampleProject: sinon.stub().resolves({ _id: this.project_id }), + createBasicProject: sinon.stub().resolves({ _id: this.project_id }), + }, + } + this.SubscriptionLocator = { + promises: { + getUsersSubscription: sinon.stub().resolves(), + }, } - this.SubscriptionLocator = { getUsersSubscription: sinon.stub() } this.LimitationsManager = { hasPaidSubscription: sinon.stub(), - userIsMemberOfGroupSubscription: sinon - .stub() - .callsArgWith(1, null, false), + promises: { + userIsMemberOfGroupSubscription: sinon.stub().resolves(false), + }, } this.TagsHandler = { - getTagsForProject: sinon.stub().callsArgWith(2, null, [ - { - name: 'test', - project_ids: [this.project_id], - }, - ]), + promises: { + getTagsForProject: sinon.stub().resolves([ + { + name: 'test', + project_ids: [this.project_id], + }, + ]), + }, addProjectToTags: sinon.stub(), } - this.UserModel = { findById: sinon.stub(), updateOne: sinon.stub() } + this.UserModel = { + findById: sinon.stub().returns({ exec: sinon.stub().resolves() }), + updateOne: sinon.stub().returns({ exec: sinon.stub().resolves() }), + } this.AuthorizationManager = { - getPrivilegeLevelForProject: sinon.stub(), + promises: { + getPrivilegeLevelForProject: sinon.stub(), + }, isRestrictedUser: sinon.stub().returns(false), } - this.EditorController = { renameProject: sinon.stub() } - this.InactiveProjectManager = { reactivateProjectIfRequired: sinon.stub() } - this.ProjectUpdateHandler = { markAsOpened: sinon.stub() } + this.EditorController = { + promises: { + renameProject: sinon.stub().resolves(), + }, + } + this.InactiveProjectManager = { + promises: { reactivateProjectIfRequired: sinon.stub() }, + } + this.ProjectUpdateHandler = { + promises: { + markAsOpened: sinon.stub().resolves(), + }, + } this.ProjectGetter = { - findAllUsersProjects: sinon.stub(), - getProject: sinon.stub(), + promises: { + findAllUsersProjects: sinon.stub().resolves(), + getProject: sinon.stub().resolves(), + }, } this.ProjectHelper = { isArchived: sinon.stub(), @@ -88,7 +113,6 @@ describe('ProjectController', function () { getAllowedImagesForUser: sinon.stub().returns([]), } this.SessionManager = { - getLoggedInUser: sinon.stub().callsArgWith(1, null, this.user), getLoggedInUserId: sinon.stub().returns(this.user._id), getSessionUser: sinon.stub().returns(this.user), isUserLoggedIn: sinon.stub().returns(true), @@ -100,30 +124,36 @@ describe('ProjectController', function () { getRequestToken: sinon.stub().returns(this.token), } this.CollaboratorsGetter = { - userIsTokenMember: sinon.stub().callsArgWith(2, null, false), - isUserInvitedMemberOfProject: sinon.stub().callsArgWith(2, null, true), + promises: { + userIsTokenMember: sinon.stub().resolves(false), + isUserInvitedMemberOfProject: sinon.stub().resolves(true), + }, } this.ProjectEntityHandler = {} this.UserGetter = { getUserFullEmails: sinon.stub().yields(null, []), - getUser: sinon - .stub() - .callsArgWith(2, null, { lastLoginIp: '192.170.18.2' }), + getUser: sinon.stub().resolves({ lastLoginIp: '192.170.18.2' }), } this.Features = { hasFeature: sinon.stub(), } this.FeaturesUpdater = { featuresEpochIsCurrent: sinon.stub().returns(true), - refreshFeatures: sinon.stub().yields(null, this.user), + promises: { + refreshFeatures: sinon.stub().resolves(this.user), + }, } this.BrandVariationsHandler = { - getBrandVariationById: sinon - .stub() - .callsArgWith(1, null, this.brandVariationDetails), + promises: { + getBrandVariationById: sinon + .stub() + .resolves(this.brandVariationDetails), + }, } this.TpdsProjectFlusher = { - flushProjectToTpdsIfNeeded: sinon.stub().yields(), + promises: { + flushProjectToTpdsIfNeeded: sinon.stub().resolves(), + }, } this.Metrics = { Timer: class { @@ -138,10 +168,14 @@ describe('ProjectController', function () { getAssignment: sinon.stub().yields(null, { variant: 'default' }), } this.SplitTestSessionHandler = { - sessionMaintenance: sinon.stub().yields(), + promises: { + sessionMaintenance: sinon.stub().resolves(), + }, } this.InstitutionsFeatures = { - hasLicence: sinon.stub().callsArgWith(1, null, false), + promises: { + hasLicence: sinon.stub().resolves(false), + }, } this.SubscriptionViewModelBuilder = { getBestSubscription: sinon.stub().yields(null, { type: 'free' }), @@ -150,7 +184,9 @@ describe('ProjectController', function () { getSurvey: sinon.stub().yields(null, {}), } this.ProjectAuditLogHandler = { - addEntry: sinon.stub().yields(), + promises: { + addEntry: sinon.stub().resolves(), + }, } this.TutorialHandler = { getInactiveTutorials: sinon.stub().returns([]), @@ -195,7 +231,9 @@ describe('ProjectController', function () { '../Subscription/SubscriptionViewModelBuilder': this.SubscriptionViewModelBuilder, '../Spelling/SpellingHandler': { - getUserDictionary: sinon.stub().yields(null, []), + promises: { + getUserDictionary: sinon.stub().resolves([]), + }, }, '../Institutions/InstitutionsFeatures': this.InstitutionsFeatures, '../Survey/SurveyHandler': this.SurveyHandler, @@ -235,10 +273,10 @@ describe('ProjectController', function () { describe('updateProjectSettings', function () { it('should update the name', function (done) { - this.EditorController.renameProject = sinon.stub().callsArg(2) + this.EditorController.promises.renameProject = sinon.stub().resolves() this.req.body = { name: (this.name = 'New name') } this.res.sendStatus = code => { - this.EditorController.renameProject + this.EditorController.promises.renameProject .calledWith(this.project_id, this.name) .should.equal(true) code.should.equal(204) @@ -248,10 +286,10 @@ describe('ProjectController', function () { }) it('should update the compiler', function (done) { - this.EditorController.setCompiler = sinon.stub().callsArg(2) + this.EditorController.promises.setCompiler = sinon.stub().resolves() this.req.body = { compiler: (this.compiler = 'pdflatex') } this.res.sendStatus = code => { - this.EditorController.setCompiler + this.EditorController.promises.setCompiler .calledWith(this.project_id, this.compiler) .should.equal(true) code.should.equal(204) @@ -261,10 +299,10 @@ describe('ProjectController', function () { }) it('should update the imageName', function (done) { - this.EditorController.setImageName = sinon.stub().callsArg(2) + this.EditorController.promises.setImageName = sinon.stub().resolves() this.req.body = { imageName: (this.imageName = 'texlive-1234.5') } this.res.sendStatus = code => { - this.EditorController.setImageName + this.EditorController.promises.setImageName .calledWith(this.project_id, this.imageName) .should.equal(true) code.should.equal(204) @@ -274,10 +312,12 @@ describe('ProjectController', function () { }) it('should update the spell check language', function (done) { - this.EditorController.setSpellCheckLanguage = sinon.stub().callsArg(2) + this.EditorController.promises.setSpellCheckLanguage = sinon + .stub() + .resolves() this.req.body = { spellCheckLanguage: (this.languageCode = 'fr') } this.res.sendStatus = code => { - this.EditorController.setSpellCheckLanguage + this.EditorController.promises.setSpellCheckLanguage .calledWith(this.project_id, this.languageCode) .should.equal(true) code.should.equal(204) @@ -287,10 +327,10 @@ describe('ProjectController', function () { }) it('should update the root doc', function (done) { - this.EditorController.setRootDoc = sinon.stub().callsArg(2) + this.EditorController.promises.setRootDoc = sinon.stub().resolves() this.req.body = { rootDocId: (this.rootDocId = 'root-doc-id') } this.res.sendStatus = code => { - this.EditorController.setRootDoc + this.EditorController.promises.setRootDoc .calledWith(this.project_id, this.rootDocId) .should.equal(true) code.should.equal(204) @@ -302,12 +342,14 @@ describe('ProjectController', function () { describe('updateProjectAdminSettings', function () { it('should update the public access level', function (done) { - this.EditorController.setPublicAccessLevel = sinon.stub().callsArg(2) + this.EditorController.promises.setPublicAccessLevel = sinon + .stub() + .resolves() this.req.body = { publicAccessLevel: 'readOnly', } this.res.sendStatus = code => { - this.EditorController.setPublicAccessLevel + this.EditorController.promises.setPublicAccessLevel .calledWith(this.project_id, 'readOnly') .should.equal(true) code.should.equal(204) @@ -317,12 +359,14 @@ describe('ProjectController', function () { }) it('should record the change in the project audit log', function (done) { - this.EditorController.setPublicAccessLevel = sinon.stub().callsArg(2) + this.EditorController.promises.setPublicAccessLevel = sinon + .stub() + .resolves() this.req.body = { publicAccessLevel: 'readOnly', } this.res.sendStatus = code => { - this.ProjectAuditLogHandler.addEntry + this.ProjectAuditLogHandler.promises.addEntry .calledWith( this.project_id, 'toggle-access-level', @@ -343,7 +387,7 @@ describe('ProjectController', function () { describe('deleteProject', function () { it('should call the project deleter', function (done) { this.res.sendStatus = code => { - this.ProjectDeleter.deleteProject + this.ProjectDeleter.promises.deleteProject .calledWith(this.project_id, { deleterUser: this.user, ipAddress: this.req.ip, @@ -359,7 +403,7 @@ describe('ProjectController', function () { describe('restoreProject', function () { it('should tell the project deleter', function (done) { this.res.sendStatus = code => { - this.ProjectDeleter.restoreProject + this.ProjectDeleter.promises.restoreProject .calledWith(this.project_id) .should.equal(true) code.should.equal(200) @@ -372,7 +416,7 @@ describe('ProjectController', function () { describe('cloneProject', function () { it('should call the project duplicator', function (done) { this.res.json = json => { - this.ProjectDuplicator.duplicate + this.ProjectDuplicator.promises.duplicate .calledWith(this.user, this.project_id, this.projectName) .should.equal(true) json.project_id.should.equal(this.project_id) @@ -386,10 +430,10 @@ describe('ProjectController', function () { it('should call the projectCreationHandler with createExampleProject', function (done) { this.req.body.template = 'example' this.res.json = json => { - this.ProjectCreationHandler.createExampleProject + this.ProjectCreationHandler.promises.createExampleProject .calledWith(this.user._id, this.projectName) .should.equal(true) - this.ProjectCreationHandler.createBasicProject.called.should.equal( + this.ProjectCreationHandler.promises.createBasicProject.called.should.equal( false ) done() @@ -400,10 +444,10 @@ describe('ProjectController', function () { it('should call the projectCreationHandler with createBasicProject', function (done) { this.req.body.template = 'basic' this.res.json = json => { - this.ProjectCreationHandler.createExampleProject.called.should.equal( + this.ProjectCreationHandler.promises.createExampleProject.called.should.equal( false ) - this.ProjectCreationHandler.createBasicProject + this.ProjectCreationHandler.promises.createBasicProject .calledWith(this.user._id, this.projectName) .should.equal(true) done() @@ -419,10 +463,10 @@ describe('ProjectController', function () { }) it('should call the editor controller', function (done) { - this.EditorController.renameProject.callsArgWith(2) + this.EditorController.promises.renameProject.resolves() this.res.sendStatus = code => { code.should.equal(200) - this.EditorController.renameProject + this.EditorController.promises.renameProject .calledWith(this.project_id, this.newProjectName) .should.equal(true) done() @@ -432,8 +476,7 @@ describe('ProjectController', function () { it('should send an error to next() if there is a problem', function (done) { let error - this.EditorController.renameProject.callsArgWith( - 2, + this.EditorController.promises.renameProject.rejects( (error = new Error('problem')) ) const next = e => { @@ -470,17 +513,17 @@ describe('ProjectController', function () { zotero: { encrypted: 'bbbb' }, }, } - this.ProjectGetter.getProject.callsArgWith(2, null, this.project) - this.UserModel.findById.callsArgWith(2, null, this.user) - this.SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, {}) - this.AuthorizationManager.getPrivilegeLevelForProject.callsArgWith( - 3, - null, + this.ProjectGetter.promises.getProject.resolves(this.project) + this.UserModel.findById.returns({ + exec: sinon.stub().resolves(this.user), + }) + this.SubscriptionLocator.promises.getUsersSubscription.resolves({}) + this.AuthorizationManager.promises.getPrivilegeLevelForProject.resolves( 'owner' ) this.ProjectDeleter.unmarkAsDeletedByExternalSource = sinon.stub() - this.InactiveProjectManager.reactivateProjectIfRequired.callsArgWith(1) - this.ProjectUpdateHandler.markAsOpened.callsArgWith(1) + this.InactiveProjectManager.promises.reactivateProjectIfRequired.resolves() + this.ProjectUpdateHandler.promises.markAsOpened.resolves() }) it('should render the project/editor page', function (done) { @@ -526,7 +569,7 @@ describe('ProjectController', function () { opts.isRestrictedTokenMember.should.equal(false) return done() } - return this.ProjectController.loadEditor(this.req, this.res) + this.ProjectController.loadEditor(this.req, this.res) }) it('should set isRestrictedTokenMember when appropriate', function (done) { @@ -536,12 +579,12 @@ describe('ProjectController', function () { opts.isRestrictedTokenMember.should.equal(true) return done() } - return this.ProjectController.loadEditor(this.req, this.res) + this.ProjectController.loadEditor(this.req, this.res) }) it('should invoke the session maintenance for logged in user', function (done) { this.res.render = () => { - this.SplitTestSessionHandler.sessionMaintenance.should.have.been.calledWith( + this.SplitTestSessionHandler.promises.sessionMaintenance.should.have.been.calledWith( this.req, this.user ) @@ -553,7 +596,7 @@ describe('ProjectController', function () { it('should invoke the session maintenance for anonymous user', function (done) { this.SessionManager.getLoggedInUserId.returns(null) this.res.render = () => { - this.SplitTestSessionHandler.sessionMaintenance.should.have.been.calledWith( + this.SplitTestSessionHandler.promises.sessionMaintenance.should.have.been.calledWith( this.req ) done() @@ -571,11 +614,16 @@ describe('ProjectController', function () { }) it('should not render the page if the project can not be accessed', function (done) { - this.AuthorizationManager.getPrivilegeLevelForProject = sinon + this.AuthorizationManager.promises.getPrivilegeLevelForProject = sinon .stub() - .callsArgWith(3, null, null) + .resolves(null) this.res.sendStatus = (resCode, opts) => { resCode.should.equal(401) + this.AuthorizationManager.promises.getPrivilegeLevelForProject.should.have.been.calledWith( + this.user._id, + this.project_id, + 'some-token' + ) done() } this.ProjectController.loadEditor(this.req, this.res) @@ -583,7 +631,7 @@ describe('ProjectController', function () { it('should reactivateProjectIfRequired', function (done) { this.res.render = (pageName, opts) => { - this.InactiveProjectManager.reactivateProjectIfRequired + this.InactiveProjectManager.promises.reactivateProjectIfRequired .calledWith(this.project_id) .should.equal(true) done() @@ -605,7 +653,7 @@ describe('ProjectController', function () { it('should mark project as opened', function (done) { this.res.render = (pageName, opts) => { - this.ProjectUpdateHandler.markAsOpened + this.ProjectUpdateHandler.promises.markAsOpened .calledWith(this.project_id) .should.equal(true) done() @@ -614,9 +662,9 @@ describe('ProjectController', function () { }) it('should call the brand variations handler for branded projects', function (done) { - this.ProjectGetter.getProject.callsArgWith(2, null, this.brandedProject) + this.ProjectGetter.promises.getProject.resolves(this.brandedProject) this.res.render = (pageName, opts) => { - this.BrandVariationsHandler.getBrandVariationById + this.BrandVariationsHandler.promises.getBrandVariationById .calledWith() .should.equal(true) done() @@ -626,7 +674,7 @@ describe('ProjectController', function () { it('should not call the brand variations handler for unbranded projects', function (done) { this.res.render = (pageName, opts) => { - this.BrandVariationsHandler.getBrandVariationById.called.should.equal( + this.BrandVariationsHandler.promises.getBrandVariationById.called.should.equal( false ) done() @@ -635,7 +683,7 @@ describe('ProjectController', function () { }) it('should expose the brand variation details as locals for branded projects', function (done) { - this.ProjectGetter.getProject.callsArgWith(2, null, this.brandedProject) + this.ProjectGetter.promises.getProject.resolves(this.brandedProject) this.res.render = (pageName, opts) => { opts.brandVariation.should.deep.equal(this.brandVariationDetails) done() @@ -645,7 +693,7 @@ describe('ProjectController', function () { it('flushes the project to TPDS if a flush is pending', function (done) { this.res.render = () => { - this.TpdsProjectFlusher.flushProjectToTpdsIfNeeded.should.have.been.calledWith( + this.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded.should.have.been.calledWith( this.project_id ) done() @@ -656,7 +704,7 @@ describe('ProjectController', function () { it('should refresh the user features if the epoch is outdated', function (done) { this.FeaturesUpdater.featuresEpochIsCurrent = sinon.stub().returns(false) this.res.render = () => { - this.FeaturesUpdater.refreshFeatures.should.have.been.calledWith( + this.FeaturesUpdater.promises.refreshFeatures.should.have.been.calledWith( this.user._id, 'load-editor' ) @@ -898,9 +946,9 @@ describe('ProjectController', function () { // default to saas enabled this.Features.hasFeature.withArgs('saas').returns(true) // default to without a subscription - this.SubscriptionLocator.getUsersSubscription = sinon + this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() - .callsArgWith(1, null, null) + .resolves(null) }) it('should not show without the saas feature', function (done) { this.Features.hasFeature.withArgs('saas').returns(false) @@ -918,9 +966,9 @@ describe('ProjectController', function () { this.ProjectController.loadEditor(this.req, this.res) }) it('should not show for a user with a personal subscription', function (done) { - this.SubscriptionLocator.getUsersSubscription = sinon + this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() - .callsArgWith(1, null, {}) + .resolves({}) this.res.render = (pageName, opts) => { expect(opts.showUpgradePrompt).to.equal(false) done() @@ -928,9 +976,9 @@ describe('ProjectController', function () { this.ProjectController.loadEditor(this.req, this.res) }) it('should not show for a user who is a member of a group subscription', function (done) { - this.LimitationsManager.userIsMemberOfGroupSubscription = sinon + this.LimitationsManager.promises.userIsMemberOfGroupSubscription = sinon .stub() - .callsArgWith(1, null, true) + .resolves(true) this.res.render = (pageName, opts) => { expect(opts.showUpgradePrompt).to.equal(false) done() @@ -938,9 +986,9 @@ describe('ProjectController', function () { this.ProjectController.loadEditor(this.req, this.res) }) it('should not show for a user with an affiliated paid university', function (done) { - this.InstitutionsFeatures.hasLicence = sinon + this.InstitutionsFeatures.promises.hasLicence = sinon .stub() - .callsArgWith(1, null, true) + .resolves(true) this.res.render = (pageName, opts) => { expect(opts.showUpgradePrompt).to.equal(false) done() @@ -999,9 +1047,9 @@ describe('ProjectController', function () { .withArgs(projects[3], this.user._id) .returns(false) - this.ProjectGetter.findAllUsersProjects = sinon + this.ProjectGetter.promises.findAllUsersProjects = sinon .stub() - .callsArgWith(2, null, []) + .resolves([]) this.ProjectController._buildProjectList = sinon.stub().returns(projects) this.SessionManager.getLoggedInUserId = sinon .stub() @@ -1033,9 +1081,9 @@ describe('ProjectController', function () { { path: '/main.tex', doc: true }, ] this.files = [{ path: '/things/a.txt' }] - this.ProjectGetter.getProject = sinon + this.ProjectGetter.promises.getProject = sinon .stub() - .callsArgWith(1, null, this.project) + .resolves(this.project) this.ProjectEntityHandler.getAllEntitiesFromProject = sinon .stub() .returns({ docs: this.docs, files: this.files }) @@ -1051,7 +1099,7 @@ describe('ProjectController', function () { { path: '/things/b.txt', type: 'doc' }, ], }) - expect(this.ProjectGetter.getProject.callCount).to.equal(1) + expect(this.ProjectGetter.promises.getProject.callCount).to.equal(1) expect( this.ProjectEntityHandler.getAllEntitiesFromProject.callCount ).to.equal(1)