diff --git a/services/web/app/src/Features/Project/ProjectGetter.js b/services/web/app/src/Features/Project/ProjectGetter.js index 9693bc429d..55e400e0f5 100644 --- a/services/web/app/src/Features/Project/ProjectGetter.js +++ b/services/web/app/src/Features/Project/ProjectGetter.js @@ -1,156 +1,129 @@ const { db } = require('../../infrastructure/mongodb') const { normalizeQuery } = require('../Helpers/Mongo') const OError = require('@overleaf/o-error') -const metrics = require('@overleaf/metrics') -const { promisifyAll } = require('@overleaf/promise-utils') const { Project } = require('../../models/Project') -const logger = require('@overleaf/logger') const LockManager = require('../../infrastructure/LockManager') const { DeletedProject } = require('../../models/DeletedProject') +const { callbackifyAll } = require('@overleaf/promise-utils') const ProjectGetter = { EXCLUDE_DEPTH: 8, - getProjectWithoutDocLines(projectId, callback) { + async getProjectWithoutDocLines(projectId) { const excludes = {} for (let i = 1; i <= ProjectGetter.EXCLUDE_DEPTH; i++) { excludes[`rootFolder${Array(i).join('.folders')}.docs.lines`] = 0 } - ProjectGetter.getProject(projectId, excludes, callback) + return await ProjectGetter.getProject(projectId, excludes) }, - getProjectWithOnlyFolders(projectId, callback) { + async getProjectWithOnlyFolders(projectId) { const excludes = {} for (let i = 1; i <= ProjectGetter.EXCLUDE_DEPTH; i++) { excludes[`rootFolder${Array(i).join('.folders')}.docs`] = 0 excludes[`rootFolder${Array(i).join('.folders')}.fileRefs`] = 0 } - ProjectGetter.getProject(projectId, excludes, callback) + return await ProjectGetter.getProject(projectId, excludes) }, - getProject(projectId, projection, callback) { - if (typeof projection === 'function' && callback == null) { - callback = projection - projection = {} - } + async getProject(projectId, projection = {}) { if (projectId == null) { - return callback(new Error('no project id provided')) + throw new Error('no project id provided') } if (typeof projection !== 'object') { - return callback(new Error('projection is not an object')) + throw new Error('projection is not an object') } if (projection.rootFolder || Object.keys(projection).length === 0) { const ProjectEntityMongoUpdateHandler = require('./ProjectEntityMongoUpdateHandler') - LockManager.runWithLock( + return await LockManager.promises.runWithLock( ProjectEntityMongoUpdateHandler.LOCK_NAMESPACE, projectId, - cb => ProjectGetter.getProjectWithoutLock(projectId, projection, cb), - callback + () => ProjectGetter.getProjectWithoutLock(projectId, projection) ) } else { - ProjectGetter.getProjectWithoutLock(projectId, projection, callback) + return await ProjectGetter.getProjectWithoutLock(projectId, projection) } }, - getProjectWithoutLock(projectId, projection, callback) { - if (typeof projection === 'function' && callback == null) { - callback = projection - projection = {} - } + async getProjectWithoutLock(projectId, projection = {}) { if (projectId == null) { - return callback(new Error('no project id provided')) + throw new Error('no project id provided') } if (typeof projection !== 'object') { - return callback(new Error('projection is not an object')) + throw new Error('projection is not an object') } - let query + const query = normalizeQuery(projectId) + + let project try { - query = normalizeQuery(projectId) - } catch (err) { - return callback(err) + project = await db.projects.findOne(query, { projection }) + } catch (error) { + OError.tag(error, 'error getting project', { + query, + projection, + }) + throw error } - db.projects.findOne(query, { projection }, function (err, project) { - if (err) { - OError.tag(err, 'error getting project', { - query, - projection, - }) - return callback(err) - } - callback(null, project) - }) + return project }, - getProjectIdByReadAndWriteToken(token, callback) { - Project.findOne( + async getProjectIdByReadAndWriteToken(token) { + const project = await Project.findOne( { 'tokens.readAndWrite': token }, - { _id: 1 }, - function (err, project) { - if (err) { - return callback(err) - } - if (project == null) { - return callback() - } - callback(null, project._id) - } - ) + { _id: 1 } + ).exec() + + if (project == null) { + return + } + + return project._id }, - findAllUsersProjects(userId, fields, callback) { + async findAllUsersProjects(userId, fields) { const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter') - Project.find( + const ownedProjects = await Project.find( { owner_ref: userId }, - fields, - function (error, ownedProjects) { - if (error) { - return callback(error) + fields + ).exec() + + const projects = + await CollaboratorsGetter.promises.getProjectsUserIsMemberOf( + userId, + fields + ) + + const result = { + owned: ownedProjects || [], + readAndWrite: projects.readAndWrite || [], + readOnly: projects.readOnly || [], + tokenReadAndWrite: projects.tokenReadAndWrite || [], + tokenReadOnly: projects.tokenReadOnly || [], + } + + // Remove duplicate projects. The order of result values is determined by the order they occur. + const tempAddedProjectsIds = new Set() + const filteredProjects = Object.entries(result).reduce((prev, current) => { + const [key, projects] = current + + prev[key] = [] + + projects.forEach(project => { + const projectId = project._id.toString() + + if (!tempAddedProjectsIds.has(projectId)) { + prev[key].push(project) + tempAddedProjectsIds.add(projectId) } - CollaboratorsGetter.getProjectsUserIsMemberOf( - userId, - fields, - function (error, projects) { - if (error) { - return callback(error) - } - const result = { - owned: ownedProjects || [], - readAndWrite: projects.readAndWrite || [], - readOnly: projects.readOnly || [], - tokenReadAndWrite: projects.tokenReadAndWrite || [], - tokenReadOnly: projects.tokenReadOnly || [], - } + }) - // Remove duplicate projects. The order of result values is determined by the order they occur. - const tempAddedProjectsIds = new Set() - const filteredProjects = Object.entries(result).reduce( - (prev, current) => { - const [key, projects] = current + return prev + }, {}) - prev[key] = [] - - projects.forEach(project => { - const projectId = project._id.toString() - - if (!tempAddedProjectsIds.has(projectId)) { - prev[key].push(project) - tempAddedProjectsIds.add(projectId) - } - }) - - return prev - }, - {} - ) - - callback(null, filteredProjects) - } - ) - } - ) + return filteredProjects }, /** @@ -159,38 +132,28 @@ const ProjectGetter = { * Projects include the user's own projects as well as collaborations with * read/write access. */ - findUsersProjectsByName(userId, projectName, callback) { - ProjectGetter.findAllUsersProjects( + async findUsersProjectsByName(userId, projectName) { + const allProjects = await ProjectGetter.findAllUsersProjects( userId, - 'name archived trashed', - (err, allProjects) => { - if (err != null) { - return callback(err) - } - const { owned, readAndWrite } = allProjects - const projects = owned.concat(readAndWrite) - const lowerCasedProjectName = projectName.toLowerCase() - const matches = projects.filter( - project => project.name.toLowerCase() === lowerCasedProjectName - ) - callback(null, matches) - } + 'name archived trashed' + ) + + const { owned, readAndWrite } = allProjects + const projects = owned.concat(readAndWrite) + const lowerCasedProjectName = projectName.toLowerCase() + return projects.filter( + project => project.name.toLowerCase() === lowerCasedProjectName ) }, - getUsersDeletedProjects(userId, callback) { - DeletedProject.find( - { - 'deleterData.deletedProjectOwnerId': userId, - }, - callback - ) + async getUsersDeletedProjects(userId) { + return await DeletedProject.find({ + 'deleterData.deletedProjectOwnerId': userId, + }).exec() }, } -;['getProject', 'getProjectWithoutDocLines'].map(method => - metrics.timeAsyncMethod(ProjectGetter, method, 'mongo.ProjectGetter', logger) -) - -ProjectGetter.promises = promisifyAll(ProjectGetter) -module.exports = ProjectGetter +module.exports = { + ...callbackifyAll(ProjectGetter), + promises: ProjectGetter, +} diff --git a/services/web/test/unit/src/Project/ProjectGetterTests.js b/services/web/test/unit/src/Project/ProjectGetterTests.js index 108723bc24..5a85eff753 100644 --- a/services/web/test/unit/src/Project/ProjectGetterTests.js +++ b/services/web/test/unit/src/Project/ProjectGetterTests.js @@ -6,35 +6,44 @@ const { ObjectId } = require('mongodb') describe('ProjectGetter', function () { beforeEach(function () { - this.callback = sinon.stub() this.project = { _id: new ObjectId() } this.projectIdStr = this.project._id.toString() this.deletedProject = { deleterData: { wombat: 'potato' } } this.userId = new ObjectId() this.DeletedProject = { - find: sinon.stub().yields(null, [this.deletedProject]), - } - this.Project = { - find: sinon.stub(), - findOne: sinon.stub().yields(null, this.project), - } - this.CollaboratorsGetter = { - getProjectsUserIsMemberOf: sinon.stub().yields(null, { - readAndWrite: [], - readOnly: [], - tokenReadAndWrite: [], - tokenReadOnly: [], + find: sinon.stub().returns({ + exec: sinon.stub().resolves([this.deletedProject]), }), } + this.Project = { + find: sinon.stub().returns({ + exec: sinon.stub().resolves(), + }), + findOne: sinon.stub().returns({ + exec: sinon.stub().resolves(this.project), + }), + } + this.CollaboratorsGetter = { + promises: { + getProjectsUserIsMemberOf: sinon.stub().resolves({ + readAndWrite: [], + readOnly: [], + tokenReadAndWrite: [], + tokenReadOnly: [], + }), + }, + } this.LockManager = { - runWithLock: sinon - .stub() - .callsFake((namespace, id, runner, callback) => runner(callback)), + promises: { + runWithLock: sinon + .stub() + .callsFake((namespace, id, runner) => runner()), + }, } this.db = { projects: { - findOne: sinon.stub().yields(null, this.project), + findOne: sinon.stub().resolves(this.project), }, users: {}, } @@ -63,19 +72,18 @@ describe('ProjectGetter', function () { describe('getProjectWithoutDocLines', function () { beforeEach(function () { - this.ProjectGetter.getProject = sinon.stub().yields() + this.ProjectGetter.promises.getProject = sinon.stub().resolves() }) describe('passing an id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithoutDocLines( - this.project._id, - this.callback + beforeEach(async function () { + await this.ProjectGetter.promises.getProjectWithoutDocLines( + this.project._id ) }) it('should call find with the project id', function () { - this.ProjectGetter.getProject + this.ProjectGetter.promises.getProject .calledWith(this.project._id) .should.equal(true) }) @@ -92,37 +100,32 @@ describe('ProjectGetter', function () { 'rootFolder.folders.folders.folders.folders.folders.folders.folders.docs.lines': 0, } - this.ProjectGetter.getProject + this.ProjectGetter.promises.getProject .calledWith(this.project._id, excludes) .should.equal(true) }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) - }) }) }) describe('getProjectWithOnlyFolders', function () { beforeEach(function () { - this.ProjectGetter.getProject = sinon.stub().yields() + this.ProjectGetter.promises.getProject = sinon.stub().resolves() }) describe('passing an id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithOnlyFolders( - this.project._id, - this.callback + beforeEach(async function () { + await this.ProjectGetter.promises.getProjectWithOnlyFolders( + this.project._id ) }) it('should call find with the project id', function () { - this.ProjectGetter.getProject + this.ProjectGetter.promises.getProject .calledWith(this.project._id) .should.equal(true) }) - it('should exclude the docs and files linesaaaa', function () { + it('should exclude the docs and files lines', function () { const excludes = { 'rootFolder.docs': 0, 'rootFolder.fileRefs': 0, @@ -141,22 +144,18 @@ describe('ProjectGetter', function () { 'rootFolder.folders.folders.folders.folders.folders.folders.folders.docs': 0, 'rootFolder.folders.folders.folders.folders.folders.folders.folders.fileRefs': 0, } - this.ProjectGetter.getProject + this.ProjectGetter.promises.getProject .calledWith(this.project._id, excludes) .should.equal(true) }) - - it('should call the callback with the project', function () { - this.callback.called.should.equal(true) - }) }) }) describe('getProject', function () { describe('without projection', function () { describe('with project id', function () { - beforeEach(function () { - this.ProjectGetter.getProject(this.projectIdStr, this.callback) + beforeEach(async function () { + await this.ProjectGetter.promises.getProject(this.projectIdStr) }) it('should call findOne with the project id', function () { @@ -168,13 +167,11 @@ describe('ProjectGetter', function () { }) describe('without project id', function () { - beforeEach(function () { - this.ProjectGetter.getProject(null, this.callback) - }) - - it('should callback with error', function () { + it('should be rejected', function () { + expect( + this.ProjectGetter.promises.getProject(null) + ).to.be.rejectedWith('no project id provided') expect(this.db.projects.findOne.callCount).to.equal(0) - expect(this.callback.lastCall.args[0]).to.be.instanceOf(Error) }) }) }) @@ -185,11 +182,10 @@ describe('ProjectGetter', function () { }) describe('with project id', function () { - beforeEach(function () { - this.ProjectGetter.getProject( + beforeEach(async function () { + await this.ProjectGetter.promises.getProject( this.projectIdStr, - this.projection, - this.callback + this.projection ) }) @@ -205,13 +201,11 @@ describe('ProjectGetter', function () { }) describe('without project id', function () { - beforeEach(function () { - this.ProjectGetter.getProject(null, this.callback) - }) - - it('should callback with error', function () { + it('should be rejected', function () { + expect( + this.ProjectGetter.promises.getProject(null) + ).to.be.rejectedWith('no project id provided') expect(this.db.projects.findOne.callCount).to.equal(0) - expect(this.callback.lastCall.args[0]).to.be.instanceOf(Error) }) }) }) @@ -220,10 +214,9 @@ describe('ProjectGetter', function () { describe('getProjectWithoutLock', function () { describe('without projection', function () { describe('with project id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithoutLock( - this.projectIdStr, - this.callback + beforeEach(async function () { + await this.ProjectGetter.promises.getProjectWithoutLock( + this.projectIdStr ) }) @@ -236,13 +229,11 @@ describe('ProjectGetter', function () { }) describe('without project id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithoutLock(null, this.callback) - }) - - it('should callback with error', function () { + it('should be rejected', function () { + expect( + this.ProjectGetter.promises.getProjectWithoutLock(null) + ).to.be.rejectedWith('no project id provided') expect(this.db.projects.findOne.callCount).to.equal(0) - expect(this.callback.lastCall.args[0]).to.be.instanceOf(Error) }) }) }) @@ -253,11 +244,10 @@ describe('ProjectGetter', function () { }) describe('with project id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithoutLock( + beforeEach(async function () { + await this.ProjectGetter.promises.getProjectWithoutLock( this.project._id, - this.projection, - this.callback + this.projection ) }) @@ -273,13 +263,11 @@ describe('ProjectGetter', function () { }) describe('without project id', function () { - beforeEach(function () { - this.ProjectGetter.getProjectWithoutLock(null, this.callback) - }) - - it('should callback with error', function () { + it('should be rejected', function () { + expect( + this.ProjectGetter.promises.getProjectWithoutLock(null) + ).to.be.rejectedWith('no project id provided') expect(this.db.projects.findOne.callCount).to.equal(0) - expect(this.callback.lastCall.args[0]).to.be.instanceOf(Error) }) }) }) @@ -295,35 +283,32 @@ describe('ProjectGetter', function () { this.projectTokenRO = { _id: 'mock-token-ro-projects' } this.Project.find .withArgs({ owner_ref: this.userId }, this.fields) - .yields(null, [this.projectOwned]) + .returns({ exec: sinon.stub().resolves([this.projectOwned]) }) }) - it('should call the callback with all the projects', function () { - this.CollaboratorsGetter.getProjectsUserIsMemberOf.yields(null, { + it('should return a promise with all the projects', async function () { + this.CollaboratorsGetter.promises.getProjectsUserIsMemberOf.resolves({ readAndWrite: [this.projectRW], readOnly: [this.projectRO], tokenReadAndWrite: [this.projectTokenRW], tokenReadOnly: [this.projectTokenRO], }) - this.ProjectGetter.findAllUsersProjects( + const projects = await this.ProjectGetter.promises.findAllUsersProjects( this.userId, - this.fields, - this.callback + this.fields ) - this.callback - .calledWith(null, { - owned: [this.projectOwned], - readAndWrite: [this.projectRW], - readOnly: [this.projectRO], - tokenReadAndWrite: [this.projectTokenRW], - tokenReadOnly: [this.projectTokenRO], - }) - .should.equal(true) + expect(projects).to.deep.equal({ + owned: [this.projectOwned], + readAndWrite: [this.projectRW], + readOnly: [this.projectRO], + tokenReadAndWrite: [this.projectTokenRW], + tokenReadOnly: [this.projectTokenRO], + }) }) - it('should remove duplicate projects', function () { - this.CollaboratorsGetter.getProjectsUserIsMemberOf.yields(null, { + it('should remove duplicate projects', async function () { + this.CollaboratorsGetter.promises.getProjectsUserIsMemberOf.resolves({ readAndWrite: [this.projectRW, this.projectOwned], readOnly: [this.projectRO, this.projectRW], tokenReadAndWrite: [this.projectTokenRW, this.projectRO], @@ -333,31 +318,28 @@ describe('ProjectGetter', function () { this.projectRO, ], }) - this.ProjectGetter.findAllUsersProjects( + const projects = await this.ProjectGetter.promises.findAllUsersProjects( this.userId, - this.fields, - this.callback + this.fields ) - this.callback - .calledWith(null, { - owned: [this.projectOwned], - readAndWrite: [this.projectRW], - readOnly: [this.projectRO], - tokenReadAndWrite: [this.projectTokenRW], - tokenReadOnly: [this.projectTokenRO], - }) - .should.equal(true) + expect(projects).to.deep.equal({ + owned: [this.projectOwned], + readAndWrite: [this.projectRW], + readOnly: [this.projectRO], + tokenReadAndWrite: [this.projectTokenRW], + tokenReadOnly: [this.projectTokenRO], + }) }) }) describe('getProjectIdByReadAndWriteToken', function () { describe('when project find returns project', function () { - this.beforeEach(function () { - this.ProjectGetter.getProjectIdByReadAndWriteToken( - 'token', - this.callback - ) + this.beforeEach(async function () { + this.projectIdFound = + await this.ProjectGetter.promises.getProjectIdByReadAndWriteToken( + 'token' + ) }) it('should find project with token', function () { @@ -366,73 +348,66 @@ describe('ProjectGetter', function () { .should.equal(true) }) - it('should callback with project id', function () { - this.callback.calledWith(null, this.project._id).should.equal(true) + it('should return the project id', function () { + expect(this.projectIdFound).to.equal(this.project._id) }) }) describe('when project not found', function () { - this.beforeEach(function () { - this.Project.findOne.yields(null, null) - this.ProjectGetter.getProjectIdByReadAndWriteToken( - 'token', - this.callback - ) - }) + it('should return undefined', async function () { + this.Project.findOne.returns({ exec: sinon.stub().resolves(null) }) + const projectId = + await this.ProjectGetter.promises.getProjectIdByReadAndWriteToken( + 'token' + ) - it('should callback empty', function () { - expect(this.callback.firstCall.args.length).to.equal(0) + expect(projectId).to.equal(undefined) }) }) describe('when project find returns error', function () { - this.beforeEach(function () { - this.Project.findOne.yields('error') - this.ProjectGetter.getProjectIdByReadAndWriteToken( - 'token', - this.callback - ) + this.beforeEach(async function () { + this.Project.findOne.returns({ exec: sinon.stub().rejects() }) }) - it('should callback with error', function () { - this.callback.calledWith('error').should.equal(true) + it('should be rejected', function () { + expect( + this.ProjectGetter.promises.getProjectIdByReadAndWriteToken('token') + ).to.be.rejected }) }) }) describe('findUsersProjectsByName', function () { - it('should perform a case-insensitive search', function (done) { + it('should perform a case-insensitive search', async function () { this.project1 = { _id: 1, name: 'find me!' } this.project2 = { _id: 2, name: 'not me!' } this.project3 = { _id: 3, name: 'FIND ME!' } this.project4 = { _id: 4, name: 'Find Me!' } - this.Project.find - .withArgs({ owner_ref: this.userId }) - .yields(null, [ - this.project1, - this.project2, - this.project3, - this.project4, - ]) - this.ProjectGetter.findUsersProjectsByName( - this.userId, + this.Project.find.withArgs({ owner_ref: this.userId }).returns({ + exec: sinon + .stub() + .resolves([ + this.project1, + this.project2, + this.project3, + this.project4, + ]), + }) + const projects = + await this.ProjectGetter.promises.findUsersProjectsByName( + this.userId, + this.project1.name + ) + const projectNames = projects.map(project => project.name) + expect(projectNames).to.have.members([ this.project1.name, - (err, projects) => { - if (err != null) { - return done(err) - } - const projectNames = projects.map(project => project.name) - expect(projectNames).to.have.members([ - this.project1.name, - this.project3.name, - this.project4.name, - ]) - done() - } - ) + this.project3.name, + this.project4.name, + ]) }) - it('should search collaborations as well', function (done) { + it('should search collaborations as well', async function () { this.project1 = { _id: 1, name: 'find me!' } this.project2 = { _id: 2, name: 'FIND ME!' } this.project3 = { _id: 3, name: 'Find Me!' } @@ -440,51 +415,38 @@ describe('ProjectGetter', function () { this.project5 = { _id: 5, name: 'FIND me!' } this.Project.find .withArgs({ owner_ref: this.userId }) - .yields(null, [this.project1]) - this.CollaboratorsGetter.getProjectsUserIsMemberOf.yields(null, { + .returns({ exec: sinon.stub().resolves([this.project1]) }) + this.CollaboratorsGetter.promises.getProjectsUserIsMemberOf.resolves({ readAndWrite: [this.project2], readOnly: [this.project3], tokenReadAndWrite: [this.project4], tokenReadOnly: [this.project5], }) - this.ProjectGetter.findUsersProjectsByName( - this.userId, + const projects = + await this.ProjectGetter.promises.findUsersProjectsByName( + this.userId, + this.project1.name + ) + expect(projects.map(project => project.name)).to.have.members([ this.project1.name, - (err, projects) => { - if (err != null) { - return done(err) - } - expect(projects.map(project => project.name)).to.have.members([ - this.project1.name, - this.project2.name, - ]) - done() - } - ) + this.project2.name, + ]) }) }) describe('getUsersDeletedProjects', function () { - it('should look up the deleted projects by deletedProjectOwnerId', function (done) { - this.ProjectGetter.getUsersDeletedProjects('giraffe', err => { - if (err) { - return done(err) - } - sinon.assert.calledWith(this.DeletedProject.find, { - 'deleterData.deletedProjectOwnerId': 'giraffe', - }) - done() + it('should look up the deleted projects by deletedProjectOwnerId', async function () { + await this.ProjectGetter.promises.getUsersDeletedProjects('giraffe') + sinon.assert.calledWith(this.DeletedProject.find, { + 'deleterData.deletedProjectOwnerId': 'giraffe', }) }) - it('should pass the found projects to the callback', function (done) { - this.ProjectGetter.getUsersDeletedProjects('giraffe', (err, docs) => { - if (err) { - return done(err) - } - expect(docs).to.deep.equal([this.deletedProject]) - done() - }) + it('should pass the found projects to the callback', async function () { + const docs = await this.ProjectGetter.promises.getUsersDeletedProjects( + 'giraffe' + ) + expect(docs).to.deep.equal([this.deletedProject]) }) }) })