Merge pull request #95 from overleaf/jpa-add-endpoint-get-deleted-docs-take-2

[misc] add a new endpoint for getting deleted docs

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)

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) {}

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) {}

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) {

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
 }
 

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 () {

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) {}

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)