Sfoglia il codice sorgente

Merge pull request #2369 from overleaf/em-imports-tpds

 Defer flushing to TPDS on project import from v1

GitOrigin-RevId: f2782326716999c37565b3e527b54444bbc53711
Eric Mc Sween 6 anni fa
parent
commit
3da8413156

+ 8 - 12
services/web/app/src/Features/Collaborators/CollaboratorsHandler.js

@@ -5,7 +5,7 @@ const ProjectGetter = require('../Project/ProjectGetter')
 const logger = require('logger-sharelatex')
 const ContactManager = require('../Contacts/ContactManager')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
+const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher')
 const CollaboratorsGetter = require('./CollaboratorsGetter')
 const Errors = require('../Errors/Errors')
 
@@ -98,14 +98,12 @@ async function addUserIdToProject(
   await Project.update({ _id: projectId }, { $addToSet: level }).exec()
 
   // Flush to TPDS in background to add files to collaborator's Dropbox
-  ProjectEntityHandler.promises
-    .flushProjectToThirdPartyDataStore(projectId)
-    .catch(err => {
-      logger.error(
-        { err, projectId, userId },
-        'error flushing to TPDS after adding collaborator'
-      )
-    })
+  TpdsProjectFlusher.promises.flushProjectToTpds(projectId).catch(err => {
+    logger.error(
+      { err, projectId, userId },
+      'error flushing to TPDS after adding collaborator'
+    )
+  })
 }
 
 async function transferProjects(fromUserId, toUserId) {
@@ -221,8 +219,6 @@ async function userIsTokenMember(userId, projectId) {
 
 async function _flushProjects(projectIds) {
   for (const projectId of projectIds) {
-    await ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore(
-      projectId
-    )
+    await TpdsProjectFlusher.promises.flushProjectToTpds(projectId)
   }
 }

+ 2 - 4
services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js

@@ -6,7 +6,7 @@ const CollaboratorsHandler = require('./CollaboratorsHandler')
 const EmailHandler = require('../Email/EmailHandler')
 const Errors = require('../Errors/Errors')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
+const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher')
 const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler')
 
 module.exports = {
@@ -46,9 +46,7 @@ async function transferOwnership(projectId, newOwnerId, options = {}) {
   await _transferOwnership(projectId, previousOwnerId, newOwnerId)
 
   // Flush project to TPDS
-  await ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore(
-    projectId
-  )
+  await TpdsProjectFlusher.promises.flushProjectToTpds(projectId)
 
   // Send confirmation emails
   const previousOwner = await UserGetter.promises.getUser(previousOwnerId)

+ 4 - 0
services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js

@@ -23,6 +23,7 @@ const async = require('async')
 const logger = require('logger-sharelatex')
 const metrics = require('metrics-sharelatex')
 const { Project } = require('../../models/Project')
+const { promisifyAll } = require('../../util/promises')
 
 module.exports = DocumentUpdaterHandler = {
   flushProjectToMongo(project_id, callback) {
@@ -442,6 +443,9 @@ module.exports = DocumentUpdaterHandler = {
     return updates
   }
 }
+module.exports.promises = promisifyAll(DocumentUpdaterHandler, {
+  without: ['_getUpdates']
+})
 
 const PENDINGUPDATESKEY = 'PendingUpdates'
 const DOCLINESKEY = 'doclines'

+ 5 - 1
services/web/app/src/Features/Project/ProjectController.js

@@ -27,6 +27,7 @@ const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler')
 const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const Modules = require('../../infrastructure/Modules')
 const ProjectEntityHandler = require('./ProjectEntityHandler')
+const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher')
 const UserGetter = require('../User/UserGetter')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 const { V1ConnectionError } = require('../Errors/Errors')
@@ -720,7 +721,10 @@ const ProjectController = {
               (error, brandVariationDetails) => cb(error, brandVariationDetails)
             )
           }
-        ]
+        ],
+        flushToTpds: cb => {
+          TpdsProjectFlusher.flushProjectToTpdsIfNeeded(projectId, cb)
+        }
       },
       (err, results) => {
         if (err != null) {

+ 49 - 176
services/web/app/src/Features/Project/ProjectEntityHandler.js

@@ -1,55 +1,29 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS205: Consider reworking code to avoid use of IIFEs
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const _ = require('underscore')
-const async = require('async')
 const path = require('path')
 const logger = require('logger-sharelatex')
 const DocstoreManager = require('../Docstore/DocstoreManager')
-const DocumentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler')
 const Errors = require('../Errors/Errors')
-const { Project } = require('../../models/Project')
 const ProjectGetter = require('./ProjectGetter')
-const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender')
 const { promisifyAll } = require('../../util/promises')
 
 const ProjectEntityHandler = {
-  getAllDocs(project_id, callback) {
-    logger.log({ project_id }, 'getting all docs for project')
+  getAllDocs(projectId, callback) {
+    logger.log({ projectId }, 'getting all docs for project')
 
     // We get the path and name info from the project, and the lines and
     // version info from the doc store.
-    return DocstoreManager.getAllDocs(project_id, function(
-      error,
-      docContentsArray
-    ) {
+    DocstoreManager.getAllDocs(projectId, (error, docContentsArray) => {
       if (error != null) {
         return callback(error)
       }
 
       // Turn array from docstore into a dictionary based on doc id
       const docContents = {}
-      for (let docContent of Array.from(docContentsArray)) {
+      for (let docContent of docContentsArray) {
         docContents[docContent._id] = docContent
       }
 
-      return ProjectEntityHandler._getAllFolders(project_id, function(
-        error,
-        folders
-      ) {
+      ProjectEntityHandler._getAllFolders(projectId, (error, folders) => {
         if (folders == null) {
           folders = {}
         }
@@ -59,7 +33,7 @@ const ProjectEntityHandler = {
         const docs = {}
         for (let folderPath in folders) {
           const folder = folders[folderPath]
-          for (let doc of Array.from(folder.docs || [])) {
+          for (let doc of folder.docs || []) {
             const content = docContents[doc._id.toString()]
             if (content != null) {
               docs[path.join(folderPath, doc.name)] = {
@@ -72,20 +46,17 @@ const ProjectEntityHandler = {
           }
         }
         logger.log(
-          { count: _.keys(docs).length, project_id },
+          { count: _.keys(docs).length, projectId },
           'returning docs for project'
         )
-        return callback(null, docs)
+        callback(null, docs)
       })
     })
   },
 
-  getAllFiles(project_id, callback) {
-    logger.log({ project_id }, 'getting all files for project')
-    return ProjectEntityHandler._getAllFolders(project_id, function(
-      err,
-      folders
-    ) {
+  getAllFiles(projectId, callback) {
+    logger.log({ projectId }, 'getting all files for project')
+    ProjectEntityHandler._getAllFolders(projectId, (err, folders) => {
       if (folders == null) {
         folders = {}
       }
@@ -95,18 +66,18 @@ const ProjectEntityHandler = {
       const files = {}
       for (let folderPath in folders) {
         const folder = folders[folderPath]
-        for (let file of Array.from(folder.fileRefs || [])) {
+        for (let file of folder.fileRefs || []) {
           if (file != null) {
             files[path.join(folderPath, file.name)] = file
           }
         }
       }
-      return callback(null, files)
+      callback(null, files)
     })
   },
 
-  getAllEntities(project_id, callback) {
-    return ProjectGetter.getProject(project_id, function(err, project) {
+  getAllEntities(projectId, callback) {
+    ProjectGetter.getProject(projectId, (err, project) => {
       if (err != null) {
         return callback(err)
       }
@@ -114,16 +85,13 @@ const ProjectEntityHandler = {
         return callback(new Errors.NotFoundError('project not found'))
       }
 
-      return ProjectEntityHandler.getAllEntitiesFromProject(project, callback)
+      ProjectEntityHandler.getAllEntitiesFromProject(project, callback)
     })
   },
 
   getAllEntitiesFromProject(project, callback) {
     logger.log({ project }, 'getting all entities for project')
-    return ProjectEntityHandler._getAllFoldersFromProject(project, function(
-      err,
-      folders
-    ) {
+    ProjectEntityHandler._getAllFoldersFromProject(project, (err, folders) => {
       if (folders == null) {
         folders = {}
       }
@@ -134,42 +102,36 @@ const ProjectEntityHandler = {
       const files = []
       for (let folderPath in folders) {
         const folder = folders[folderPath]
-        for (let doc of Array.from(folder.docs || [])) {
+        for (let doc of folder.docs || []) {
           if (doc != null) {
             docs.push({ path: path.join(folderPath, doc.name), doc })
           }
         }
-        for (let file of Array.from(folder.fileRefs || [])) {
+        for (let file of folder.fileRefs || []) {
           if (file != null) {
             files.push({ path: path.join(folderPath, file.name), file })
           }
         }
       }
-      return callback(null, docs, files)
+      callback(null, docs, files)
     })
   },
 
-  getAllDocPathsFromProjectById(project_id, callback) {
-    return ProjectGetter.getProjectWithoutDocLines(project_id, function(
-      err,
-      project
-    ) {
+  getAllDocPathsFromProjectById(projectId, callback) {
+    ProjectGetter.getProjectWithoutDocLines(projectId, (err, project) => {
       if (err != null) {
         return callback(err)
       }
       if (project == null) {
         return callback(Errors.NotFoundError('no project'))
       }
-      return ProjectEntityHandler.getAllDocPathsFromProject(project, callback)
+      ProjectEntityHandler.getAllDocPathsFromProject(project, callback)
     })
   },
 
   getAllDocPathsFromProject(project, callback) {
     logger.log({ project }, 'getting all docs for project')
-    return ProjectEntityHandler._getAllFoldersFromProject(project, function(
-      err,
-      folders
-    ) {
+    ProjectEntityHandler._getAllFoldersFromProject(project, (err, folders) => {
       if (folders == null) {
         folders = {}
       }
@@ -179,129 +141,51 @@ const ProjectEntityHandler = {
       const docPath = {}
       for (let folderPath in folders) {
         const folder = folders[folderPath]
-        for (let doc of Array.from(folder.docs || [])) {
+        for (let doc of folder.docs || []) {
           docPath[doc._id] = path.join(folderPath, doc.name)
         }
       }
       logger.log(
-        { count: _.keys(docPath).length, project_id: project._id },
+        { count: _.keys(docPath).length, projectId: project._id },
         'returning docPaths for project'
       )
-      return callback(null, docPath)
+      callback(null, docPath)
     })
   },
 
-  flushProjectToThirdPartyDataStore(project_id, callback) {
-    logger.log({ project_id }, 'flushing project to tpds')
-    return DocumentUpdaterHandler.flushProjectToMongo(project_id, function(
-      error
-    ) {
-      if (error != null) {
-        return callback(error)
-      }
-      return ProjectGetter.getProject(project_id, { name: true }, function(
-        error,
-        project
-      ) {
-        if (error != null) {
-          return callback(error)
-        }
-        const requests = []
-        return ProjectEntityHandler.getAllDocs(project_id, function(
-          error,
-          docs
-        ) {
-          if (error != null) {
-            return callback(error)
-          }
-          for (let docPath in docs) {
-            const doc = docs[docPath]
-            ;((docPath, doc) =>
-              requests.push(cb =>
-                TpdsUpdateSender.addDoc(
-                  {
-                    project_id,
-                    doc_id: doc._id,
-                    path: docPath,
-                    project_name: project.name,
-                    rev: doc.rev || 0
-                  },
-                  cb
-                )
-              ))(docPath, doc)
-          }
-          return ProjectEntityHandler.getAllFiles(project_id, function(
-            error,
-            files
-          ) {
-            if (error != null) {
-              return callback(error)
-            }
-            for (let filePath in files) {
-              const file = files[filePath]
-              ;((filePath, file) =>
-                requests.push(cb =>
-                  TpdsUpdateSender.addFile(
-                    {
-                      project_id,
-                      file_id: file._id,
-                      path: filePath,
-                      project_name: project.name,
-                      rev: file.rev
-                    },
-                    cb
-                  )
-                ))(filePath, file)
-            }
-            return async.series(requests, function(err) {
-              logger.log({ project_id }, 'finished flushing project to tpds')
-              return callback(err)
-            })
-          })
-        })
-      })
-    })
-  },
-
-  getDoc(project_id, doc_id, options, callback) {
+  getDoc(projectId, docId, options, callback) {
     if (options == null) {
       options = {}
     }
-    if (callback == null) {
-      callback = function(error, lines, rev) {}
-    }
     if (typeof options === 'function') {
       callback = options
       options = {}
     }
 
-    return DocstoreManager.getDoc(project_id, doc_id, options, callback)
+    DocstoreManager.getDoc(projectId, docId, options, callback)
   },
 
-  getDocPathByProjectIdAndDocId(project_id, doc_id, callback) {
-    logger.log({ project_id, doc_id }, 'getting path for doc and project')
-    return ProjectGetter.getProjectWithoutDocLines(project_id, function(
-      err,
-      project
-    ) {
+  getDocPathByProjectIdAndDocId(projectId, docId, callback) {
+    logger.log({ projectId, docId }, 'getting path for doc and project')
+    ProjectGetter.getProjectWithoutDocLines(projectId, (err, project) => {
       if (err != null) {
         return callback(err)
       }
       if (project == null) {
         return callback(new Errors.NotFoundError('no project'))
       }
-      function recursivelyFindDocInFolder(basePath, doc_id, folder) {
-        let docInCurrentFolder = Array.from(folder.docs || []).find(
-          currentDoc => currentDoc._id.toString() === doc_id.toString()
+      function recursivelyFindDocInFolder(basePath, docId, folder) {
+        let docInCurrentFolder = (folder.docs || []).find(
+          currentDoc => currentDoc._id.toString() === docId.toString()
         )
         if (docInCurrentFolder != null) {
           return path.join(basePath, docInCurrentFolder.name)
         } else {
           let docPath, childFolder
-          for (childFolder of Array.from(folder.folders || [])) {
+          for (childFolder of folder.folders || []) {
             docPath = recursivelyFindDocInFolder(
               path.join(basePath, childFolder.name),
-              doc_id,
+              docId,
               childFolder
             )
             if (docPath != null) {
@@ -313,53 +197,42 @@ const ProjectEntityHandler = {
       }
       const docPath = recursivelyFindDocInFolder(
         '/',
-        doc_id,
+        docId,
         project.rootFolder[0]
       )
       if (docPath == null) {
         return callback(new Errors.NotFoundError('no doc'))
       }
-      return callback(null, docPath)
+      callback(null, docPath)
     })
   },
 
-  _getAllFolders(project_id, callback) {
-    logger.log({ project_id }, 'getting all folders for project')
-    return ProjectGetter.getProjectWithoutDocLines(project_id, function(
-      err,
-      project
-    ) {
+  _getAllFolders(projectId, callback) {
+    logger.log({ projectId }, 'getting all folders for project')
+    ProjectGetter.getProjectWithoutDocLines(projectId, (err, project) => {
       if (err != null) {
         return callback(err)
       }
       if (project == null) {
         return callback(new Errors.NotFoundError('no project'))
       }
-      return ProjectEntityHandler._getAllFoldersFromProject(project, callback)
+      ProjectEntityHandler._getAllFoldersFromProject(project, callback)
     })
   },
 
   _getAllFoldersFromProject(project, callback) {
     const folders = {}
-    var processFolder = function(basePath, folder) {
+    function processFolder(basePath, folder) {
       folders[basePath] = folder
-      return (() => {
-        const result = []
-        for (let childFolder of Array.from(folder.folders || [])) {
-          if (childFolder.name != null) {
-            result.push(
-              processFolder(path.join(basePath, childFolder.name), childFolder)
-            )
-          } else {
-            result.push(undefined)
-          }
+      for (let childFolder of folder.folders || []) {
+        if (childFolder.name != null) {
+          processFolder(path.join(basePath, childFolder.name), childFolder)
         }
-        return result
-      })()
+      }
     }
 
     processFolder('/', project.rootFolder[0])
-    return callback(null, folders)
+    callback(null, folders)
   }
 }
 

+ 3 - 3
services/web/app/src/Features/ServerAdmin/AdminController.js

@@ -28,6 +28,7 @@ const RecurlyWrapper = require('../Subscription/RecurlyWrapper')
 const SubscriptionHandler = require('../Subscription/SubscriptionHandler')
 const projectEntityHandler = require('../Project/ProjectEntityHandler')
 const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender')
+const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher')
 const EditorRealTimeController = require('../Editor/EditorRealTimeController')
 const SystemMessageManager = require('../SystemMessages/SystemMessageManager')
 
@@ -130,9 +131,8 @@ module.exports = AdminController = {
   },
 
   flushProjectToTpds(req, res) {
-    return projectEntityHandler.flushProjectToThirdPartyDataStore(
-      req.body.project_id,
-      err => res.sendStatus(200)
+    return TpdsProjectFlusher.flushProjectToTpds(req.body.project_id, err =>
+      res.sendStatus(200)
     )
   },
 

+ 99 - 0
services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js

@@ -0,0 +1,99 @@
+const { callbackify } = require('util')
+const logger = require('logger-sharelatex')
+const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
+const ProjectGetter = require('../Project/ProjectGetter')
+const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
+const { Project } = require('../../models/Project')
+const TpdsUpdateSender = require('./TpdsUpdateSender')
+
+module.exports = {
+  flushProjectToTpds: callbackify(flushProjectToTpds),
+  deferProjectFlushToTpds: callbackify(deferProjectFlushToTpds),
+  flushProjectToTpdsIfNeeded: callbackify(flushProjectToTpdsIfNeeded),
+  promises: {
+    flushProjectToTpds,
+    deferProjectFlushToTpds,
+    flushProjectToTpdsIfNeeded
+  }
+}
+
+/**
+ * Flush a complete project to the TPDS.
+ */
+async function flushProjectToTpds(projectId) {
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    name: true,
+    deferredTpdsFlushCounter: true
+  })
+  await _flushProjectToTpds(project)
+}
+
+/**
+ * Flush a project to TPDS if a flush is pending
+ */
+async function flushProjectToTpdsIfNeeded(projectId) {
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    name: true,
+    deferredTpdsFlushCounter: true
+  })
+  if (project.deferredTpdsFlushCounter > 0) {
+    await _flushProjectToTpds(project)
+  }
+}
+
+async function _flushProjectToTpds(project) {
+  logger.debug({ projectId: project._id }, 'flushing project to TPDS')
+  logger.debug({ projectId: project._id }, 'finished flushing project to TPDS')
+  await DocumentUpdaterHandler.promises.flushProjectToMongo(project._id)
+  const [docs, files] = await Promise.all([
+    ProjectEntityHandler.promises.getAllDocs(project._id),
+    ProjectEntityHandler.promises.getAllFiles(project._id)
+  ])
+  for (const [docPath, doc] of Object.entries(docs)) {
+    await TpdsUpdateSender.promises.addDoc({
+      project_id: project._id,
+      doc_id: doc._id,
+      path: docPath,
+      project_name: project.name,
+      rev: doc.rev || 0
+    })
+  }
+  for (const [filePath, file] of Object.entries(files)) {
+    await TpdsUpdateSender.promises.addFile({
+      project_id: project._id,
+      file_id: file._id,
+      path: filePath,
+      project_name: project.name,
+      rev: file.rev
+    })
+  }
+  await _resetDeferredTpdsFlushCounter(project)
+}
+
+/**
+ * Reset the TPDS pending flush counter.
+ *
+ * To avoid concurrency problems, the flush counter is not reset if it has been
+ * incremented since we fetched it from the database.
+ */
+async function _resetDeferredTpdsFlushCounter(project) {
+  if (project.deferredTpdsFlushCounter > 0) {
+    await Project.updateOne(
+      {
+        _id: project._id,
+        deferredTpdsFlushCounter: { $lte: project.deferredTpdsFlushCounter }
+      },
+      { $set: { deferredTpdsFlushCounter: 0 } }
+    ).exec()
+  }
+}
+
+/**
+ * Mark a project as pending a flush to TPDS.
+ */
+async function deferProjectFlushToTpds(projectId) {
+  await Project.updateOne(
+    { _id: projectId },
+    { $inc: { deferredTpdsFlushCounter: 1 } }
+  ).exec()
+}

+ 2 - 1
services/web/app/src/models/Project.js

@@ -114,7 +114,8 @@ const ProjectSchema = new Schema({
       }
     }
   ],
-  auditLog: [AuditLogEntrySchema]
+  auditLog: [AuditLogEntrySchema],
+  deferredTpdsFlushCounter: { type: Number }
 })
 
 ProjectSchema.statics.getProject = function(projectOrId, fields, callback) {

+ 3 - 0
services/web/test/acceptance/bootstrap.js

@@ -2,6 +2,9 @@ const chai = require('chai')
 chai.use(require('chai-as-promised'))
 chai.use(require('chaid'))
 
+// Do not truncate assertion errors
+chai.config.truncateThreshold = 0
+
 // Crash the process on an unhandled promise rejection
 process.on('unhandledRejection', err => {
   console.error('Unhandled promise rejection:', err)

+ 7 - 7
services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js

@@ -36,9 +36,9 @@ describe('CollaboratorsHandler', function() {
       addContact: sinon.stub()
     }
     this.ProjectMock = sinon.mock(Project)
-    this.ProjectEntityHandler = {
+    this.TpdsProjectFlusher = {
       promises: {
-        flushProjectToThirdPartyDataStore: sinon.stub().resolves()
+        flushProjectToTpds: sinon.stub().resolves()
       }
     }
     this.ProjectGetter = {
@@ -60,7 +60,7 @@ describe('CollaboratorsHandler', function() {
         '../User/UserGetter': this.UserGetter,
         '../Contacts/ContactManager': this.ContactManager,
         '../../models/Project': { Project },
-        '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
+        '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
         '../Project/ProjectGetter': this.ProjectGetter,
         '../Errors/Errors': Errors,
         './CollaboratorsGetter': this.CollaboratorsGetter
@@ -123,7 +123,7 @@ describe('CollaboratorsHandler', function() {
 
       it('should flush the project to the TPDS', function() {
         expect(
-          this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
+          this.TpdsProjectFlusher.promises.flushProjectToTpds
         ).to.have.been.calledWith(this.project._id)
       })
 
@@ -158,7 +158,7 @@ describe('CollaboratorsHandler', function() {
 
       it('should flush the project to the TPDS', function() {
         expect(
-          this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
+          this.TpdsProjectFlusher.promises.flushProjectToTpds
         ).to.have.been.calledWith(this.project._id)
       })
     })
@@ -326,7 +326,7 @@ describe('CollaboratorsHandler', function() {
         await sleep(100) // let the background tasks run
         for (const project of this.projects) {
           expect(
-            this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
+            this.TpdsProjectFlusher.promises.flushProjectToTpds
           ).to.have.been.calledWith(project._id)
         }
       })
@@ -334,7 +334,7 @@ describe('CollaboratorsHandler', function() {
 
     describe('when flushing to TPDS fails', function() {
       it('should log an error but not fail', async function() {
-        this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore.rejects(
+        this.TpdsProjectFlusher.promises.flushProjectToTpds.rejects(
           new Error('oops')
         )
         await this.CollaboratorsHandler.promises.transferProjects(

+ 4 - 4
services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js

@@ -38,9 +38,9 @@ describe('OwnershipTransferHandler', function() {
         moveEntity: sinon.stub().resolves()
       }
     }
-    this.ProjectEntityHandler = {
+    this.TpdsProjectFlusher = {
       promises: {
-        flushProjectToThirdPartyDataStore: sinon.stub().resolves()
+        flushProjectToTpds: sinon.stub().resolves()
       }
     }
     this.CollaboratorsHandler = {
@@ -69,7 +69,7 @@ describe('OwnershipTransferHandler', function() {
           Project: this.ProjectModel
         },
         '../User/UserGetter': this.UserGetter,
-        '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
+        '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
         '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler,
         '../Email/EmailHandler': this.EmailHandler,
         './CollaboratorsHandler': this.CollaboratorsHandler,
@@ -174,7 +174,7 @@ describe('OwnershipTransferHandler', function() {
         this.collaborator._id
       )
       expect(
-        this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
+        this.TpdsProjectFlusher.promises.flushProjectToTpds
       ).to.have.been.calledWith(this.project._id)
     })
 

+ 15 - 3
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -117,6 +117,9 @@ describe('ProjectController', function() {
         .stub()
         .callsArgWith(1, null, this.brandVariationDetails)
     }
+    this.TpdsProjectFlusher = {
+      flushProjectToTpdsIfNeeded: sinon.stub().yields()
+    }
     this.getUserAffiliations = sinon.stub().callsArgWith(1, null, [
       {
         email: 'test@overleaf.com',
@@ -154,9 +157,7 @@ describe('ProjectController', function() {
         '../Subscription/LimitationsManager': this.LimitationsManager,
         '../Tags/TagsHandler': this.TagsHandler,
         '../Notifications/NotificationsHandler': this.NotificationsHandler,
-        '../../models/User': {
-          User: this.UserModel
-        },
+        '../../models/User': { User: this.UserModel },
         '../Authorization/AuthorizationManager': this.AuthorizationManager,
         '../InactiveData/InactiveProjectManager': this.InactiveProjectManager,
         './ProjectUpdateHandler': this.ProjectUpdateHandler,
@@ -180,6 +181,7 @@ describe('ProjectController', function() {
         '../Institutions/InstitutionsAPI': {
           getUserAffiliations: this.getUserAffiliations
         },
+        '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
         '../V1/V1Handler': {},
         '../../models/Project': {}
       }
@@ -1105,6 +1107,16 @@ describe('ProjectController', function() {
       }
       this.ProjectController.loadEditor(this.req, this.res)
     })
+
+    it('flushes the project to TPDS if a flush is pending', function(done) {
+      this.res.render = () => {
+        this.TpdsProjectFlusher.flushProjectToTpdsIfNeeded.should.have.been.calledWith(
+          this.project_id
+        )
+        done()
+      }
+      this.ProjectController.loadEditor(this.req, this.res)
+    })
   })
 
   describe('userProjectsJson', function() {

+ 0 - 95
services/web/test/unit/src/Project/ProjectEntityHandlerTests.js

@@ -306,101 +306,6 @@ describe('ProjectEntityHandler', function() {
     })
   })
 
-  describe('flushProjectToThirdPartyDataStore', function() {
-    beforeEach(function(done) {
-      this.project = {
-        _id: project_id,
-        name: 'Mock project name'
-      }
-      this.DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().yields()
-      this.docs = {
-        '/doc/one': (this.doc1 = { _id: 'mock-doc-1', lines: ['one'], rev: 5 }),
-        '/doc/two': (this.doc2 = { _id: 'mock-doc-2', lines: ['two'], rev: 6 })
-      }
-      this.files = {
-        '/file/one': (this.file1 = { _id: 'mock-file-1', rev: 7 }),
-        '/file/two': (this.file2 = { _id: 'mock-file-2', rev: 8 })
-      }
-      this.ProjectEntityHandler.getAllDocs = sinon
-        .stub()
-        .yields(null, this.docs)
-      this.ProjectEntityHandler.getAllFiles = sinon
-        .stub()
-        .yields(null, this.files)
-
-      this.ProjectGetter.getProject = sinon.stub().yields(null, this.project)
-
-      this.ProjectEntityHandler.flushProjectToThirdPartyDataStore(
-        project_id,
-        () => done()
-      )
-    })
-
-    it('should flush the project from the doc updater', function() {
-      this.DocumentUpdaterHandler.flushProjectToMongo
-        .calledWith(project_id)
-        .should.equal(true)
-    })
-
-    it('should look up the project in mongo', function() {
-      this.ProjectGetter.getProject.calledWith(project_id).should.equal(true)
-    })
-
-    it('should get all the docs in the project', function() {
-      this.ProjectEntityHandler.getAllDocs
-        .calledWith(project_id)
-        .should.equal(true)
-    })
-
-    it('should get all the files in the project', function() {
-      this.ProjectEntityHandler.getAllFiles
-        .calledWith(project_id)
-        .should.equal(true)
-    })
-
-    it('should flush each doc to the TPDS', function() {
-      return (() => {
-        const result = []
-        for (let path in this.docs) {
-          const doc = this.docs[path]
-          result.push(
-            this.TpdsUpdateSender.addDoc
-              .calledWith({
-                project_id,
-                doc_id: doc._id,
-                project_name: this.project.name,
-                rev: doc.rev,
-                path
-              })
-              .should.equal(true)
-          )
-        }
-        return result
-      })()
-    })
-
-    it('should flush each file to the TPDS', function() {
-      return (() => {
-        const result = []
-        for (let path in this.files) {
-          const file = this.files[path]
-          result.push(
-            this.TpdsUpdateSender.addFile
-              .calledWith({
-                project_id,
-                file_id: file._id,
-                project_name: this.project.name,
-                rev: file.rev,
-                path
-              })
-              .should.equal(true)
-          )
-        }
-        return result
-      })()
-    })
-  })
-
   describe('getDoc', function() {
     beforeEach(function() {
       this.lines = ['mock', 'doc', 'lines']

+ 241 - 0
services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js

@@ -0,0 +1,241 @@
+const { expect } = require('chai')
+const sinon = require('sinon')
+const SandboxedModule = require('sandboxed-module')
+const { ObjectId } = require('mongodb')
+const { Project } = require('../helpers/models/Project')
+
+const MODULE_PATH =
+  '../../../../app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher'
+
+describe('TpdsProjectFlusher', function() {
+  beforeEach(function() {
+    this.project = { _id: ObjectId() }
+    this.docs = {
+      '/doc/one': { _id: 'mock-doc-1', lines: ['one'], rev: 5 },
+      '/doc/two': { _id: 'mock-doc-2', lines: ['two'], rev: 6 }
+    }
+    this.files = {
+      '/file/one': { _id: 'mock-file-1', rev: 7 },
+      '/file/two': { _id: 'mock-file-2', rev: 8 }
+    }
+    this.DocumentUpdaterHandler = {
+      promises: {
+        flushProjectToMongo: sinon.stub().resolves()
+      }
+    }
+    this.ProjectGetter = {
+      promises: {
+        getProject: sinon.stub().resolves(this.project)
+      }
+    }
+    this.ProjectEntityHandler = {
+      promises: {
+        getAllDocs: sinon
+          .stub()
+          .withArgs(this.project._id)
+          .resolves(this.docs),
+        getAllFiles: sinon
+          .stub()
+          .withArgs(this.project._id)
+          .resolves(this.files)
+      }
+    }
+    this.TpdsUpdateSender = {
+      promises: {
+        addDoc: sinon.stub().resolves(),
+        addFile: sinon.stub().resolves()
+      }
+    }
+    this.ProjectMock = sinon.mock(Project)
+
+    this.TpdsProjectFlusher = SandboxedModule.require(MODULE_PATH, {
+      requires: {
+        '../DocumentUpdater/DocumentUpdaterHandler': this
+          .DocumentUpdaterHandler,
+        '../Project/ProjectGetter': this.ProjectGetter,
+        '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
+        '../../models/Project': { Project },
+        './TpdsUpdateSender': this.TpdsUpdateSender
+      }
+    })
+  })
+
+  afterEach(function() {
+    this.ProjectMock.restore()
+  })
+
+  describe('flushProjectToTpds', function() {
+    describe('usually', function() {
+      beforeEach(async function() {
+        await this.TpdsProjectFlusher.promises.flushProjectToTpds(
+          this.project._id
+        )
+      })
+
+      it('should flush the project from the doc updater', function() {
+        expect(
+          this.DocumentUpdaterHandler.promises.flushProjectToMongo
+        ).to.have.been.calledWith(this.project._id)
+      })
+
+      it('should flush each doc to the TPDS', 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,
+              rev: doc.rev,
+              path
+            }
+          )
+        }
+      })
+
+      it('should flush each file to the TPDS', function() {
+        for (const [path, file] of Object.entries(this.files)) {
+          expect(
+            this.TpdsUpdateSender.promises.addFile
+          ).to.have.been.calledWith({
+            project_id: this.project._id,
+            file_id: file._id,
+            project_name: this.project.name,
+            rev: file.rev,
+            path
+          })
+        }
+      })
+    })
+
+    describe('when a TPDS flush is pending', function() {
+      beforeEach(async function() {
+        this.project.deferredTpdsFlushCounter = 2
+        this.ProjectMock.expects('updateOne')
+          .withArgs(
+            {
+              _id: this.project._id,
+              deferredTpdsFlushCounter: { $lte: 2 }
+            },
+            { $set: { deferredTpdsFlushCounter: 0 } }
+          )
+          .chain('exec')
+          .resolves()
+        await this.TpdsProjectFlusher.promises.flushProjectToTpds(
+          this.project._id
+        )
+      })
+
+      it('resets the deferred flush counter', function() {
+        this.ProjectMock.verify()
+      })
+    })
+  })
+
+  describe('deferProjectFlushToTpds', function() {
+    beforeEach(async function() {
+      this.ProjectMock.expects('updateOne')
+        .withArgs(
+          {
+            _id: this.project._id
+          },
+          { $inc: { deferredTpdsFlushCounter: 1 } }
+        )
+        .chain('exec')
+        .resolves()
+      await this.TpdsProjectFlusher.promises.deferProjectFlushToTpds(
+        this.project._id
+      )
+    })
+
+    it('increments the deferred flush counter', function() {
+      this.ProjectMock.verify()
+    })
+  })
+
+  describe('flushProjectToTpdsIfNeeded', function() {
+    let cases = [0, undefined]
+    cases.forEach(counterValue => {
+      describe(`when the deferred flush counter is ${counterValue}`, function() {
+        beforeEach(async function() {
+          this.project.deferredTpdsFlushCounter = counterValue
+          await this.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded(
+            this.project._id
+          )
+        })
+
+        it("doesn't flush the project from the doc updater", function() {
+          expect(this.DocumentUpdaterHandler.promises.flushProjectToMongo).not
+            .to.have.been.called
+        })
+
+        it("doesn't flush any doc", function() {
+          expect(this.TpdsUpdateSender.promises.addDoc).not.to.have.been.called
+        })
+
+        it("doesn't flush any file", function() {
+          expect(this.TpdsUpdateSender.promises.addFile).not.to.have.been.called
+        })
+      })
+    })
+
+    cases = [1, 2]
+    cases.forEach(counterValue => {
+      describe(`when the deferred flush counter is ${counterValue}`, function() {
+        beforeEach(async function() {
+          this.project.deferredTpdsFlushCounter = counterValue
+          this.ProjectMock.expects('updateOne')
+            .withArgs(
+              {
+                _id: this.project._id,
+                deferredTpdsFlushCounter: { $lte: counterValue }
+              },
+              { $set: { deferredTpdsFlushCounter: 0 } }
+            )
+            .chain('exec')
+            .resolves()
+          await this.TpdsProjectFlusher.promises.flushProjectToTpdsIfNeeded(
+            this.project._id
+          )
+        })
+
+        it('flushes the project from the doc updater', function() {
+          expect(
+            this.DocumentUpdaterHandler.promises.flushProjectToMongo
+          ).to.have.been.calledWith(this.project._id)
+        })
+
+        it('flushes each doc to the TPDS', 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,
+              rev: doc.rev,
+              path
+            })
+          }
+        })
+
+        it('flushes each file to the TPDS', function() {
+          for (const [path, file] of Object.entries(this.files)) {
+            expect(
+              this.TpdsUpdateSender.promises.addFile
+            ).to.have.been.calledWith({
+              project_id: this.project._id,
+              file_id: file._id,
+              project_name: this.project.name,
+              rev: file.rev,
+              path
+            })
+          }
+        })
+
+        it('resets the deferred flush counter', function() {
+          this.ProjectMock.verify()
+        })
+      })
+    })
+  })
+})