diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js index 2f8904a271..219b30a946 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js +++ b/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) } } diff --git a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js b/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js index 976e333da4..8221cc428a 100644 --- a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js +++ b/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) diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 6d0ea86019..add0058355 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/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' diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index a3187915a8..defb42189b 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/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) { diff --git a/services/web/app/src/Features/Project/ProjectEntityHandler.js b/services/web/app/src/Features/Project/ProjectEntityHandler.js index aab074432c..0f4e19b66b 100644 --- a/services/web/app/src/Features/Project/ProjectEntityHandler.js +++ b/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) } } diff --git a/services/web/app/src/Features/ServerAdmin/AdminController.js b/services/web/app/src/Features/ServerAdmin/AdminController.js index 7a5f9f377f..a94ffbd8c0 100644 --- a/services/web/app/src/Features/ServerAdmin/AdminController.js +++ b/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) ) }, diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher.js new file mode 100644 index 0000000000..420c562b32 --- /dev/null +++ b/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() +} diff --git a/services/web/app/src/models/Project.js b/services/web/app/src/models/Project.js index 65c10a5c48..ccc4e08337 100644 --- a/services/web/app/src/models/Project.js +++ b/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) { diff --git a/services/web/test/acceptance/bootstrap.js b/services/web/test/acceptance/bootstrap.js index 96d08c50bd..badb8ecf83 100644 --- a/services/web/test/acceptance/bootstrap.js +++ b/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) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js index 245036c50f..9839416ebe 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js +++ b/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( diff --git a/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js b/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js index 538e1bd612..5cdb540de2 100644 --- a/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js +++ b/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) }) diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index 8934c00850..ebd42baf59 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/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() { diff --git a/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js index d18cad273f..bc44037798 100644 --- a/services/web/test/unit/src/Project/ProjectEntityHandlerTests.js +++ b/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'] diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js new file mode 100644 index 0000000000..c5218574f9 --- /dev/null +++ b/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() + }) + }) + }) + }) +})