From 8915e1d026c79db979144276019da90e08a10e08 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 18 Feb 2021 10:10:14 +0000 Subject: [PATCH] [misc] add a new endpoint for getting deleted docs --- services/docstore/app.js | 1 + services/docstore/app/js/DocManager.js | 4 + services/docstore/app/js/HttpController.js | 18 +++ services/docstore/app/js/MongoManager.js | 19 +++ services/docstore/config/settings.defaults.js | 2 + .../test/acceptance/js/DeletingDocsTests.js | 132 ++++++++++++++++++ .../acceptance/js/helpers/DocstoreClient.js | 16 +++ .../test/unit/js/MongoManagerTests.js | 47 ++++++- 8 files changed, 238 insertions(+), 1 deletion(-) diff --git a/services/docstore/app.js b/services/docstore/app.js index 790a916f33..08260dfbba 100644 --- a/services/docstore/app.js +++ b/services/docstore/app.js @@ -48,6 +48,7 @@ app.param('doc_id', function (req, res, next, docId) { Metrics.injectMetricsRoute(app) +app.get('/project/:project_id/doc-deleted', HttpController.getAllDeletedDocs) app.get('/project/:project_id/doc', HttpController.getAllDocs) app.get('/project/:project_id/ranges', HttpController.getAllRanges) app.get('/project/:project_id/doc/:doc_id', HttpController.getDoc) diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index 2f19087388..4a97c3688a 100644 --- a/services/docstore/app/js/DocManager.js +++ b/services/docstore/app/js/DocManager.js @@ -150,6 +150,10 @@ module.exports = DocManager = { ) }, + getAllDeletedDocs(project_id, filter, callback) { + MongoManager.getProjectsDeletedDocs(project_id, filter, callback) + }, + getAllNonDeletedDocs(project_id, filter, callback) { if (callback == null) { callback = function (error, docs) {} diff --git a/services/docstore/app/js/HttpController.js b/services/docstore/app/js/HttpController.js index 59603ca06a..0d7fc62061 100644 --- a/services/docstore/app/js/HttpController.js +++ b/services/docstore/app/js/HttpController.js @@ -95,6 +95,24 @@ module.exports = HttpController = { ) }, + getAllDeletedDocs(req, res, next) { + const { project_id } = req.params + logger.log({ project_id }, 'getting all deleted docs') + DocManager.getAllDeletedDocs(project_id, { name: true }, function ( + error, + docs + ) { + if (error) { + return next(error) + } + res.json( + docs.map((doc) => { + return { _id: doc._id.toString(), name: doc.name } + }) + ) + }) + }, + getAllRanges(req, res, next) { if (next == null) { next = function (error) {} diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index ea8f7f710b..753abaf1ea 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/services/docstore/app/js/MongoManager.js @@ -14,6 +14,7 @@ let MongoManager const { db, ObjectId } = require('./mongodb') const logger = require('logger-sharelatex') const metrics = require('@overleaf/metrics') +const Settings = require('settings-sharelatex') const { promisify } = require('util') module.exports = MongoManager = { @@ -33,6 +34,24 @@ module.exports = MongoManager = { ) }, + getProjectsDeletedDocs(project_id, filter, callback) { + db.docs + .find( + { + project_id: ObjectId(project_id.toString()), + deleted: true, + // TODO(das7pad): remove name filter after back filling data + name: { $exists: true } + }, + { + projection: filter, + sort: { deletedAt: -1 }, + limit: Settings.max_deleted_docs + } + ) + .toArray(callback) + }, + getProjectsDocs(project_id, options, filter, callback) { const query = { project_id: ObjectId(project_id.toString()) } if (!options.include_deleted) { diff --git a/services/docstore/config/settings.defaults.js b/services/docstore/config/settings.defaults.js index 1ab77e784b..808470b950 100644 --- a/services/docstore/config/settings.defaults.js +++ b/services/docstore/config/settings.defaults.js @@ -38,6 +38,8 @@ const Settings = { } }, + max_deleted_docs: parseInt(process.env.MAX_DELETED_DOCS, 10) || 2000, + max_doc_length: parseInt(process.env.MAX_DOC_LENGTH) || 2 * 1024 * 1024 // 2mb } diff --git a/services/docstore/test/acceptance/js/DeletingDocsTests.js b/services/docstore/test/acceptance/js/DeletingDocsTests.js index 27d3c212c7..232931102e 100644 --- a/services/docstore/test/acceptance/js/DeletingDocsTests.js +++ b/services/docstore/test/acceptance/js/DeletingDocsTests.js @@ -207,6 +207,23 @@ function deleteTestSuite(deleteDoc) { describe('Delete via DELETE', function () { deleteTestSuite(DocstoreClient.deleteDocLegacy) + + describe('when the doc gets no name on delete', function () { + beforeEach(function (done) { + DocstoreClient.deleteDocLegacy(this.project_id, this.doc_id, done) + }) + + it('should not show the doc in the deleted docs response', function (done) { + DocstoreClient.getAllDeletedDocs( + this.project_id, + (error, deletedDocs) => { + if (error) return done(error) + expect(deletedDocs).to.deep.equal([]) + done() + } + ) + }) + }) }) describe('Delete via PATCH', function () { @@ -292,6 +309,121 @@ describe('Delete via PATCH', function () { expect(this.res.statusCode).to.equal(400) }) }) + + describe('before deleting anything', function () { + it('should show nothing in deleted docs response', function (done) { + DocstoreClient.getAllDeletedDocs( + this.project_id, + (error, deletedDocs) => { + if (error) return done(error) + expect(deletedDocs).to.deep.equal([]) + done() + } + ) + }) + }) + + describe('when the doc gets a name on delete', function () { + beforeEach(function (done) { + DocstoreClient.deleteDoc(this.project_id, this.doc_id, done) + }) + + it('should show the doc in deleted docs response', function (done) { + DocstoreClient.getAllDeletedDocs( + this.project_id, + (error, deletedDocs) => { + if (error) return done(error) + expect(deletedDocs).to.deep.equal([ + { _id: this.doc_id.toString(), name: 'main.tex' } + ]) + done() + } + ) + }) + + describe('after deleting multiple docs', function () { + beforeEach('create doc2', function (done) { + this.doc_id2 = ObjectId() + DocstoreClient.createDoc( + this.project_id, + this.doc_id2, + this.lines, + this.version, + this.ranges, + done + ) + }) + beforeEach('delete doc2', function (done) { + DocstoreClient.deleteDocWithName( + this.project_id, + this.doc_id2, + 'two.tex', + done + ) + }) + beforeEach('create doc3', function (done) { + this.doc_id3 = ObjectId() + DocstoreClient.createDoc( + this.project_id, + this.doc_id3, + this.lines, + this.version, + this.ranges, + done + ) + }) + beforeEach('delete doc3', function (done) { + DocstoreClient.deleteDocWithName( + this.project_id, + this.doc_id3, + 'three.tex', + done + ) + }) + it('should show all the docs as deleted', function (done) { + DocstoreClient.getAllDeletedDocs( + this.project_id, + (error, deletedDocs) => { + if (error) return done(error) + + expect(deletedDocs).to.deep.equal([ + { _id: this.doc_id3.toString(), name: 'three.tex' }, + { _id: this.doc_id2.toString(), name: 'two.tex' }, + { _id: this.doc_id.toString(), name: 'main.tex' } + ]) + done() + } + ) + }) + + describe('with one more than max_deleted_docs permits', function () { + let maxDeletedDocsBefore + beforeEach(function () { + maxDeletedDocsBefore = Settings.max_deleted_docs + Settings.max_deleted_docs = 2 + }) + afterEach(function () { + Settings.max_deleted_docs = maxDeletedDocsBefore + }) + + it('should omit the first deleted doc', function (done) { + DocstoreClient.getAllDeletedDocs( + this.project_id, + (error, deletedDocs) => { + if (error) return done(error) + + expect(deletedDocs).to.deep.equal([ + { _id: this.doc_id3.toString(), name: 'three.tex' }, + { _id: this.doc_id2.toString(), name: 'two.tex' } + // dropped main.tex + ]) + done() + } + ) + }) + }) + }) + }) }) describe("Destroying a project's documents", function () { diff --git a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js index 7248fff744..fda0a66e1c 100644 --- a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js +++ b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js @@ -85,6 +85,22 @@ module.exports = DocstoreClient = { ) }, + getAllDeletedDocs(project_id, callback) { + request.get( + { + url: `http://localhost:${settings.internal.docstore.port}/project/${project_id}/doc-deleted`, + json: true + }, + (error, res, body) => { + if (error) return callback(error) + if (res.statusCode !== 200) { + return callback(new Error('unexpected statusCode')) + } + callback(null, body) + } + ) + }, + getAllRanges(project_id, callback) { if (callback == null) { callback = function (error, res, body) {} diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index eca77a55d9..0b073937f0 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/services/docstore/test/unit/js/MongoManagerTests.js @@ -28,7 +28,8 @@ describe('MongoManager', function () { ObjectId }, '@overleaf/metrics': { timeAsyncMethod: sinon.stub() }, - 'logger-sharelatex': { log() {} } + 'logger-sharelatex': { log() {} }, + 'settings-sharelatex': { max_deleted_docs: 42 } }, globals: { console @@ -175,6 +176,50 @@ describe('MongoManager', function () { }) }) + describe('getProjectsDeletedDocs', function () { + beforeEach(function (done) { + this.filter = { name: true } + this.doc1 = { _id: '1', name: 'mock-doc1.tex' } + this.doc2 = { _id: '2', name: 'mock-doc2.tex' } + this.doc3 = { _id: '3', name: 'mock-doc3.tex' } + this.db.docs.find = sinon.stub().returns({ + toArray: sinon.stub().yields(null, [this.doc1, this.doc2, this.doc3]) + }) + this.callback.callsFake(done) + this.MongoManager.getProjectsDeletedDocs( + this.project_id, + this.filter, + this.callback + ) + }) + + it('should find the deleted docs via the project_id', function () { + this.db.docs.find + .calledWith({ + project_id: ObjectId(this.project_id), + deleted: true, + name: { $exists: true } + }) + .should.equal(true) + }) + + it('should filter, sort by deletedAt and limit', function () { + this.db.docs.find + .calledWith(sinon.match.any, { + projection: this.filter, + sort: { deletedAt: -1 }, + limit: 42 + }) + .should.equal(true) + }) + + it('should call the callback with the docs', function () { + this.callback + .calledWith(null, [this.doc1, this.doc2, this.doc3]) + .should.equal(true) + }) + }) + describe('upsertIntoDocCollection', function () { beforeEach(function () { this.db.docs.updateOne = sinon.stub().callsArgWith(3, this.stubbedErr)