[web] optimize project lookup for tpds operations (#35466)

* [web] store ObjectIds in addUserToProject acceptance test helper

* [web] optimize project lookup for tpds operations

GitOrigin-RevId: 03db9fc450556072f177d56f640fcc6114b295d4
This commit is contained in:
Jakob Ackermann
2026-09-17 11:28:44 +00:00
committed by Copybot
parent dfd816801d
commit 8270641b42
7 changed files with 539 additions and 242 deletions
@@ -40,6 +40,19 @@ const { ObjectId } = mongodb
// Wrapper for determining multiple dimensions of project access.
class ProjectAccess {
// Projection for the project fields the constructor needs.
static PROJECTION = {
owner_ref: 1,
collaberator_refs: 1,
readOnly_refs: 1,
tokenAccessReadOnly_refs: 1,
tokenAccessReadAndWrite_refs: 1,
publicAccesLevel: 1,
pendingEditor_refs: 1,
reviewer_refs: 1,
pendingReviewer_refs: 1,
}
/** @type {ProjectMember[]} */
#members
@@ -201,6 +214,15 @@ class ProjectAccess {
return false
}
/**
* @param {string | ObjectId} userId
* @return {boolean}
*/
isOwner(userId) {
if (!userId) return false
return this.#ownerId.toString() === userId.toString()
}
/**
* @param {string | ObjectId} userId
* @return {boolean}
@@ -283,17 +305,10 @@ 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,
readOnly_refs: 1,
tokenAccessReadOnly_refs: 1,
tokenAccessReadAndWrite_refs: 1,
publicAccesLevel: 1,
pendingEditor_refs: 1,
reviewer_refs: 1,
pendingReviewer_refs: 1,
})
const project = await ProjectGetter.promises.getProject(
projectId,
ProjectAccess.PROJECTION
)
if (!project) {
throw new Errors.NotFoundError(`no project found with id ${projectId}`)
}
@@ -141,20 +141,30 @@ const ProjectGetter = {
* Return all projects with the given name that belong to the given user.
*
* Projects include the user's own projects as well as collaborations with
* read/write access.
* read/write access. Read-only, review and token-access collaborations are
* not considered.
*/
async findUsersProjectsByName(userId, projectName) {
const allProjects = await ProjectGetter.findAllUsersProjects(
userId,
'name archived trashed overleaf'
)
const fields = 'name archived trashed overleaf'
const [ownedProjects, readAndWriteProjects] = await Promise.all([
Project.find({ owner_ref: userId }, fields).exec(),
Project.find({ collaberator_refs: userId }, fields).exec(),
])
const { owned, readAndWrite } = allProjects
const projects = owned.concat(readAndWrite)
const lowerCasedProjectName = projectName.toLowerCase()
return projects.filter(
project => project.name.toLowerCase() === lowerCasedProjectName
)
const seenProjectIds = new Set()
const matches = []
for (const project of ownedProjects.concat(readAndWriteProjects)) {
const projectId = project._id.toString()
if (seenProjectIds.has(projectId)) {
continue
}
seenProjectIds.add(projectId)
if (project.name.toLowerCase() === lowerCasedProjectName) {
matches.push(project)
}
}
return matches
},
async getUsersDeletedProjects(userId) {
@@ -1,6 +1,7 @@
import { callbackify } from 'node:util'
import UpdateMerger from './UpdateMerger.mjs'
import logger from '@overleaf/logger'
import CollaboratorsGetter from '../Collaborators/CollaboratorsGetter.mjs'
import NotificationsBuilder from '../Notifications/NotificationsBuilder.mjs'
import ProjectCreationHandler from '../Project/ProjectCreationHandler.mjs'
import ProjectDeleter from '../Project/ProjectDeleter.mjs'
@@ -10,6 +11,8 @@ import ProjectRootDocManager from '../Project/ProjectRootDocManager.mjs'
import FileTypeManager from '../Uploads/FileTypeManager.mjs'
import Modules from '../../infrastructure/Modules.mjs'
const { ProjectAccess } = CollaboratorsGetter
async function newUpdate(
userId,
projectId,
@@ -92,21 +95,27 @@ async function getOrCreateProject(userId, projectId, projectName) {
}
async function findProjectByIdWithRWAccess(userId, projectId) {
const allProjects = await ProjectGetter.promises.findAllUsersProjects(
userId,
'name archived trashed overleaf'
)
for (const projects of [allProjects.owned, allProjects.readAndWrite]) {
for (const project of projects) {
if (project._id.toString() === projectId) {
if (ProjectHelper.isArchivedOrTrashed(project, userId)) {
return null
} else {
return project
}
}
}
const project = await ProjectGetter.promises.getProject(projectId, {
...ProjectAccess.PROJECTION,
name: 1,
archived: 1,
trashed: 1,
overleaf: 1,
})
if (project == null) {
return null
}
const projectAccess = new ProjectAccess(project)
if (
!projectAccess.isOwner(userId) &&
!projectAccess.isUserInvitedReadWriteMember(userId)
) {
return null
}
if (ProjectHelper.isArchivedOrTrashed(project, userId)) {
return null
}
return project
}
async function getOrCreateProjectByName(userId, projectName) {
@@ -5,6 +5,7 @@ import ProjectOptionsHandler from '../../../app/src/Features/Project/ProjectOpti
import ProjectRootDocManager from '../../../app/src/Features/Project/ProjectRootDocManager.mjs'
import request from './helpers/request.js'
import User from './helpers/User.mjs'
import { db, ObjectId } from '../../../app/src/infrastructure/mongodb.mjs'
describe('TpdsUpdateTests', function () {
beforeEach(function () {
@@ -156,6 +157,157 @@ describe('TpdsUpdateTests', function () {
})
})
describe('update a new file by project id', function () {
it('should add the file', function (done) {
request(
{
method: 'POST',
url: `/project/${this.projectId}/user/${this.owner._id}/update/by-id.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
body: 'test one two',
},
(error, response) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
const json = JSON.parse(response.body)
expect(json.status).to.equal('applied')
ProjectGetter.getProject(this.projectId, (error, project) => {
if (error) {
throw error
}
const projectFolder = project.rootFolder[0]
const file = projectFolder.docs.find(e => e.name === 'by-id.tex')
expect(file).to.exist
done()
})
}
)
})
it('should reject the update when the project is archived', function (done) {
this.owner.request(
{
url: `/Project/${this.projectId}/archive`,
method: 'post',
},
err => {
expect(err).to.not.exist
request(
{
method: 'POST',
url: `/project/${this.projectId}/user/${this.owner._id}/update/by-id.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
body: 'test one two',
},
(error, response) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
const json = JSON.parse(response.body)
expect(json.status).to.equal('rejected')
ProjectGetter.getProject(this.projectId, (error, project) => {
if (error) {
throw error
}
const projectFolder = project.rootFolder[0]
const file = projectFolder.docs.find(
e => e.name === 'by-id.tex'
)
expect(file).to.not.exist
done()
})
}
)
}
)
})
})
describe('delete a file by project id', function () {
it('should delete the file', function (done) {
request(
{
method: 'DELETE',
url: `/project/${this.projectId}/user/${this.owner._id}/update/main.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
},
(error, response) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
ProjectGetter.getProject(this.projectId, (error, project) => {
if (error) {
throw error
}
const projectFolder = project.rootFolder[0]
const file = projectFolder.docs.find(e => e.name === 'main.tex')
expect(file).to.not.exist
done()
})
}
)
})
it('should not delete the file when the user only has read-only access', function (done) {
this.reader = new User()
this.reader.login(error => {
expect(error).to.not.exist
this.owner.addUserToProject(
this.projectId,
this.reader,
'readOnly',
error => {
expect(error).to.not.exist
request(
{
method: 'DELETE',
url: `/project/${this.projectId}/user/${this.reader._id}/update/main.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
},
(error, response) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
ProjectGetter.getProject(this.projectId, (error, project) => {
if (error) {
throw error
}
const projectFolder = project.rootFolder[0]
const file = projectFolder.docs.find(
e => e.name === 'main.tex'
)
expect(file).to.exist
done()
})
}
)
}
)
})
})
})
describe('resolving a project by name', function () {
it('should return the id of the matching project', function (done) {
request(
@@ -344,6 +496,144 @@ describe('TpdsUpdateTests', function () {
}
)
})
it('should match the project name case-insensitively', function (done) {
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectName: 'TEST-Project' },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({
status: 'success',
projectId: this.projectId,
historyId: 1,
otMigrationStage: 0,
})
done()
}
)
})
it('should resolve the project when the user is an invited read-write collaborator', function (done) {
this.collaborator = new User()
this.collaborator.login(error => {
expect(error).to.not.exist
this.owner.addUserToProject(
this.projectId,
this.collaborator,
'readAndWrite',
error => {
expect(error).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.collaborator._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectName: 'test-project' },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({
status: 'success',
projectId: this.projectId,
historyId: 1,
otMigrationStage: 0,
})
done()
}
)
}
)
})
})
it('should not treat a project that is both owned and a collaboration as a duplicate', function (done) {
this.owner.addUserToProject(
this.projectId,
this.owner,
'readAndWrite',
error => {
expect(error).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectName: 'test-project' },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({
status: 'success',
projectId: this.projectId,
historyId: 1,
otMigrationStage: 0,
})
done()
}
)
}
)
})
it('should reject when more than one project matches the name', function (done) {
this.owner.createProject('test-project', (error, secondProjectId) => {
expect(error).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectName: 'test-project' },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
ProjectGetter.findAllUsersProjects(
this.owner._id,
'name',
(err, projects) => {
expect(err).to.not.exist
expect(projects.owned.length).to.equal(2)
done()
}
)
}
)
})
})
})
describe('resolving a project by id', function () {
@@ -409,6 +699,162 @@ describe('TpdsUpdateTests', function () {
)
})
})
it('should reject when the project does not exist', function (done) {
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectId: new ObjectId().toString() },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
done()
}
)
})
it('should reject when the user has no access to the project', function (done) {
this.otherUser = new User()
this.otherUser.login(error => {
expect(error).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.otherUser._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectId: this.projectId },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
done()
}
)
})
})
it('should reject when the user is a token-access read-write member', function (done) {
this.tokenMember = new User()
this.tokenMember.login(error => {
expect(error).to.not.exist
db.projects
.updateOne(
{ _id: new ObjectId(this.projectId) },
{
$set: { publicAccesLevel: 'tokenBased' },
$addToSet: {
tokenAccessReadAndWrite_refs: new ObjectId(
this.tokenMember._id
),
},
}
)
.then(() => {
request(
{
method: 'POST',
url: `/user/${this.tokenMember._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectId: this.projectId },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
done()
}
)
})
.catch(done)
})
})
it('should reject when the project is archived', function (done) {
this.owner.request(
{
url: `/Project/${this.projectId}/archive`,
method: 'post',
},
err => {
expect(err).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectId: this.projectId },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
done()
}
)
}
)
})
it('should reject when the project is trashed', function (done) {
this.owner.request(
{
url: `/project/${this.projectId}/trash`,
method: 'post',
},
err => {
expect(err).to.not.exist
request(
{
method: 'POST',
url: `/user/${this.owner._id}/project/resolve`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
json: { projectId: this.projectId },
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ status: 'rejected' })
done()
}
)
}
)
})
})
describe('update when the project is archived', function () {
@@ -976,22 +976,26 @@ class User {
}
addUserToProject(projectId, user, privileges, callback) {
// NOTE: user._id is a string; convert it to an ObjectId so that the
// stored refs match production data and reverse queries (e.g.
// { collaberator_refs: userId }) find the project.
const userId = new ObjectId(user._id)
let updateOp
if (privileges === 'readAndWrite') {
updateOp = { $addToSet: { collaberator_refs: user._id } }
updateOp = { $addToSet: { collaberator_refs: userId } }
} else if (privileges === 'readOnly') {
updateOp = { $addToSet: { readOnly_refs: user._id } }
updateOp = { $addToSet: { readOnly_refs: userId } }
} else if (privileges === 'pendingEditor') {
updateOp = {
$addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id },
$addToSet: { readOnly_refs: userId, pendingEditor_refs: userId },
}
} else if (privileges === 'pendingReviewer') {
updateOp = {
$addToSet: { readOnly_refs: user._id, pendingReviewer_refs: user._id },
$addToSet: { readOnly_refs: userId, pendingReviewer_refs: userId },
}
} else if (privileges === 'review') {
updateOp = {
$addToSet: { reviewer_refs: user._id },
$addToSet: { reviewer_refs: userId },
}
}
db.projects.updateOne({ _id: new ObjectId(projectId) }, updateOp, callback)
@@ -317,54 +317,8 @@ describe('ProjectGetter', function () {
})
})
describe('findUsersProjectsByName', function () {
it('should perform a case-insensitive search', async function (ctx) {
ctx.project1 = { _id: 1, name: 'find me!' }
ctx.project2 = { _id: 2, name: 'not me!' }
ctx.project3 = { _id: 3, name: 'FIND ME!' }
ctx.project4 = { _id: 4, name: 'Find Me!' }
ctx.Project.find.withArgs({ owner_ref: ctx.userId }).returns({
exec: sinon
.stub()
.resolves([ctx.project1, ctx.project2, ctx.project3, ctx.project4]),
})
const projects = await ctx.ProjectGetter.promises.findUsersProjectsByName(
ctx.userId,
ctx.project1.name
)
const projectNames = projects.map(project => project.name)
expect(projectNames).to.have.members([
ctx.project1.name,
ctx.project3.name,
ctx.project4.name,
])
})
it('should search collaborations as well', async function (ctx) {
ctx.project1 = { _id: 1, name: 'find me!' }
ctx.project2 = { _id: 2, name: 'FIND ME!' }
ctx.project3 = { _id: 3, name: 'Find Me!' }
ctx.project4 = { _id: 4, name: 'find ME!' }
ctx.project5 = { _id: 5, name: 'FIND me!' }
ctx.Project.find
.withArgs({ owner_ref: ctx.userId })
.returns({ exec: sinon.stub().resolves([ctx.project1]) })
ctx.CollaboratorsGetter.promises.getProjectsUserIsMemberOf.resolves({
readAndWrite: [ctx.project2],
readOnly: [ctx.project3],
tokenReadAndWrite: [ctx.project4],
tokenReadOnly: [ctx.project5],
})
const projects = await ctx.ProjectGetter.promises.findUsersProjectsByName(
ctx.userId,
ctx.project1.name
)
expect(projects.map(project => project.name)).to.have.members([
ctx.project1.name,
ctx.project2.name,
])
})
})
// findUsersProjectsByName is covered by the TpdsUpdateTests acceptance
// tests ("resolving a project by name").
describe('getUsersDeletedProjects', function () {
it('should look up the deleted projects by deletedProjectOwnerId', async function (ctx) {
@@ -65,13 +65,12 @@ describe('TpdsUpdateHandler', function () {
markAsDeletedByExternalSource: sinon.stub().resolves(),
},
}
ctx.CollaboratorsGetter = {
ProjectAccess: class {},
}
ctx.ProjectGetter = {
promises: {
findUsersProjectsByName: sinon.stub(),
findAllUsersProjects: sinon.stub().resolves({
owned: Object.values(ctx.projects),
readAndWrite: [],
}),
},
}
ctx.ProjectHelper = {
@@ -105,6 +104,13 @@ describe('TpdsUpdateHandler', function () {
default: ctx.Modules,
}))
vi.doMock(
'../../../../app/src/Features/Collaborators/CollaboratorsGetter',
() => ({
default: ctx.CollaboratorsGetter,
})
)
vi.doMock(
'../../../../app/src/Features/Notifications/NotificationsBuilder',
() => ({
@@ -148,45 +154,10 @@ describe('TpdsUpdateHandler', function () {
ctx.TpdsUpdateHandler = (await import(MODULE_PATH)).default
})
// Updates, deletes and resolves by project id are covered by the
// TpdsUpdateTests acceptance tests.
describe('getting an update', function () {
describe('byId', function () {
describe('with no matching project', function () {
beforeEach(function (ctx) {
ctx.projectId = new ObjectId().toString()
})
receiveUpdateById()
expectProjectNotCreated()
expectUpdateNotProcessed()
})
describe('with one matching active project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.active1._id.toString()
})
receiveUpdateById()
expectProjectNotCreated()
expectUpdateProcessed()
})
describe('with one matching archived project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.archived1._id.toString()
})
receiveUpdateById()
expectProjectNotCreated()
expectUpdateNotProcessed()
})
describe('with one matching trashed project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.trashed._id.toString()
})
receiveUpdateById()
expectProjectNotCreated()
expectUpdateNotProcessed()
})
})
describe('with no matching project', function () {
setupMatchingProjects([])
receiveUpdate()
@@ -246,44 +217,6 @@ describe('TpdsUpdateHandler', function () {
})
describe('getting a file delete', function () {
describe('byId', function () {
describe('with no matching project', function () {
beforeEach(function (ctx) {
ctx.projectId = new ObjectId().toString()
})
receiveFileDeleteById()
expectDeleteNotProcessed()
expectProjectNotDeleted()
})
describe('with one matching active project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.active1._id.toString()
})
receiveFileDeleteById()
expectDeleteProcessed()
expectProjectNotDeleted()
})
describe('with one matching archived project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.archived1._id.toString()
})
receiveFileDeleteById()
expectDeleteNotProcessed()
expectProjectNotDeleted()
})
describe('with one matching trashed project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.trashed._id.toString()
})
receiveFileDeleteById()
expectDeleteNotProcessed()
expectProjectNotDeleted()
})
})
describe('with no matching project', function () {
setupMatchingProjects([])
receiveFileDelete()
@@ -426,44 +359,6 @@ describe('TpdsUpdateHandler', function () {
})
describe('getting or creating a project', function () {
describe('byId', function () {
describe('with no matching project', function () {
beforeEach(function (ctx) {
ctx.projectId = new ObjectId().toString()
})
resolveProjectById()
expectProjectNotCreated()
expectNoProjectResolved()
})
describe('with one matching active project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.active1._id.toString()
})
resolveProjectById()
expectProjectNotCreated()
expectProjectResolved()
})
describe('with one matching archived project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.archived1._id.toString()
})
resolveProjectById()
expectProjectNotCreated()
expectNoProjectResolved()
})
describe('with one matching trashed project', function () {
beforeEach(function (ctx) {
ctx.projectId = ctx.projects.trashed._id.toString()
})
resolveProjectById()
expectProjectNotCreated()
expectNoProjectResolved()
})
})
describe('with no matching project', function () {
setupMatchingProjects([])
resolveProjectByName()
@@ -530,19 +425,6 @@ function receiveUpdate() {
})
}
function receiveUpdateById() {
beforeEach(async function (ctx) {
await ctx.TpdsUpdateHandler.promises.newUpdate(
ctx.userId,
ctx.projectId,
'', // projectName
ctx.path,
ctx.update,
ctx.source
)
})
}
function receiveFileDelete() {
beforeEach(async function (ctx) {
await ctx.TpdsUpdateHandler.promises.deleteUpdate(
@@ -555,18 +437,6 @@ function receiveFileDelete() {
})
}
function receiveFileDeleteById() {
beforeEach(async function (ctx) {
await ctx.TpdsUpdateHandler.promises.deleteUpdate(
ctx.userId,
ctx.projectId,
'', // projectName
ctx.path,
ctx.source
)
})
}
function receiveProjectDelete() {
beforeEach(async function (ctx) {
await ctx.TpdsUpdateHandler.promises.deleteUpdate(
@@ -601,17 +471,6 @@ function resolveProjectByName() {
})
}
function resolveProjectById() {
beforeEach(async function (ctx) {
ctx.resolvedProject =
await ctx.TpdsUpdateHandler.promises.getOrCreateProject(
ctx.userId,
ctx.projectId,
'' // projectName
)
})
}
/* Expectations */
function expectProjectCreated() {