From d56477565cf32dfb5b9027340ad8093f935630a6 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 17 Apr 2026 07:44:32 +0200 Subject: [PATCH] [web] deduplicate getting the project when loading the editor (#32762) * [web] enable async local storage on all the endpoints Co-authored-by: Brian Gough * [web] deduplicate getting the project when loading the editor Co-authored-by: Brian Gough * [web] use ProjectAccess state for computing analytics segmentation Co-authored-by: Brian Gough * [web] restore ownership of active flag and deferredTpdsFlushCounter * [web] add missing await * [web] update unit tests * [web] add metrics for project access caching * [web] add missing test mock * [web] invalidate async local storage when changing project access * [web] deduplicate project lookup when checking for token access * [web] add helper function for getting cached ProjectAccess * [web] add acceptance test for caching of ProjectAccess * [web] account for saas-only project access in tests * Revert "[web] enable async local storage on all the endpoints" This reverts commit 1b82f3b935040e8cfd180d1f6bf4183a655580e2. * [web] add async local storage to project endpoints in top-50 * [web] invalidate async local storage for project access from modules Co-authored-by: Jessica Lawshe --------- Co-authored-by: Brian Gough Co-authored-by: Jessica Lawshe GitOrigin-RevId: 3eea7956b24e6f937dc1c17948681063d4dca3ea --- .../Collaborators/CollaboratorsGetter.mjs | 94 ++++++++++++++++++- .../Collaborators/CollaboratorsHandler.mjs | 7 ++ .../OwnershipTransferHandler.mjs | 2 + .../app/src/Features/Editor/EditorRouter.mjs | 4 + .../src/Features/History/HistoryRouter.mjs | 11 +++ .../InactiveData/InactiveProjectManager.mjs | 20 ++-- .../Features/Project/ProjectController.mjs | 26 ++--- .../Subscription/LimitationsManager.mjs | 8 +- .../TpdsProjectFlusher.mjs | 9 +- .../src/Features/Uploads/UploadsRouter.mjs | 3 + services/web/app/src/router.mjs | 6 ++ .../test/acceptance/src/ProjectCRUDTests.mjs | 27 ++++++ .../test/acceptance/src/helpers/metrics.mjs | 15 +++ .../CollaboratorsGetter.test.mjs | 4 + .../InactiveProjectManager.test.mjs | 13 +-- .../src/Project/ProjectController.test.mjs | 19 +++- .../Subscription/LimitationsManager.test.mjs | 16 +--- .../TpdsProjectFlusher.test.mjs | 4 +- 18 files changed, 219 insertions(+), 69 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.mjs index 320429da5e..ca795592c2 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.mjs @@ -12,6 +12,8 @@ import Errors from '../Errors/Errors.js' import ProjectEditorHandler from '../Project/ProjectEditorHandler.mjs' import Sources from '../Authorization/Sources.mjs' import PrivilegeLevels from '../Authorization/PrivilegeLevels.mjs' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' +import Metrics from '@overleaf/metrics' const { ObjectId } = mongodb @@ -44,6 +46,12 @@ class ProjectAccess { /** @type {PublicAccessLevel} */ #publicAccessLevel + /** @type {ObjectId} */ + #ownerId + + /** @type {Record} */ + #stats + /** * @param {{ owner_ref: ObjectId; collaberator_refs: ObjectId[]; readOnly_refs: ObjectId[]; tokenAccessReadAndWrite_refs: ObjectId[]; tokenAccessReadOnly_refs: ObjectId[]; publicAccesLevel: PublicAccessLevel; pendingEditor_refs: ObjectId[]; reviewer_refs: ObjectId[]; pendingReviewer_refs: ObjectId[]; }} project */ @@ -59,7 +67,28 @@ class ProjectAccess { project.reviewer_refs, project.pendingReviewer_refs ) + this.#stats = { + reviewers: (project.reviewer_refs || []).length, + namedEditors: (project.collaberator_refs || []).length, + pendingEditors: (project.pendingEditor_refs || []).length, + tokenEditors: (project.tokenAccessReadAndWrite_refs || []).length, + } this.#publicAccessLevel = project.publicAccesLevel + this.#ownerId = project.owner_ref + } + + /** + * @return {ObjectId} + */ + getOwnerId() { + return this.#ownerId + } + + /** + * @return {Record} + */ + getStats() { + return this.#stats } /** @@ -154,6 +183,24 @@ class ProjectAccess { return false } + /** + * @param {string | ObjectId} userId + * @return {boolean} + */ + isUserReadWriteTokenMember(userId) { + if (!userId) return false + for (const member of this.#members) { + if ( + member.id === userId.toString() && + member.source === Sources.TOKEN && + member.privilegeLevel === PrivilegeLevels.READ_AND_WRITE + ) { + return true + } + } + return false + } + /** * @param {string | ObjectId} userId * @return {boolean} @@ -212,10 +259,30 @@ class ProjectAccess { } } +/** + * @param {string} projectId + * @param {string} path + * @return {ProjectAccess|null} + * @private + */ +function _getCachedProjectAccess(projectId, path) { + const store = AsyncLocalStorage.storage.getStore() + const key = `projectAccess:${projectId}` + if (store && store[key]) { + Metrics.inc('project_access_cache', 1, { status: 'hit', path }) + return store[key] + } + Metrics.inc('project_access_cache', 1, { status: 'miss', path }) + return null +} + /** * @param {any} projectId */ async function getProjectAccess(projectId) { + let projectAccess = _getCachedProjectAccess(projectId, 'full') + if (projectAccess) return projectAccess + const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, collaberator_refs: 1, @@ -230,7 +297,25 @@ async function getProjectAccess(projectId) { if (!project) { throw new Errors.NotFoundError(`no project found with id ${projectId}`) } - return new ProjectAccess(project) + projectAccess = new ProjectAccess(project) + const store = AsyncLocalStorage.storage.getStore() + const key = `projectAccess:${projectId}` + if (store) store[key] = projectAccess + return projectAccess +} + +/** + * @param {string} projectId + * @return {Promise} + */ +async function getProjectOwnerId(projectId) { + const projectAccess = _getCachedProjectAccess(projectId, 'project-owner') + if (projectAccess) return projectAccess.getOwnerId() + + const project = await ProjectGetter.promises.getProject(projectId, { + owner_ref: true, + }) + return project.owner_ref } /** @@ -451,6 +536,9 @@ async function getAllInvitedMembers(projectId) { * @param {any} projectId */ async function userIsTokenMember(userId, projectId) { + const projectAccess = _getCachedProjectAccess(projectId, 'token-member') + if (projectAccess) return projectAccess.isUserTokenMember(userId) + userId = new ObjectId(userId.toString()) projectId = new ObjectId(projectId.toString()) const project = await Project.findOne( @@ -473,6 +561,9 @@ async function userIsTokenMember(userId, projectId) { * @param {any} projectId */ async function userIsReadWriteTokenMember(userId, projectId) { + const projectAccess = _getCachedProjectAccess(projectId, 'rw-token-member') + if (projectAccess) return projectAccess.isUserReadWriteTokenMember(userId) + userId = new ObjectId(userId.toString()) projectId = new ObjectId(projectId.toString()) const project = await Project.findOne( @@ -645,6 +736,7 @@ export default { userIsTokenMember, userIsReadWriteTokenMember, getAllInvitedMembers, + getProjectOwnerId, }, ProjectAccess, } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs index 945c14034c..73ad53508b 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs @@ -12,6 +12,7 @@ import Errors from '../Errors/Errors.js' import TpdsUpdateSender from '../ThirdPartyDataStore/TpdsUpdateSender.mjs' import EditorRealTimeController from '../Editor/EditorRealTimeController.mjs' import ProjectAuditLogHandler from '../Project/ProjectAuditLogHandler.mjs' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' export default { userIsTokenMember: callbackify(userIsTokenMember), @@ -31,6 +32,7 @@ export default { } async function removeUserFromProject(projectId, userId) { + AsyncLocalStorage.removeItem(`projectAccess:${projectId}`) try { await Project.updateOne( { _id: projectId }, @@ -97,6 +99,7 @@ async function addUserIdToProject( privilegeLevel, { pendingEditor, pendingReviewer } = {} ) { + AsyncLocalStorage.removeItem(`projectAccess:${projectId}`) const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, name: 1, @@ -199,6 +202,9 @@ async function transferProjects(fromUserId, toUserId) { ).exec() const projectIds = projects.map(p => p._id) logger.debug({ projectIds, fromUserId, toUserId }, 'transferring projects') + for (const projectId of projectIds) { + AsyncLocalStorage.removeItem(`projectAccess:${projectId}`) + } await Project.updateMany( { owner_ref: fromUserId }, @@ -273,6 +279,7 @@ async function setCollaboratorPrivilegeLevel( { pendingEditor, pendingReviewer } = {}, auditInfo = {} ) { + AsyncLocalStorage.removeItem(`projectAccess:${projectId}`) // Make sure we're only updating the project if the user is already a // collaborator const query = { diff --git a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.mjs b/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.mjs index 8df7f78c6e..99d77cc254 100644 --- a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.mjs +++ b/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.mjs @@ -13,6 +13,7 @@ import OError from '@overleaf/o-error' import TagsHandler from '../Tags/TagsHandler.mjs' import { promiseMapWithLimit } from '@overleaf/promise-utils' import LimitationsManager from '../Subscription/LimitationsManager.mjs' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' export default { promises: { @@ -212,6 +213,7 @@ async function _determinePrivilegeLevelForPreviousOwner(projectId) { } async function _transferOwnership(projectId, previousOwnerId, newOwnerId) { + AsyncLocalStorage.removeItem(`projectAccess:${projectId}`) // Remove new owner from collaborators list await CollaboratorsHandler.promises.removeUserFromProject( projectId, diff --git a/services/web/app/src/Features/Editor/EditorRouter.mjs b/services/web/app/src/Features/Editor/EditorRouter.mjs index 09833221a4..20b8a6cb43 100644 --- a/services/web/app/src/Features/Editor/EditorRouter.mjs +++ b/services/web/app/src/Features/Editor/EditorRouter.mjs @@ -3,6 +3,7 @@ import AuthenticationController from '../Authentication/AuthenticationController import AuthorizationMiddleware from '../Authorization/AuthorizationMiddleware.mjs' import { RateLimiter } from '../../infrastructure/RateLimiter.mjs' import RateLimiterMiddleware from '../Security/RateLimiterMiddleware.mjs' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' const rateLimiters = { addDocToProject: new RateLimiter('add-doc-to-project', { @@ -20,6 +21,7 @@ export default { apply(webRouter, privateApiRouter) { webRouter.post( '/project/:Project_id/doc', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanWriteProjectContent, RateLimiterMiddleware.rateLimit(rateLimiters.addDocToProject, { params: ['Project_id'], @@ -37,6 +39,7 @@ export default { webRouter.post( '/project/:Project_id/:entity_type/:entity_id/rename', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity ) @@ -48,6 +51,7 @@ export default { webRouter.delete( '/project/:Project_id/file/:entity_id', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanWriteProjectContent, EditorHttpController.deleteFile ) diff --git a/services/web/app/src/Features/History/HistoryRouter.mjs b/services/web/app/src/Features/History/HistoryRouter.mjs index 5db1d689b4..1025cd7036 100644 --- a/services/web/app/src/Features/History/HistoryRouter.mjs +++ b/services/web/app/src/Features/History/HistoryRouter.mjs @@ -6,6 +6,7 @@ import AuthenticationController from '../Authentication/AuthenticationController import AuthorizationMiddleware from '../Authorization/AuthorizationMiddleware.mjs' import RateLimiterMiddleware from '../Security/RateLimiterMiddleware.mjs' import HistoryController from './HistoryController.mjs' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' const rateLimiters = { downloadProjectRevision: new RateLimiter('download-project-revision', { @@ -33,12 +34,14 @@ function apply(webRouter, privateApiRouter) { webRouter.head( '/project/:project_id/blob/:hash', RateLimiterMiddleware.rateLimit(rateLimiters.getProjectBlob), + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.headBlob ) webRouter.get( '/project/:project_id/blob/:hash', RateLimiterMiddleware.rateLimit(rateLimiters.getProjectBlob), + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.getBlob ) @@ -47,24 +50,28 @@ function apply(webRouter, privateApiRouter) { webRouter.get( '/project/:Project_id/updates', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.proxyToHistoryApiAndInjectUserDetails ) webRouter.get( '/project/:Project_id/doc/:doc_id/diff', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.proxyToHistoryApi ) webRouter.get( '/project/:Project_id/diff', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.proxyToHistoryApiAndInjectUserDetails ) webRouter.get( '/project/:Project_id/filetree/diff', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.proxyToHistoryApi @@ -103,6 +110,7 @@ function apply(webRouter, privateApiRouter) { webRouter.post( '/project/:Project_id/flush', RateLimiterMiddleware.rateLimit(rateLimiters.flushHistory), + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.proxyToHistoryApi @@ -117,6 +125,7 @@ function apply(webRouter, privateApiRouter) { webRouter.get( '/project/:Project_id/labels', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.getLabels @@ -136,12 +145,14 @@ function apply(webRouter, privateApiRouter) { webRouter.get( '/project/:project_id/latest/history', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.getLatestHistory ) webRouter.get( '/project/:project_id/changes', + AsyncLocalStorage.middleware, AuthorizationMiddleware.blockRestrictedUserFromProject, AuthorizationMiddleware.ensureUserCanReadProject, HistoryController.getChanges diff --git a/services/web/app/src/Features/InactiveData/InactiveProjectManager.mjs b/services/web/app/src/Features/InactiveData/InactiveProjectManager.mjs index 53f9f569e5..9eeaf4df44 100644 --- a/services/web/app/src/Features/InactiveData/InactiveProjectManager.mjs +++ b/services/web/app/src/Features/InactiveData/InactiveProjectManager.mjs @@ -2,7 +2,6 @@ import OError from '@overleaf/o-error' import logger from '@overleaf/logger' import DocstoreManager from '../Docstore/DocstoreManager.mjs' import DocumentUpdaterHandler from '../DocumentUpdater/DocumentUpdaterHandler.mjs' -import ProjectGetter from '../Project/ProjectGetter.mjs' import ProjectUpdateHandler from '../Project/ProjectUpdateHandler.mjs' import { Project } from '../../models/Project.mjs' import Modules from '../../infrastructure/Modules.mjs' @@ -33,19 +32,12 @@ function findInactiveProjects(limit, daysOld) { } const InactiveProjectManager = { - async reactivateProjectIfRequired(projectId) { - let project - try { - project = await ProjectGetter.promises.getProject(projectId, { - active: true, - }) - } catch (err) { - OError.tag(err, 'error getting project', { - project_id: projectId, - }) - throw err - } - + /** + * @param {{_id: ObjectId, active: boolean }} project + * @return {Promise} + */ + async reactivateProjectIfRequired(project) { + const projectId = project._id.toString() logger.debug( { projectId, active: project.active }, 'seeing if need to reactivate project' diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index 9eaf9ec495..04fa90928a 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -539,17 +539,16 @@ const _ProjectController = { const responses = await pProps({ userValues: userId ? getUserValues(userId) : defaultUserValues(), project: ProjectGetter.promises.getProject(projectId, { + _id: 1, name: 1, + active: 1, + deferredTpdsFlushCounter: 1, lastUpdated: 1, track_changes: 1, owner_ref: 1, brandVariationId: 1, overleaf: 1, tokens: 1, - tokenAccessReadAndWrite_refs: 1, // used for link sharing analytics - collaberator_refs: 1, // used for link sharing analytics - pendingEditor_refs: 1, // used for link sharing analytics - reviewer_refs: 1, }), userIsMemberOfGroupSubscription: sessionUser ? (async () => @@ -559,16 +558,15 @@ const _ProjectController = { ) ).isMember)() : false, - _flushToTpds: - TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded(projectId), - _activate: - InactiveProjectManager.promises.reactivateProjectIfRequired( - projectId - ), }) const { project, userValues, userIsMemberOfGroupSubscription } = responses + await Promise.all([ + InactiveProjectManager.promises.reactivateProjectIfRequired(project), + TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded(project), + ]) + const { user, learnedWords, @@ -705,9 +703,11 @@ const _ProjectController = { project.owner_ref ) if (userId) { + const projectAccess = + await CollaboratorsGetter.promises.getProjectAccess(projectId) const planLimit = ownerFeatures?.collaborators || 0 - const namedEditors = project.collaberator_refs?.length || 0 - const pendingEditors = project.pendingEditor_refs?.length || 0 + const { namedEditors, pendingEditors, tokenEditors } = + projectAccess.getStats() const exceedAtLimit = planLimit > -1 && namedEditors >= planLimit let mode = 'edit' @@ -727,7 +727,7 @@ const _ProjectController = { projectId: project._id, namedEditors, pendingEditors, - tokenEditors: project.tokenAccessReadAndWrite_refs?.length || 0, + tokenEditors, planLimit, exceedAtLimit, } diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.mjs b/services/web/app/src/Features/Subscription/LimitationsManager.mjs index 4033a090c3..6c00579bc1 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.mjs +++ b/services/web/app/src/Features/Subscription/LimitationsManager.mjs @@ -2,7 +2,6 @@ import logger from '@overleaf/logger' -import ProjectGetter from '../Project/ProjectGetter.mjs' import UserGetter from '../User/UserGetter.mjs' import SubscriptionLocator from './SubscriptionLocator.mjs' import Settings from '@overleaf/settings' @@ -15,10 +14,9 @@ import { callbackify, callbackifyMultiResult } from '@overleaf/promise-utils' * @param {any} projectId */ async function allowedNumberOfCollaboratorsInProject(projectId) { - const project = await ProjectGetter.promises.getProject(projectId, { - owner_ref: true, - }) - return await allowedNumberOfCollaboratorsForUser(project.owner_ref) + const ownerId = + await CollaboratorsGetter.promises.getProjectOwnerId(projectId) + return await allowedNumberOfCollaboratorsForUser(ownerId) } /** diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.mjs b/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.mjs index 206682972b..d49ffacacd 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.mjs +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.mjs @@ -34,13 +34,10 @@ async function flushProjectToTpds(projectId) { * Flush a project to TPDS if a flush is pending. This is called when * projects are loaded in the editor and triggers a sync to dropbox for * projects that were imported from Overleaf v1. + * @param {{name: string, deferredTpdsFlushCounter: number, overleaf: { history: { id: (string|number)}} }} project + * @return {Promise} */ -async function flushProjectToTpdsIfNeeded(projectId) { - const project = await ProjectGetter.promises.getProject(projectId, { - name: true, - deferredTpdsFlushCounter: true, - 'overleaf.history.id': 1, - }) +async function flushProjectToTpdsIfNeeded(project) { if (project.deferredTpdsFlushCounter > 0) { await _flushProjectToTpds(project) } diff --git a/services/web/app/src/Features/Uploads/UploadsRouter.mjs b/services/web/app/src/Features/Uploads/UploadsRouter.mjs index 82e6658caa..98716f2ba1 100644 --- a/services/web/app/src/Features/Uploads/UploadsRouter.mjs +++ b/services/web/app/src/Features/Uploads/UploadsRouter.mjs @@ -4,6 +4,7 @@ import ProjectUploadController from './ProjectUploadController.mjs' import { RateLimiter } from '../../infrastructure/RateLimiter.mjs' import RateLimiterMiddleware from '../Security/RateLimiterMiddleware.mjs' import Settings from '@overleaf/settings' +import AsyncLocalStorage from '../../infrastructure/AsyncLocalStorage.mjs' const rateLimiters = { projectUpload: new RateLimiter('project-upload', { @@ -47,6 +48,7 @@ export default { webRouter.post( fileUploadEndpoint, fileUploadRateLimit, + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanWriteProjectContent, ProjectUploadController.multerMiddleware, ProjectUploadController.uploadFile @@ -56,6 +58,7 @@ export default { fileUploadEndpoint, fileUploadRateLimit, AuthenticationController.requireLogin(), + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanWriteProjectContent, ProjectUploadController.multerMiddleware, ProjectUploadController.uploadFile diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index a6b967e715..a8b940afba 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -589,6 +589,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { RateLimiterMiddleware.rateLimit(rateLimiters.compileProjectHttp, { params: ['Project_id'], }), + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, CompileController.compile ) @@ -601,6 +602,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { webRouter.get( '/project/:Project_id/output/cached/output.overleaf.json', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, ClsiCacheController.getLatestBuildFromCache ) @@ -680,11 +682,13 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { ) webRouter.get( '/project/:Project_id/sync/code', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, CompileController.proxySyncCode ) webRouter.get( '/project/:Project_id/sync/pdf', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, CompileController.proxySyncPdf ) @@ -778,6 +782,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { webRouter.get( '/project/:project_id/metadata', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, Settings.allowAnonymousReadAndWriteSharing ? (req, res, next) => { @@ -788,6 +793,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { ) webRouter.post( '/project/:project_id/doc/:doc_id/metadata', + AsyncLocalStorage.middleware, AuthorizationMiddleware.ensureUserCanReadProject, Settings.allowAnonymousReadAndWriteSharing ? (req, res, next) => { diff --git a/services/web/test/acceptance/src/ProjectCRUDTests.mjs b/services/web/test/acceptance/src/ProjectCRUDTests.mjs index 12d4e3e0c7..af711f29be 100644 --- a/services/web/test/acceptance/src/ProjectCRUDTests.mjs +++ b/services/web/test/acceptance/src/ProjectCRUDTests.mjs @@ -5,11 +5,22 @@ import mongodb from 'mongodb-legacy' import cheerio from 'cheerio' import { Subscription } from '../../../app/src/models/Subscription.mjs' import Features from '../../../app/src/infrastructure/Features.mjs' +import Metrics from './helpers/metrics.mjs' const ObjectId = mongodb.ObjectId const User = UserHelper.promises +async function getProjectAccessStats() { + const hit = await Metrics.promises.sumMetrics( + s => s.startsWith('project_access_cache') && s.includes('"hit"') + ) + const miss = await Metrics.promises.sumMetrics( + s => s.startsWith('project_access_cache') && s.includes('"miss"') + ) + return { hit, miss } +} + describe('Project CRUD', function () { beforeEach(async function () { this.user = new User() @@ -65,6 +76,22 @@ describe('Project CRUD', function () { '' ) }) + + it('should cache the project access', async function () { + const prev = await getProjectAccessStats() + await loadProject(this.user, this.projectId) + if (Features.hasFeature('saas')) { + expect(await getProjectAccessStats()).to.deep.equal({ + hit: prev.hit + 7, + miss: prev.miss + 1, + }) + } else { + expect(await getProjectAccessStats()).to.deep.equal({ + hit: prev.hit + 4, + miss: prev.miss + 1, + }) + } + }) }) describe("when project doesn't exist", function () { diff --git a/services/web/test/acceptance/src/helpers/metrics.mjs b/services/web/test/acceptance/src/helpers/metrics.mjs index cb47f3d91d..a8a5e11387 100644 --- a/services/web/test/acceptance/src/helpers/metrics.mjs +++ b/services/web/test/acceptance/src/helpers/metrics.mjs @@ -2,6 +2,10 @@ import { callbackify } from 'node:util' import request from './request.js' import metrics from '@overleaf/metrics' +/** + * @param {(s: string) => bool} matcher + * @return {Promise} + */ async function getMetric(matcher) { const { body } = await request.promises.request('/metrics') const found = body.split('\n').find(matcher) @@ -9,6 +13,16 @@ async function getMetric(matcher) { return parseInt(found.split(' ')[1], 0) } +/** + * @param {(s: string) => bool} matcher + * @return {Promise} + */ +async function sumMetrics(matcher) { + const { body = '' } = await request.promises.request('/metrics') + const found = body.split('\n').filter(matcher) + return found.reduce((sum, next) => sum + parseInt(next.split(' ')[1], 0), 0) +} + /* sets all metrics to zero https://github.com/siimon/prom-client?tab=readme-ov-file#resetting-metrics */ @@ -21,5 +35,6 @@ export default { resetMetrics, promises: { getMetric, + sumMetrics, }, } diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetter.test.mjs b/services/web/test/unit/src/Collaborators/CollaboratorsGetter.test.mjs index 9d304f421d..7a1bd22a4e 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetter.test.mjs +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetter.test.mjs @@ -70,6 +70,10 @@ describe('CollaboratorsGetter', function () { buildUserModelView: sinon.stub(), } + vi.doMock('@overleaf/metrics', () => ({ + default: { inc: sinon.stub() }, + })) + vi.doMock('mongodb-legacy', () => ({ default: { ObjectId }, })) diff --git a/services/web/test/unit/src/InactiveData/InactiveProjectManager.test.mjs b/services/web/test/unit/src/InactiveData/InactiveProjectManager.test.mjs index 42851e1836..e6ecb99467 100644 --- a/services/web/test/unit/src/InactiveData/InactiveProjectManager.test.mjs +++ b/services/web/test/unit/src/InactiveData/InactiveProjectManager.test.mjs @@ -56,10 +56,6 @@ describe('InactiveProjectManager', function () { }) ) - vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({ - default: ctx.ProjectGetter, - })) - vi.doMock('../../../../app/src/models/Project', () => ({})) vi.doMock('../../../../app/src/infrastructure/Modules', () => ({ @@ -79,15 +75,14 @@ describe('InactiveProjectManager', function () { describe('reactivateProjectIfRequired', function () { beforeEach(function (ctx) { - ctx.project = { active: false } - ctx.ProjectGetter.promises.getProject.resolves(ctx.project) + ctx.project = { _id: ctx.project_id, active: false } ctx.ProjectUpdateHandler.promises.markAsActive.resolves() }) it('should call unarchiveProject', async function (ctx) { ctx.DocstoreManager.promises.unarchiveProject.resolves() await ctx.InactiveProjectManager.promises.reactivateProjectIfRequired( - ctx.project_id + ctx.project ) ctx.DocstoreManager.promises.unarchiveProject @@ -102,7 +97,7 @@ describe('InactiveProjectManager', function () { ctx.DocstoreManager.promises.unarchiveProject.rejects() await expect( ctx.InactiveProjectManager.promises.reactivateProjectIfRequired( - ctx.project_id + ctx.project ) ).to.be.rejected @@ -118,7 +113,7 @@ describe('InactiveProjectManager', function () { ctx.project.active = true ctx.DocstoreManager.promises.unarchiveProject.resolves() await ctx.InactiveProjectManager.promises.reactivateProjectIfRequired( - ctx.project_id + ctx.project ) ctx.DocstoreManager.promises.unarchiveProject .calledWith(ctx.project_id) diff --git a/services/web/test/unit/src/Project/ProjectController.test.mjs b/services/web/test/unit/src/Project/ProjectController.test.mjs index e74657aebd..af78d06920 100644 --- a/services/web/test/unit/src/Project/ProjectController.test.mjs +++ b/services/web/test/unit/src/Project/ProjectController.test.mjs @@ -112,7 +112,9 @@ describe('ProjectController', function () { }, } ctx.InactiveProjectManager = { - promises: { reactivateProjectIfRequired: sinon.stub() }, + promises: { + reactivateProjectIfRequired: sinon.stub(), + }, } ctx.ProjectUpdateHandler = { promises: { @@ -144,6 +146,15 @@ describe('ProjectController', function () { } ctx.CollaboratorsGetter = { promises: { + getProjectAccess: sinon.stub().resolves({ + getStats() { + return { + namedEditors: 1, + pendingEditors: 2, + tokenEditors: 3, + } + }, + }), userIsTokenMember: sinon.stub().resolves(false), isUserInvitedMemberOfProject: sinon.stub().resolves(true), userIsReadWriteTokenMember: sinon.stub().resolves(false), @@ -1018,11 +1029,11 @@ describe('ProjectController', function () { }) }) - it('should reactivateProjectIfRequired', async function (ctx) { + it('should call reactivateProjectIfRequired', async function (ctx) { await new Promise(resolve => { ctx.res.render = (pageName, opts) => { ctx.InactiveProjectManager.promises.reactivateProjectIfRequired - .calledWith(ctx.project_id) + .calledWith(ctx.project) .should.equal(true) resolve() } @@ -1096,7 +1107,7 @@ describe('ProjectController', function () { await new Promise(resolve => { ctx.res.render = () => { ctx.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded.should.have.been.calledWith( - ctx.project_id + ctx.project ) resolve() } diff --git a/services/web/test/unit/src/Subscription/LimitationsManager.test.mjs b/services/web/test/unit/src/Subscription/LimitationsManager.test.mjs index 4c98509782..6328403d3c 100644 --- a/services/web/test/unit/src/Subscription/LimitationsManager.test.mjs +++ b/services/web/test/unit/src/Subscription/LimitationsManager.test.mjs @@ -13,17 +13,6 @@ describe('LimitationsManager', function () { _id: (ctx.projectId = 'project-id'), owner_ref: ctx.userId, } - ctx.ProjectGetter = { - promises: { - getProject: sinon.stub().callsFake(async (projectId, fields) => { - if (projectId === ctx.projectId) { - return ctx.project - } else { - return null - } - }), - }, - } ctx.UserGetter = { promises: { getUser: sinon.stub().callsFake(async (userId, filter) => { @@ -48,6 +37,7 @@ describe('LimitationsManager', function () { promises: { getInvitedEditCollaboratorCount: sinon.stub().resolves(0), getMemberIdPrivilegeLevel: sinon.stub(), + getProjectOwnerId: sinon.stub().resolves(ctx.project.owner_ref), }, } @@ -57,10 +47,6 @@ describe('LimitationsManager', function () { }, } - vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({ - default: ctx.ProjectGetter, - })) - vi.doMock('../../../../app/src/Features/User/UserGetter', () => ({ default: ctx.UserGetter, })) diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusher.test.mjs b/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusher.test.mjs index 11aa7bc7bd..f510526d9a 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusher.test.mjs +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusher.test.mjs @@ -197,7 +197,7 @@ describe('TpdsProjectFlusher', function () { beforeEach(async function (ctx) { ctx.project.deferredTpdsFlushCounter = counterValue await ctx.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded( - ctx.project._id + ctx.project ) }) @@ -232,7 +232,7 @@ describe('TpdsProjectFlusher', function () { .chain('exec') .resolves() await ctx.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded( - ctx.project._id + ctx.project ) })