diff --git a/services/web/app/src/Features/Project/ProjectDeleter.js b/services/web/app/src/Features/Project/ProjectDeleter.js index e7ca8552c3..b4c9734a37 100644 --- a/services/web/app/src/Features/Project/ProjectDeleter.js +++ b/services/web/app/src/Features/Project/ProjectDeleter.js @@ -382,7 +382,7 @@ async function expireDeletedProject(projectId) { : Promise.resolve(), FilestoreHandler.promises.deleteProject(deletedProject.project._id), TpdsUpdateSender.promises.deleteProject({ - project_id: deletedProject.project._id, + projectId: deletedProject.project._id, }), ChatApiHandler.promises.destroyProject(deletedProject.project._id), hardDeleteDeletedFiles(deletedProject.project._id), diff --git a/services/web/app/src/Features/Project/ProjectDetailsHandler.js b/services/web/app/src/Features/Project/ProjectDetailsHandler.js index 48de5f01a3..20d9c0401b 100644 --- a/services/web/app/src/Features/Project/ProjectDetailsHandler.js +++ b/services/web/app/src/Features/Project/ProjectDetailsHandler.js @@ -114,8 +114,8 @@ async function renameProject(projectId, newName) { const oldProjectName = project.name await Project.updateOne({ _id: projectId }, { name: newName }).exec() await TpdsUpdateSender.promises.moveEntity({ - project_id: projectId, - project_name: oldProjectName, + projectId, + projectName: oldProjectName, newProjectName: newName, }) } diff --git a/services/web/app/src/Features/Project/ProjectEntityHandler.js b/services/web/app/src/Features/Project/ProjectEntityHandler.js index 304681a4de..d2bc808c40 100644 --- a/services/web/app/src/Features/Project/ProjectEntityHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityHandler.js @@ -35,6 +35,7 @@ const ProjectEntityHandler = { name: doc.name, lines: content.lines, rev: content.rev, + folder, } } } @@ -54,7 +55,7 @@ const ProjectEntityHandler = { for (const { path: folderPath, folder } of folders) { for (const file of iterablePaths(folder, 'fileRefs')) { if (file != null) { - files[path.join(folderPath, file.name)] = file + files[path.join(folderPath, file.name)] = { ...file, folder } } } } diff --git a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js index ae5d2e1776..7ccad7c604 100644 --- a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js @@ -88,7 +88,7 @@ function getDocContext(projectId, docId, callback) { } ProjectLocator.findElement( { project, element_id: docId, type: 'docs' }, - (err, doc, path) => { + (err, doc, path, folder) => { if (err && err instanceof Errors.NotFoundError) { // (Soft-)Deleted docs are removed from the file-tree (rootFolder). // docstore can tell whether it exists and is (soft)-deleted. @@ -127,6 +127,7 @@ function getDocContext(projectId, docId, callback) { projectName: project.name, isDeletedDoc: true, path: null, + folder: null, }) } } @@ -143,6 +144,7 @@ function getDocContext(projectId, docId, callback) { projectName: project.name, isDeletedDoc: false, path: path.fileSystem, + folder, }) } } @@ -174,7 +176,7 @@ const ProjectEntityUpdateHandler = { if (err) { return callback(err) } - const { projectName, isDeletedDoc, path } = ctx + const { projectName, isDeletedDoc, path, folder } = ctx logger.debug( { projectId, docId }, 'telling docstore manager to update doc' @@ -209,11 +211,12 @@ const ProjectEntityUpdateHandler = { ) TpdsUpdateSender.addDoc( { - project_id: projectId, + projectId, path, - doc_id: docId, - project_name: projectName, + docId, + projectName, rev, + folderId: folder?._id, }, callback ) @@ -281,11 +284,12 @@ const ProjectEntityUpdateHandler = { } TpdsUpdateSender.addDoc( { - project_id: projectId, - doc_id: doc != null ? doc._id : undefined, - path: result && result.path && result.path.fileSystem, - project_name: project.name, + projectId, + docId: doc != null ? doc._id : undefined, + path: result?.path?.fileSystem, + projectName: project.name, rev: 0, + folderId, }, err => { if (err != null) { @@ -447,11 +451,12 @@ const ProjectEntityUpdateHandler = { } TpdsUpdateSender.addFile( { - project_id: projectId, - file_id: fileRef._id, - path: result && result.path && result.path.fileSystem, - project_name: project.name, + projectId, + fileId: fileRef._id, + path: result?.path?.fileSystem, + projectName: project.name, rev: fileRef.rev, + folderId, }, err => { if (err != null) { @@ -555,115 +560,77 @@ const ProjectEntityUpdateHandler = { }, }), - replaceFile: wrapWithLock({ - beforeLock(next) { - return function ( - projectId, - fileId, - fsPath, - linkedFileData, - userId, - source, - callback - ) { - // create a new file - const fileArgs = { - name: 'dummy-upload-filename', - linkedFileData, + _replaceFile( + projectId, + fileId, + fsPath, + linkedFileData, + userId, + newFileRef, + fileStoreUrl, + folderId, + source, + callback + ) { + ProjectEntityMongoUpdateHandler.replaceFileWithNew( + projectId, + fileId, + newFileRef, + (err, oldFileRef, project, path, newProject) => { + if (err != null) { + return callback(err) } - FileStoreHandler.uploadFileFromDisk( - projectId, - fileArgs, - fsPath, - (err, fileStoreUrl, fileRef) => { + const oldFiles = [ + { + file: oldFileRef, + path: path.fileSystem, + }, + ] + const newFiles = [ + { + file: newFileRef, + path: path.fileSystem, + url: fileStoreUrl, + }, + ] + const projectHistoryId = + project.overleaf && + project.overleaf.history && + project.overleaf.history.id + // Increment the rev for an in-place update (with the same path) so the third-party-datastore + // knows this is a new file. + // Ideally we would get this from ProjectEntityMongoUpdateHandler.replaceFileWithNew + // but it returns the original oldFileRef (after incrementing the rev value in mongo), + // so we add 1 to the rev from that. This isn't atomic and relies on the lock + // but it is acceptable for now. + TpdsUpdateSender.addFile( + { + projectId: project._id, + fileId: newFileRef._id, + path: path.fileSystem, + rev: oldFileRef.rev + 1, + projectName: project.name, + folderId, + }, + err => { if (err != null) { return callback(err) } - next( + ProjectUpdateHandler.markAsUpdated(projectId, new Date(), userId) + + DocumentUpdaterHandler.updateProjectStructure( projectId, - fileId, - fsPath, - linkedFileData, + projectHistoryId, userId, - fileRef, - fileStoreUrl, + { oldFiles, newFiles, newProject }, source, callback ) } ) } - }, - withLock( - projectId, - fileId, - fsPath, - linkedFileData, - userId, - newFileRef, - fileStoreUrl, - source, - callback - ) { - ProjectEntityMongoUpdateHandler.replaceFileWithNew( - projectId, - fileId, - newFileRef, - (err, oldFileRef, project, path, newProject) => { - if (err != null) { - return callback(err) - } - const oldFiles = [ - { - file: oldFileRef, - path: path.fileSystem, - }, - ] - const newFiles = [ - { - file: newFileRef, - path: path.fileSystem, - url: fileStoreUrl, - }, - ] - const projectHistoryId = - project.overleaf && - project.overleaf.history && - project.overleaf.history.id - // Increment the rev for an in-place update (with the same path) so the third-party-datastore - // knows this is a new file. - // Ideally we would get this from ProjectEntityMongoUpdateHandler.replaceFileWithNew - // but it returns the original oldFileRef (after incrementing the rev value in mongo), - // so we add 1 to the rev from that. This isn't atomic and relies on the lock - // but it is acceptable for now. - TpdsUpdateSender.addFile( - { - project_id: project._id, - file_id: newFileRef._id, - path: path.fileSystem, - rev: oldFileRef.rev + 1, - project_name: project.name, - }, - err => { - if (err != null) { - return callback(err) - } - ProjectUpdateHandler.markAsUpdated(projectId, new Date(), userId) - - DocumentUpdaterHandler.updateProjectStructure( - projectId, - projectHistoryId, - userId, - { oldFiles, newFiles, newProject }, - source, - callback - ) - } - ) - } - ) - }, - }), + ) + }, upsertDoc: wrapWithLock(function ( projectId, @@ -713,11 +680,12 @@ const ProjectEntityUpdateHandler = { } TpdsUpdateSender.addDoc( { - project_id: projectId, - doc_id: doc._id, + projectId, + docId: doc._id, path: filePath, - project_name: project.name, + projectName: project.name, rev: existingFile.rev + 1, + folderId, }, err => { if (err) { @@ -908,11 +876,12 @@ const ProjectEntityUpdateHandler = { project.overleaf.history.id TpdsUpdateSender.addFile( { - project_id: project._id, - file_id: newFileRef._id, + projectId: project._id, + fileId: newFileRef._id, path: path.fileSystem, rev: newFileRef.rev, - project_name: project.name, + projectName: project.name, + folderId, }, err => { if (err) { @@ -957,8 +926,7 @@ const ProjectEntityUpdateHandler = { } ) } else if (existingFile) { - // this calls directly into the replaceFile main task (without the beforeLock part) - return ProjectEntityUpdateHandler.replaceFile.mainTask( + return ProjectEntityUpdateHandler._replaceFile( projectId, existingFile._id, fsPath, @@ -966,6 +934,7 @@ const ProjectEntityUpdateHandler = { userId, newFileRef, fileStoreUrl, + folderId, source, err => { if (err != null) { @@ -1167,9 +1136,11 @@ const ProjectEntityUpdateHandler = { } TpdsUpdateSender.deleteEntity( { - project_id: projectId, + projectId, path: path.fileSystem, - project_name: projectBeforeDeletion.name, + projectName: projectBeforeDeletion.name, + entityId, + entityType, }, error => { if (error != null) { @@ -1286,11 +1257,14 @@ const ProjectEntityUpdateHandler = { // do not wait TpdsUpdateSender.promises .moveEntity({ - project_id: projectId, - project_name: project.name, + projectId, + projectName: project.name, startPath, endPath, rev, + entityId, + entityType, + folderId: destFolderId, }) .catch(err => { logger.error({ err }, 'error sending tpds update') @@ -1342,11 +1316,14 @@ const ProjectEntityUpdateHandler = { // do not wait TpdsUpdateSender.promises .moveEntity({ - project_id: projectId, - project_name: project.name, + projectId, + projectName: project.name, startPath, endPath, rev, + entityId, + entityType, + folderId: null, // this means the folder has not changed }) .catch(err => { logger.error({ err }, 'error sending tpds update') diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js index a5407897d7..75d262396d 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js @@ -51,20 +51,22 @@ async function _flushProjectToTpds(project) { ]) for (const [docPath, doc] of Object.entries(docs)) { await TpdsUpdateSender.promises.addDoc({ - project_id: project._id, - doc_id: doc._id, + projectId: project._id, + docId: doc._id, path: docPath, - project_name: project.name, + projectName: project.name, rev: doc.rev || 0, + folderId: doc.folder._id, }) } for (const [filePath, file] of Object.entries(files)) { await TpdsUpdateSender.promises.addFile({ - project_id: project._id, - file_id: file._id, + projectId: project._id, + fileId: file._id, path: filePath, - project_name: project.name, + projectName: project.name, rev: file.rev, + folderId: file.folder._id, }) } await _resetDeferredTpdsFlushCounter(project) diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js index 67672b8277..448e079cf4 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js @@ -3,7 +3,7 @@ const _ = require('lodash') const { callbackify } = require('util') const logger = require('@overleaf/logger') const metrics = require('@overleaf/metrics') -const path = require('path') +const Path = require('path') const request = require('request-promise-native') const settings = require('@overleaf/settings') @@ -13,90 +13,116 @@ const UserGetter = require('../User/UserGetter.js').promises const tpdsUrl = _.get(settings, ['apis', 'thirdPartyDataStore', 'url']) -async function addDoc(options) { +async function addDoc(params) { metrics.inc('tpds.add-doc') + const { projectId, path, docId, projectName, rev, folderId } = params - options.streamOrigin = + const streamOrigin = settings.apis.docstore.pubUrl + - path.join( - `/project/${options.project_id}`, - `/doc/${options.doc_id}`, - '/raw' - ) - options.entity_id = options.doc_id - options.entity_type = 'doc' + Path.join(`/project/${projectId}`, `/doc/${docId}`, '/raw') - return addEntity(options) + await addEntity({ + projectId, + path, + projectName, + rev, + folderId, + streamOrigin, + entityId: docId, + entityType: 'doc', + }) } -async function addEntity(options) { - const projectUserIds = await getProjectUsersIds(options.project_id) +async function addEntity(params) { + const { + projectId, + path, + projectName, + rev, + folderId, + streamOrigin, + entityId, + entityType, + } = params + + const projectUserIds = await getProjectUsersIds(projectId) for (const userId of projectUserIds) { const job = { method: 'post', headers: { - sl_entity_id: options.entity_id, - sl_entity_type: options.entity_type, - sl_entity_rev: options.rev, - sl_project_id: options.project_id, + sl_entity_id: entityId, + sl_entity_type: entityType, + sl_entity_rev: rev, + sl_project_id: projectId, sl_all_user_ids: JSON.stringify([userId]), sl_project_owner_user_id: projectUserIds[0], + sl_folder_id: folderId, }, - uri: buildTpdsUrl(userId, options.project_name, options.path), + uri: buildTpdsUrl(userId, projectName, path), title: 'addFile', - streamOrigin: options.streamOrigin, + streamOrigin, } await enqueue(userId, 'pipeStreamFrom', job) } } -async function addFile(options) { +async function addFile(params) { metrics.inc('tpds.add-file') - - options.streamOrigin = + const { projectId, fileId, path, projectName, rev, folderId } = params + const streamOrigin = settings.apis.filestore.url + - path.join(`/project/${options.project_id}`, `/file/${options.file_id}`) - options.entity_id = options.file_id - options.entity_type = 'file' + Path.join(`/project/${projectId}`, `/file/${fileId}`) - return addEntity(options) + await addEntity({ + projectId, + path, + projectName, + rev, + folderId, + streamOrigin, + entityId: fileId, + entityType: 'file', + }) } -function buildMovePaths(options) { - if (options.newProjectName) { +function buildMovePaths(params) { + if (params.newProjectName) { return { - startPath: path.join('/', options.project_name, '/'), - endPath: path.join('/', options.newProjectName, '/'), + startPath: Path.join('/', params.projectName, '/'), + endPath: Path.join('/', params.newProjectName, '/'), } } else { return { - startPath: path.join('/', options.project_name, '/', options.startPath), - endPath: path.join('/', options.project_name, '/', options.endPath), + startPath: Path.join('/', params.projectName, '/', params.startPath), + endPath: Path.join('/', params.projectName, '/', params.endPath), } } } function buildTpdsUrl(userId, projectName, filePath) { - const projectPath = encodeURIComponent(path.join(projectName, '/', filePath)) + const projectPath = encodeURIComponent(Path.join(projectName, '/', filePath)) return `${tpdsUrl}/user/${userId}/entity/${projectPath}` } -async function deleteEntity(options) { +async function deleteEntity(params) { metrics.inc('tpds.delete-entity') + const { projectId, path, projectName, entityId, entityType } = params - const projectUserIds = await getProjectUsersIds(options.project_id) + const projectUserIds = await getProjectUsersIds(projectId) for (const userId of projectUserIds) { const job = { method: 'delete', headers: { - sl_project_id: options.project_id, + sl_project_id: projectId, sl_all_user_ids: JSON.stringify([userId]), sl_project_owner_user_id: projectUserIds[0], + sl_entity_id: entityId, + sl_entity_type: entityType, }, - uri: buildTpdsUrl(userId, options.project_name, options.path), + uri: buildTpdsUrl(userId, projectName, path), title: 'deleteEntity', sl_all_user_ids: JSON.stringify([userId]), } @@ -105,7 +131,8 @@ async function deleteEntity(options) { } } -async function deleteProject(options) { +async function deleteProject(params) { + const { projectId } = params // deletion only applies to project archiver const projectArchiverUrl = _.get(settings, [ 'apis', @@ -120,13 +147,13 @@ async function deleteProject(options) { // send the request directly to project archiver, bypassing third-party-datastore try { await request({ - uri: `${settings.apis.project_archiver.url}/project/${options.project_id}`, + uri: `${settings.apis.project_archiver.url}/project/${projectId}`, method: 'delete', }) return true } catch (err) { logger.error( - { err, project_id: options.project_id }, + { err, projectId }, 'error deleting project in third party datastore (project_archiver)' ) return false @@ -176,23 +203,34 @@ async function getProjectUsersIds(projectId) { return [ownerUserId, ...dropboxUserIds] } -async function moveEntity(options) { +async function moveEntity(params) { metrics.inc('tpds.move-entity') + const { projectId, rev, entityId, entityType, folderId } = params - const projectUserIds = await getProjectUsersIds(options.project_id) - const { endPath, startPath } = buildMovePaths(options) + const projectUserIds = await getProjectUsersIds(projectId) + const { endPath, startPath } = buildMovePaths(params) for (const userId of projectUserIds) { + const headers = { + sl_project_id: projectId, + sl_entity_rev: rev, + sl_all_user_ids: JSON.stringify([userId]), + sl_project_owner_user_id: projectUserIds[0], + } + if (entityId != null) { + headers.sl_entity_id = entityId + } + if (entityType != null) { + headers.sl_entity_type = entityType + } + if (folderId != null) { + headers.sl_folder_id = folderId + } const job = { method: 'put', title: 'moveEntity', uri: `${tpdsUrl}/user/${userId}/entity`, - headers: { - sl_project_id: options.project_id, - sl_entity_rev: options.rev, - sl_all_user_ids: JSON.stringify([userId]), - sl_project_owner_user_id: projectUserIds[0], - }, + headers, json: { user_id: userId, endPath, diff --git a/services/web/scripts/delete_orphaned_project_archives.js b/services/web/scripts/delete_orphaned_project_archives.js index 4e9972d4f1..a4a67c5710 100644 --- a/services/web/scripts/delete_orphaned_project_archives.js +++ b/services/web/scripts/delete_orphaned_project_archives.js @@ -89,7 +89,7 @@ async function hardDeleteProjectArchiverData(prefix) { await sleep(1000 * i) try { const ok = await TpdsUpdateSender.promises.deleteProject({ - project_id: encodeURIComponent(prefix), + projectId: encodeURIComponent(prefix), }) if (ok) { return diff --git a/services/web/test/unit/src/Project/ProjectDeleterTests.js b/services/web/test/unit/src/Project/ProjectDeleterTests.js index f0c49e7a8e..a72bd17433 100644 --- a/services/web/test/unit/src/Project/ProjectDeleterTests.js +++ b/services/web/test/unit/src/Project/ProjectDeleterTests.js @@ -491,7 +491,7 @@ describe('ProjectDeleter', function () { expect( this.TpdsUpdateSender.promises.deleteProject ).to.have.been.calledWith({ - project_id: this.deletedProjects[0].project._id, + projectId: this.deletedProjects[0].project._id, }) }) diff --git a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js index 067876c413..0edf69eb3c 100644 --- a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js @@ -173,8 +173,8 @@ describe('ProjectDetailsHandler', function () { await this.handler.promises.renameProject(this.project._id, this.newName) expect(this.TpdsUpdateSender.promises.moveEntity).to.have.been.calledWith( { - project_id: this.project._id, - project_name: this.project.name, + projectId: this.project._id, + projectName: this.project.name, newProjectName: this.newName, } ) diff --git a/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js index f6023682a7..49efe9cad5 100644 --- a/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js @@ -112,22 +112,22 @@ describe('ProjectEntityHandler', function () { }) it('should call the callback with the docs with the lines and rev included', function () { - this.callback - .calledWith(null, { - '/doc1': { - _id: this.doc1._id, - lines: this.lines1, - name: this.doc1.name, - rev: this.rev1, - }, - '/folder1/doc2': { - _id: this.doc2._id, - lines: this.lines2, - name: this.doc2.name, - rev: this.rev2, - }, - }) - .should.equal(true) + this.callback.should.have.been.calledWith(null, { + '/doc1': { + _id: this.doc1._id, + lines: this.lines1, + name: this.doc1.name, + rev: this.rev1, + folder: this.project.rootFolder[0], + }, + '/folder1/doc2': { + _id: this.doc2._id, + lines: this.lines2, + name: this.doc2.name, + rev: this.rev2, + folder: this.folder1, + }, + }) }) }) @@ -138,12 +138,10 @@ describe('ProjectEntityHandler', function () { }) it('should call the callback with the files', function () { - this.callback - .calledWith(null, { - '/file1': this.file1, - '/folder1/file2': this.file2, - }) - .should.equal(true) + this.callback.should.have.been.calledWith(null, { + '/file1': { ...this.file1, folder: this.project.rootFolder[0] }, + '/folder1/file2': { ...this.file2, folder: this.folder1 }, + }) }) }) diff --git a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js index 3fdaf0ca85..f4cbfcaa91 100644 --- a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js @@ -184,11 +184,17 @@ describe('ProjectEntityUpdateHandler', function () { this.ranges = { mock: 'ranges' } this.lastUpdatedAt = new Date().getTime() this.lastUpdatedBy = 'fake-last-updater-id' + this.parentFolder = { _id: ObjectId() } this.DocstoreManager.isDocDeleted.yields(null, false) this.ProjectGetter.getProject.yields(null, this.project) - this.ProjectLocator.findElement.yields(null, this.doc, { - fileSystem: this.path, - }) + this.ProjectLocator.findElement.yields( + null, + this.doc, + { + fileSystem: this.path, + }, + this.parentFolder + ) this.TpdsUpdateSender.addDoc.yields() }) @@ -248,15 +254,14 @@ describe('ProjectEntityUpdateHandler', function () { }) it('should send the doc the to the TPDS', function () { - this.TpdsUpdateSender.addDoc - .calledWith({ - project_id: projectId, - project_name: this.project.name, - doc_id: docId, - rev: this.rev, - path: this.path, - }) - .should.equal(true) + this.TpdsUpdateSender.addDoc.should.have.been.calledWith({ + projectId, + projectName: this.project.name, + docId, + rev: this.rev, + path: this.path, + folderId: this.parentFolder._id, + }) }) it('should call the callback', function () { @@ -645,11 +650,12 @@ describe('ProjectEntityUpdateHandler', function () { it('notifies the tpds', function () { this.TpdsUpdateSender.addFile .calledWith({ - project_id: projectId, - project_name: this.project.name, - file_id: fileId, + projectId, + projectName: this.project.name, + fileId, rev: 0, path: this.path, + folderId, }) .should.equal(true) }) @@ -719,111 +725,6 @@ describe('ProjectEntityUpdateHandler', function () { }) }) - describe('replaceFile', function () { - beforeEach(function () { - // replacement file now creates a new file object - this.newFileUrl = 'new-file-url' - this.newFile = { - _id: newFileId, - name: 'dummy-upload-filename', - rev: 0, - linkedFileData: this.linkedFileData, - } - this.oldFile = { _id: fileId, rev: 3 } - this.path = '/path/to/file' - this.newProject = 'new project' - this.FileStoreHandler.uploadFileFromDisk.yields( - null, - this.newFileUrl, - this.newFile - ) - this.ProjectEntityMongoUpdateHandler._insertDeletedFileReference.yields() - this.ProjectEntityMongoUpdateHandler.replaceFileWithNew.yields( - null, - this.oldFile, - this.project, - { fileSystem: this.path }, - this.newProject - ) - this.ProjectEntityUpdateHandler.replaceFile( - projectId, - fileId, - this.fileSystemPath, - this.linkedFileData, - userId, - this.source, - this.callback - ) - }) - - it('uploads a new version of the file', function () { - this.FileStoreHandler.uploadFileFromDisk - .calledWith( - projectId, - { - name: 'dummy-upload-filename', - linkedFileData: this.linkedFileData, - }, - this.fileSystemPath - ) - .should.equal(true) - }) - - it('replaces the file in mongo', function () { - this.ProjectEntityMongoUpdateHandler.replaceFileWithNew - .calledWith(projectId, fileId, this.newFile) - .should.equal(true) - }) - - it('notifies the tpds', function () { - this.TpdsUpdateSender.addFile - .calledWith({ - project_id: projectId, - project_name: this.project.name, - file_id: newFileId, - rev: this.oldFile.rev + 1, - path: this.path, - }) - .should.equal(true) - }) - - it('should mark the project as updated', function () { - const args = this.ProjectUpdater.markAsUpdated.args[0] - args[0].should.equal(projectId) - args[1].should.exist - args[2].should.equal(userId) - }) - - it('updates the project structure in the doc updater', function () { - const oldFiles = [ - { - file: this.oldFile, - path: this.path, - }, - ] - const newFiles = [ - { - file: this.newFile, - path: this.path, - url: this.newFileUrl, - }, - ] - this.DocumentUpdaterHandler.updateProjectStructure - .calledWith( - projectId, - projectHistoryId, - userId, - { - oldFiles, - newFiles, - newProject: this.newProject, - }, - this.source - ) - .should.equal(true) - }) - }) - describe('upsertDoc', function () { describe('upserting into an invalid folder', function () { beforeEach(function () { @@ -1035,11 +936,12 @@ describe('ProjectEntityUpdateHandler', function () { it('sends the doc to TPDS', function () { expect(this.TpdsUpdateSender.addDoc).to.have.been.calledWith({ - project_id: projectId, - doc_id: this.newDoc._id, + projectId, + docId: this.newDoc._id, path: this.filePath, - project_name: this.newProject.name, + projectName: this.newProject.name, rev: this.existingFile.rev + 1, + folderId, }) }) @@ -1090,7 +992,7 @@ describe('ProjectEntityUpdateHandler', function () { this.FileStoreHandler.uploadFileFromDisk.yields( null, this.fileUrl, - this.newFile + this.file ) }) @@ -1117,13 +1019,17 @@ describe('ProjectEntityUpdateHandler', function () { describe('updating an existing file', function () { beforeEach(function () { - this.existingFile = { _id: fileId, name: this.fileName } + this.existingFile = { _id: fileId, name: this.fileName, rev: 1 } this.folder = { _id: folderId, fileRefs: [this.existingFile], docs: [] } this.ProjectLocator.findElement.yields(null, this.folder) - this.ProjectEntityUpdateHandler.replaceFile = { - mainTask: sinon.stub().yields(null, this.newFile), - } - + this.newProject = 'new-project-stub' + this.ProjectEntityMongoUpdateHandler.replaceFileWithNew.yields( + null, + this.existingFile, + this.project, + { fileSystem: this.fileSystemPath }, + this.newProject + ) this.ProjectEntityUpdateHandler.upsertFile( projectId, folderId, @@ -1136,17 +1042,66 @@ describe('ProjectEntityUpdateHandler', function () { ) }) - it('replaces the file', function () { - expect( - this.ProjectEntityUpdateHandler.replaceFile.mainTask - ).to.be.calledWith( + it('uploads a new version of the file', function () { + this.FileStoreHandler.uploadFileFromDisk.should.have.been.calledWith( projectId, - fileId, - this.fileSystemPath, - this.linkedFileData, + { + name: this.fileName, + linkedFileData: this.linkedFileData, + }, + this.fileSystemPath + ) + }) + + it('replaces the file in mongo', function () { + this.ProjectEntityMongoUpdateHandler.replaceFileWithNew.should.have.been.calledWith( + projectId, + this.existingFile._id, + this.file + ) + }) + + it('notifies the tpds', function () { + this.TpdsUpdateSender.addFile.should.have.been.calledWith({ + projectId, + projectName: this.project.name, + fileId: this.file._id, + rev: this.existingFile.rev + 1, + path: this.fileSystemPath, + folderId, + }) + }) + + it('should mark the project as updated', function () { + const args = this.ProjectUpdater.markAsUpdated.args[0] + args[0].should.equal(projectId) + args[1].should.exist + args[2].should.equal(userId) + }) + + it('updates the project structure in the doc updater', function () { + const oldFiles = [ + { + file: this.existingFile, + path: this.fileSystemPath, + }, + ] + const newFiles = [ + { + file: this.file, + path: this.fileSystemPath, + url: this.fileUrl, + }, + ] + this.DocumentUpdaterHandler.updateProjectStructure.should.have.been.calledWith( + projectId, + projectHistoryId, userId, - this.newFile, - this.fileUrl, + { + oldFiles, + newFiles, + newProject: this.newProject, + }, this.source ) }) @@ -1262,7 +1217,12 @@ describe('ProjectEntityUpdateHandler', function () { element_id: this.existingDoc._id, type: 'doc', }) - .yields(null, this.existingDoc, { fileSystem: this.path }) + .yields( + null, + this.existingDoc, + { fileSystem: this.path }, + this.folder + ) this.newFileUrl = 'new-file-url' this.newFile = { @@ -1642,9 +1602,11 @@ describe('ProjectEntityUpdateHandler', function () { it('it notifies the tpds', function () { this.TpdsUpdateSender.deleteEntity .calledWith({ - project_id: projectId, + projectId, path: this.path, - project_name: this.projectBeforeDeletion.name, + projectName: this.projectBeforeDeletion.name, + entityId: docId, + entityType: 'doc', }) .should.equal(true) }) @@ -1826,11 +1788,14 @@ describe('ProjectEntityUpdateHandler', function () { it('notifies tpds', function () { this.TpdsUpdateSender.promises.moveEntity .calledWith({ - project_id: projectId, - project_name: this.project_name, + projectId, + projectName: this.project_name, startPath: this.startPath, endPath: this.endPath, rev: this.rev, + entityId: docId, + entityType: 'doc', + folderId, }) .should.equal(true) }) @@ -1887,11 +1852,14 @@ describe('ProjectEntityUpdateHandler', function () { it('notifies tpds', function () { this.TpdsUpdateSender.promises.moveEntity .calledWith({ - project_id: projectId, - project_name: this.project_name, + projectId, + projectName: this.project_name, startPath: this.startPath, endPath: this.endPath, rev: this.rev, + entityId: docId, + entityType: 'doc', + folderId: null, }) .should.equal(true) }) @@ -2637,7 +2605,7 @@ describe('ProjectEntityUpdateHandler', function () { element_id: this.doc._id, type: 'doc', }) - .yields(null, this.doc, { fileSystem: this.path }) + .yields(null, this.doc, { fileSystem: this.path }, this.folder) this.ProjectLocator.findElement .withArgs({ project_id: this.project._id.toString(), diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js index 1d00d14cac..6f01521b40 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js @@ -10,13 +10,24 @@ const MODULE_PATH = describe('TpdsProjectFlusher', function () { beforeEach(function () { this.project = { _id: ObjectId() } + this.folder = { _id: ObjectId() } this.docs = { - '/doc/one': { _id: 'mock-doc-1', lines: ['one'], rev: 5 }, - '/doc/two': { _id: 'mock-doc-2', lines: ['two'], rev: 6 }, + '/doc/one': { + _id: 'mock-doc-1', + lines: ['one'], + rev: 5, + folder: this.folder, + }, + '/doc/two': { + _id: 'mock-doc-2', + lines: ['two'], + rev: 6, + folder: this.folder, + }, } this.files = { - '/file/one': { _id: 'mock-file-1', rev: 7 }, - '/file/two': { _id: 'mock-file-2', rev: 8 }, + '/file/one': { _id: 'mock-file-1', rev: 7, folder: this.folder }, + '/file/two': { _id: 'mock-file-2', rev: 8, folder: this.folder }, } this.DocumentUpdaterHandler = { promises: { @@ -79,11 +90,12 @@ describe('TpdsProjectFlusher', function () { for (const [path, doc] of Object.entries(this.docs)) { expect(this.TpdsUpdateSender.promises.addDoc).to.have.been.calledWith( { - project_id: this.project._id, - doc_id: doc._id, - project_name: this.project.name, + projectId: this.project._id, + docId: doc._id, + projectName: this.project.name, rev: doc.rev, path, + folderId: this.folder._id, } ) } @@ -94,11 +106,12 @@ describe('TpdsProjectFlusher', function () { expect( this.TpdsUpdateSender.promises.addFile ).to.have.been.calledWith({ - project_id: this.project._id, - file_id: file._id, - project_name: this.project.name, + projectId: this.project._id, + fileId: file._id, + projectName: this.project.name, rev: file.rev, path, + folderId: this.folder._id, }) } }) @@ -206,11 +219,12 @@ describe('TpdsProjectFlusher', function () { expect( this.TpdsUpdateSender.promises.addDoc ).to.have.been.calledWith({ - project_id: this.project._id, - doc_id: doc._id, - project_name: this.project.name, + projectId: this.project._id, + docId: doc._id, + projectName: this.project.name, rev: doc.rev, path, + folderId: this.folder._id, }) } }) @@ -220,11 +234,12 @@ describe('TpdsProjectFlusher', function () { expect( this.TpdsUpdateSender.promises.addFile ).to.have.been.calledWith({ - project_id: this.project._id, - file_id: file._id, - project_name: this.project.name, + projectId: this.project._id, + fileId: file._id, + projectName: this.project.name, rev: file.rev, path, + folderId: this.folder._id, }) } }) diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js index 32159cf80b..37f42549e3 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js @@ -53,7 +53,7 @@ describe('TpdsUpdateSender', function () { this.UserGetter = { promises: { getUsers }, } - this.updateSender = SandboxedModule.require(modulePath, { + this.TpdsUpdateSender = SandboxedModule.require(modulePath, { requires: { mongodb: { ObjectId }, '@overleaf/settings': this.settings, @@ -69,7 +69,7 @@ describe('TpdsUpdateSender', function () { describe('enqueue', function () { it('should not call request if there is no tpdsworker url', async function () { - await this.updateSender.promises.enqueue(null, null, null) + await this.TpdsUpdateSender.promises.enqueue(null, null, null) this.request.should.not.have.been.called }) @@ -78,7 +78,7 @@ describe('TpdsUpdateSender', function () { const group0 = 'myproject' const method0 = 'somemethod0' const job0 = 'do something' - await this.updateSender.promises.enqueue(group0, method0, job0) + await this.TpdsUpdateSender.promises.enqueue(group0, method0, job0) const args = this.request.firstCall.args[0] args.json.group.should.equal(group0) args.json.job.should.equal(job0) @@ -98,11 +98,11 @@ describe('TpdsUpdateSender', function () { const fileId = '4545345' const path = '/some/path/here.jpg' - await this.updateSender.promises.addFile({ - project_id: projectId, - file_id: fileId, + await this.TpdsUpdateSender.promises.addFile({ + projectId, + fileId, path, - project_name: projectName, + projectName, }) const { @@ -151,12 +151,12 @@ describe('TpdsUpdateSender', function () { const path = '/some/path/here.tex' const lines = ['line1', 'line2', 'line3'] - await this.updateSender.promises.addDoc({ - project_id: projectId, - doc_id: docId, + await this.TpdsUpdateSender.promises.addDoc({ + projectId, + docId, path, docLines: lines, - project_name: projectName, + projectName, }) const { @@ -201,10 +201,10 @@ describe('TpdsUpdateSender', function () { it('deleting entity', async function () { const path = '/path/here/t.tex' - await this.updateSender.promises.deleteEntity({ - project_id: projectId, + await this.TpdsUpdateSender.promises.deleteEntity({ + projectId, path, - project_name: projectName, + projectName, }) const { @@ -247,11 +247,11 @@ describe('TpdsUpdateSender', function () { const startPath = 'staring/here/file.tex' const endPath = 'ending/here/file.tex' - await this.updateSender.promises.moveEntity({ - project_id: projectId, + await this.TpdsUpdateSender.promises.moveEntity({ + projectId, startPath, endPath, - project_name: projectName, + projectName, }) const { @@ -295,9 +295,9 @@ describe('TpdsUpdateSender', function () { const oldProjectName = '/oldProjectName/' const newProjectName = '/newProjectName/' - await this.updateSender.promises.moveEntity({ - project_id: projectId, - project_name: oldProjectName, + await this.TpdsUpdateSender.promises.moveEntity({ + projectId, + projectName: oldProjectName, newProjectName, }) @@ -339,7 +339,7 @@ describe('TpdsUpdateSender', function () { }) it('pollDropboxForUser', async function () { - await this.updateSender.promises.pollDropboxForUser(userId) + await this.TpdsUpdateSender.promises.pollDropboxForUser(userId) const { group: group0, @@ -357,12 +357,12 @@ describe('TpdsUpdateSender', function () { }) describe('deleteProject', function () { it('should not call request if there is no project archiver url', async function () { - await this.updateSender.promises.deleteProject({ project_id: projectId }) + await this.TpdsUpdateSender.promises.deleteProject({ projectId }) this.request.should.not.have.been.called }) it('should make a delete request to project archiver', async function () { this.settings.apis.project_archiver = { url: projectArchiverUrl } - await this.updateSender.promises.deleteProject({ project_id: projectId }) + await this.TpdsUpdateSender.promises.deleteProject({ projectId }) const { uri, method } = this.request.firstCall.args[0] method.should.equal('delete') uri.should.equal(`${projectArchiverUrl}/project/${projectId}`)