diff --git a/services/web/app/src/Features/Chat/ChatApiHandler.js b/services/web/app/src/Features/Chat/ChatApiHandler.js index 8d3db0e034..408ba43121 100644 --- a/services/web/app/src/Features/Chat/ChatApiHandler.js +++ b/services/web/app/src/Features/Chat/ChatApiHandler.js @@ -15,6 +15,20 @@ let ChatApiHandler const OError = require('@overleaf/o-error') const request = require('request') const settings = require('@overleaf/settings') +const { promisify } = require('util') + +function destroyProject(project_id, callback) { + if (callback == null) { + callback = function () {} + } + return ChatApiHandler._apiRequest( + { + url: `${settings.apis.chat.internal_url}/project/${project_id}`, + method: 'DELETE', + }, + callback + ) +} module.exports = ChatApiHandler = { _apiRequest(opts, callback) { @@ -165,4 +179,10 @@ module.exports = ChatApiHandler = { callback ) }, + + destroyProject, + + promises: { + destroyProject: promisify(destroyProject), + }, } diff --git a/services/web/app/src/Features/Project/ProjectDeleter.js b/services/web/app/src/Features/Project/ProjectDeleter.js index e1f2c6406b..fd290e1ac2 100644 --- a/services/web/app/src/Features/Project/ProjectDeleter.js +++ b/services/web/app/src/Features/Project/ProjectDeleter.js @@ -17,6 +17,7 @@ const EditorRealTimeController = require('../Editor/EditorRealTimeController') const HistoryManager = require('../History/HistoryManager') const FilestoreHandler = require('../FileStore/FileStoreHandler') const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') +const ChatApiHandler = require('../Chat/ChatApiHandler') const moment = require('moment') const { promiseMapWithLimit } = require('../../util/promises') @@ -386,6 +387,7 @@ async function expireDeletedProject(projectId) { TpdsUpdateSender.promises.deleteProject({ project_id: deletedProject.project._id, }), + ChatApiHandler.promises.destroyProject(deletedProject.project._id), hardDeleteDeletedFiles(deletedProject.project._id), ]) diff --git a/services/web/scripts/delete_orphaned_chat_threads.js b/services/web/scripts/delete_orphaned_chat_threads.js new file mode 100644 index 0000000000..1c30544d3d --- /dev/null +++ b/services/web/scripts/delete_orphaned_chat_threads.js @@ -0,0 +1,191 @@ +const READ_CONCURRENCY_SECONDARY = + parseInt(process.env.READ_CONCURRENCY_SECONDARY, 10) || 1000 +const READ_CONCURRENCY_PRIMARY = + parseInt(process.env.READ_CONCURRENCY_PRIMARY, 10) || 500 +const WRITE_CONCURRENCY = parseInt(process.env.WRITE_CONCURRENCY, 10) || 10 +const BATCH_SIZE = parseInt(process.env.BATCH_SIZE, 10) || 100 +const DRY_RUN = process.env.DRY_RUN !== 'false' +const MAX_CHATS_TO_DESTROY = + parseInt(process.env.MAX_CHATS_TO_DESTROY, 10) || false +// persist fallback in order to keep batchedUpdate in-sync +process.env.BATCH_SIZE = BATCH_SIZE +// raise mongo timeout to 10mins if otherwise unspecified +process.env.MONGO_SOCKET_TIMEOUT = + parseInt(process.env.MONGO_SOCKET_TIMEOUT, 10) || 600000 + +const { ObjectId, ReadPreference } = require('mongodb') +const { db } = require('../app/src/infrastructure/mongodb') +const { promiseMapWithLimit } = require('../app/src/util/promises') +const { batchedUpdate } = require('./helpers/batchedUpdate') +const ChatApiHandler = require('../app/src/Features/Chat/ChatApiHandler') + +console.log({ + DRY_RUN, + WRITE_CONCURRENCY, + BATCH_SIZE, + MAX_CHATS_TO_DESTROY, +}) + +const RESULT = { + DRY_RUN, + projectChatsDestroyed: 0, + continueFrom: null, +} + +async function processBatch(_, rooms) { + if (rooms.length && rooms[0]._id) { + RESULT.continueFrom = rooms[0]._id + } + + // Logic taken from delete_orphaned_docs_online_check.js + // gets projectIds from rooms, + // then checks 'expired' status of project + + const projectIds = Array.from( + new Set(rooms.map(room => room.project_id.toString())) + ).map(ObjectId) + console.log( + `Checking projects (${projectIds.length})`, + JSON.stringify(projectIds) + ) + + const doubleCheckProjectIdsOnPrimary = [] + async function checkProjectOnSecondary(projectId) { + if (await checkProjectExistsOnSecondary(projectId)) { + // Finding a project with secondary confidence is sufficient. + return + } + // At this point, the secondaries deem this project as having orphaned chat. + doubleCheckProjectIdsOnPrimary.push(projectId) + } + + const projectsWithOrphanedChat = [] + async function checkProjectOnPrimary(projectId) { + if (await checkProjectExistsOnPrimary(projectId)) { + // The project is actually live. + return + } + projectsWithOrphanedChat.push(projectId) + } + + await promiseMapWithLimit( + READ_CONCURRENCY_SECONDARY, + projectIds, + checkProjectOnSecondary + ) + await promiseMapWithLimit( + READ_CONCURRENCY_PRIMARY, + doubleCheckProjectIdsOnPrimary, + checkProjectOnPrimary + ) + + console.log( + `Destroying chat for projects (${projectsWithOrphanedChat.length})`, + JSON.stringify(projectsWithOrphanedChat) + ) + if (!DRY_RUN) { + await promiseMapWithLimit( + WRITE_CONCURRENCY, + projectsWithOrphanedChat, + ChatApiHandler.promises.destroyProject + ) + } + RESULT.projectChatsDestroyed += projectsWithOrphanedChat.length + + console.log(RESULT) + if ( + MAX_CHATS_TO_DESTROY && + RESULT.projectChatsDestroyed >= MAX_CHATS_TO_DESTROY + ) { + console.log( + `MAX_CHATS_TO_DELETE limit (${MAX_CHATS_TO_DESTROY}) reached. Stopping.` + ) + process.exit(0) + } +} + +async function getDeletedProject(projectId, readPreference) { + return await db.deletedProjects.findOne( + { 'deleterData.deletedProjectId': projectId }, + { + // There is no index on .project. Pull down something small. + projection: { 'project._id': 1 }, + readPreference, + } + ) +} + +async function getProject(projectId, readPreference) { + return await db.projects.findOne( + { _id: projectId }, + { + // Pulling down an empty object is fine for differentiating with null. + projection: { _id: 0 }, + readPreference, + } + ) +} + +async function checkProjectExistsWithReadPreference(projectId, readPreference) { + // NOTE: Possible race conditions! + // There are two processes which are racing with our queries: + // 1. project deletion + // 2. project restoring + // For 1. we check the projects collection before deletedProjects. + // If a project were to be delete in this very moment, we should see the + // soft-deleted entry which is created before deleting the projects entry. + // For 2. we check the projects collection after deletedProjects again. + // If a project were to be restored in this very moment, it is very likely + // to see the projects entry again. + // Unlikely edge case: Restore+Deletion in rapid succession. + // We could add locking to the ProjectDeleter for ruling ^ out. + if (await getProject(projectId, readPreference)) { + // The project is live. + return true + } + const deletedProject = await getDeletedProject(projectId, readPreference) + if (deletedProject && deletedProject.project) { + // The project is registered for hard-deletion. + return true + } + if (await getProject(projectId, readPreference)) { + // The project was just restored. + return true + } + // The project does not exist. + return false +} + +async function checkProjectExistsOnPrimary(projectId) { + return await checkProjectExistsWithReadPreference( + projectId, + ReadPreference.PRIMARY + ) +} + +async function checkProjectExistsOnSecondary(projectId) { + return await checkProjectExistsWithReadPreference( + projectId, + ReadPreference.SECONDARY + ) +} + +async function main() { + const projection = { + _id: 1, + project_id: 1, + } + await batchedUpdate('rooms', {}, processBatch, projection) + console.log('Final') + console.log(RESULT) +} + +main() + .then(() => { + console.log('Done.') + process.exit(0) + }) + .catch(error => { + console.error({ error }) + process.exit(1) + }) diff --git a/services/web/test/acceptance/src/DeletionTests.js b/services/web/test/acceptance/src/DeletionTests.js index 268cf0a368..6e09a706c4 100644 --- a/services/web/test/acceptance/src/DeletionTests.js +++ b/services/web/test/acceptance/src/DeletionTests.js @@ -6,12 +6,14 @@ const settings = require('@overleaf/settings') const { db, ObjectId } = require('../../../app/src/infrastructure/mongodb') const MockDocstoreApiClass = require('./mocks/MockDocstoreApi') const MockFilestoreApiClass = require('./mocks/MockFilestoreApi') +const MockChatApiClass = require('./mocks/MockChatApi') -let MockDocstoreApi, MockFilestoreApi +let MockDocstoreApi, MockFilestoreApi, MockChatApi before(function () { MockDocstoreApi = MockDocstoreApiClass.instance() MockFilestoreApi = MockFilestoreApiClass.instance() + MockChatApi = MockChatApiClass.instance() }) describe('Deleting a user', function () { @@ -310,6 +312,7 @@ describe('Deleting a project', function () { MockFilestoreApi.files[this.projectId.toString()] = { dummyFile: 'wombat', } + MockChatApi.projects[this.projectId.toString()] = ['message'] }) }) @@ -370,6 +373,28 @@ describe('Deleting a project', function () { ) }) + it('Should destroy the chat', function (done) { + expect(MockChatApi.projects[this.projectId.toString()]).to.exist + + request.post( + `/internal/project/${this.projectId}/expire-deleted-project`, + { + auth: { + user: settings.apis.web.user, + pass: settings.apis.web.pass, + sendImmediately: true, + }, + }, + (error, res) => { + expect(error).not.to.exist + expect(res.statusCode).to.equal(200) + + expect(MockChatApi.projects[this.projectId.toString()]).not.to.exist + done() + } + ) + }) + it('Should remove the project data from mongo', function (done) { db.deletedProjects.findOne( { 'deleterData.deletedProjectId': ObjectId(this.projectId) }, diff --git a/services/web/test/acceptance/src/mocks/MockChatApi.js b/services/web/test/acceptance/src/mocks/MockChatApi.js index 522af5e56e..b6abf82ddc 100644 --- a/services/web/test/acceptance/src/mocks/MockChatApi.js +++ b/services/web/test/acceptance/src/mocks/MockChatApi.js @@ -22,6 +22,12 @@ class MockChatApi extends AbstractMockApi { res.json(Object.assign({ room_id: projectId }, message)) } + destroyProject(req, res) { + const projectId = req.params.project_id + delete this.projects[projectId] + res.sendStatus(204) + } + applyRoutes() { this.app.get('/project/:project_id/messages', (req, res) => this.getGlobalMessages(req, res) @@ -29,6 +35,9 @@ class MockChatApi extends AbstractMockApi { this.app.post('/project/:project_id/messages', (req, res) => this.sendGlobalMessage(req, res) ) + this.app.delete('/project/:project_id', (req, res) => + this.destroyProject(req, res) + ) } } diff --git a/services/web/test/unit/src/Project/ProjectDeleterTests.js b/services/web/test/unit/src/Project/ProjectDeleterTests.js index 2ba5361eb4..4d0b05305d 100644 --- a/services/web/test/unit/src/Project/ProjectDeleterTests.js +++ b/services/web/test/unit/src/Project/ProjectDeleterTests.js @@ -135,6 +135,11 @@ describe('ProjectDeleter', function () { this.Features = { hasFeature: sinon.stub().returns(true), } + this.ChatApiHandler = { + promises: { + destroyProject: sinon.stub().resolves(), + }, + } this.ProjectDeleter = SandboxedModule.require(modulePath, { requires: { @@ -148,6 +153,7 @@ describe('ProjectDeleter', function () { '../Tags/TagsHandler': this.TagsHandler, '../FileStore/FileStoreHandler': this.FileStoreHandler, '../ThirdPartyDataStore/TpdsUpdateSender': this.TpdsUpdateSender, + '../Chat/ChatApiHandler': this.ChatApiHandler, '../Collaborators/CollaboratorsHandler': this.CollaboratorsHandler, '../Collaborators/CollaboratorsGetter': this.CollaboratorsGetter, '../Docstore/DocstoreManager': this.DocstoreManager, @@ -488,6 +494,12 @@ describe('ProjectDeleter', function () { project_id: this.deletedProjects[0].project._id, }) }) + + it('should destroy the chat threads and messages', function () { + expect( + this.ChatApiHandler.promises.destroyProject + ).to.have.been.calledWith(this.deletedProjects[0].project._id) + }) }) describe('when history-v1 is not available', function () { @@ -577,6 +589,11 @@ describe('ProjectDeleter', function () { expect(this.TpdsUpdateSender.promises.deleteProject).to.not.have.been .called }) + + it('should not destroy the chat threads and messages', function () { + expect(this.ChatApiHandler.promises.destroyProject).to.not.have.been + .called + }) }) })