Przeglądaj źródła

Merge pull request #9409 from overleaf/em-send-more-metadata-to-tpds

Send more metadata to TPDS

GitOrigin-RevId: e6a76a9974fbd8a7fc6a6b861c88ff4728301bc3
Eric Mc Sween 4 lat temu
rodzic
commit
e3303bb276

+ 1 - 1
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),

+ 2 - 2
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,
   })
 }

+ 2 - 1
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 }
           }
         }
       }

+ 101 - 124
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')

+ 8 - 6
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)

+ 87 - 49
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,

+ 1 - 1
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

+ 1 - 1
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,
         })
       })
 

+ 2 - 2
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,
         }
       )

+ 20 - 22
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 },
+        })
       })
     })
 

+ 114 - 146
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(),

+ 31 - 16
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,
             })
           }
         })

+ 23 - 23
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}`)