Merge pull request #85 from overleaf/jpa-background-flush-on-delete

[DocManager] optionally flush docs out of mongo when deleting them

services/docstore/app/js/DocManager.js

@@ -20,6 +20,7 @@ const logger = require('logger-sharelatex')
 const _ = require('underscore')
 const DocArchive = require('./DocArchiveManager')
 const RangeManager = require('./RangeManager')
+const Settings = require('settings-sharelatex')
 
 module.exports = DocManager = {
   // TODO: For historical reasons, the doc version is currently stored in the docOps
@@ -304,6 +305,19 @@ module.exports = DocManager = {
           )
         )
       }
+
+      if (Settings.docstore.archiveOnSoftDelete) {
+        // The user will not read this doc anytime soon. Flush it out of mongo.
+        DocArchive.archiveDocById(project_id, doc_id, (err) => {
+          if (err) {
+            logger.warn(
+              { project_id, doc_id, err },
+              'archiving a single doc in the background failed'
+            )
+          }
+        })
+      }
+
       return MongoManager.markDocAsDeleted(project_id, doc_id, callback)
     })
   }

services/docstore/config/settings.defaults.js

@@ -22,6 +22,8 @@ const Settings = {
   },
 
   docstore: {
+    archiveOnSoftDelete: process.env.ARCHIVE_ON_SOFT_DELETE === 'true',
+
     backend: process.env.BACKEND || 's3',
     healthCheck: {
       project_id: process.env.HEALTH_CHECK_PROJECT_ID

services/docstore/test/acceptance/js/DeletingDocsTests.js

@@ -17,6 +17,7 @@ const { db, ObjectId } = require('../../../app/js/mongodb')
 const { expect } = chai
 const DocstoreApp = require('./helpers/DocstoreApp')
 const Errors = require('../../../app/js/Errors')
+const Settings = require('settings-sharelatex')
 
 const DocstoreClient = require('./helpers/DocstoreClient')
 
@@ -86,7 +87,7 @@ describe('Deleting a doc', function () {
       )
     })
 
-    return it('should insert a deleted doc into the docs collection', function (done) {
+    it('should insert a deleted doc into the docs collection', function (done) {
       return db.docs.find({ _id: this.doc_id }).toArray((error, docs) => {
         docs[0]._id.should.deep.equal(this.doc_id)
         docs[0].lines.should.deep.equal(this.lines)
@@ -94,6 +95,74 @@ describe('Deleting a doc', function () {
         return done()
       })
     })
+
+    it('should not export the doc to s3', function (done) {
+      setTimeout(() => {
+        DocstoreClient.getS3Doc(this.project_id, this.doc_id, (error) => {
+          expect(error).to.be.instanceOf(Errors.NotFoundError)
+          done()
+        })
+      }, 1000)
+    })
+  })
+
+  describe('when archiveOnSoftDelete is enabled', function () {
+    let archiveOnSoftDelete
+    beforeEach(function overwriteSetting() {
+      archiveOnSoftDelete = Settings.docstore.archiveOnSoftDelete
+      Settings.docstore.archiveOnSoftDelete = true
+    })
+    afterEach(function restoreSetting() {
+      Settings.docstore.archiveOnSoftDelete = archiveOnSoftDelete
+    })
+
+    beforeEach(function deleteDoc(done) {
+      DocstoreClient.deleteDoc(this.project_id, this.doc_id, (error, res) => {
+        this.res = res
+        done()
+      })
+    })
+
+    beforeEach(function waitForBackgroundFlush(done) {
+      setTimeout(done, 500)
+    })
+
+    afterEach(function cleanupDoc(done) {
+      db.docs.remove({ _id: this.doc_id }, done)
+    })
+
+    it('should set the deleted flag in the doc', function (done) {
+      db.docs.findOne({ _id: this.doc_id }, (error, doc) => {
+        if (error) {
+          return done(error)
+        }
+        expect(doc.deleted).to.equal(true)
+        done()
+      })
+    })
+
+    it('should set inS3 and unset lines and ranges in the doc', function (done) {
+      db.docs.findOne({ _id: this.doc_id }, (error, doc) => {
+        if (error) {
+          return done(error)
+        }
+        expect(doc.lines).to.not.exist
+        expect(doc.ranges).to.not.exist
+        expect(doc.inS3).to.equal(true)
+        done()
+      })
+    })
+
+    it('should set the doc in s3 correctly', function (done) {
+      DocstoreClient.getS3Doc(this.project_id, this.doc_id, (error, s3_doc) => {
+        if (error) {
+          return done(error)
+        }
+        expect(s3_doc.lines).to.deep.equal(this.lines)
+        expect(s3_doc.ranges).to.deep.equal(this.ranges)
+        done()
+      })
+    })
   })
 
   describe('when the doc exists in another project', function () {

services/docstore/test/unit/js/DocManagerTests.js

@@ -15,6 +15,7 @@
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 const chai = require('chai')
+chai.use(require('sinon-chai'))
 const { assert } = require('chai')
 chai.should()
 const { expect } = chai
@@ -34,6 +35,7 @@ describe('DocManager', function () {
           },
           shouldUpdateRanges: sinon.stub().returns(false)
         }),
+        'settings-sharelatex': (this.settings = { docstore: {} }),
         'logger-sharelatex': (this.logger = {
           log: sinon.stub(),
           warn() {},
@@ -423,13 +425,15 @@ describe('DocManager', function () {
 
   describe('deleteDoc', function () {
     describe('when the doc exists', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
+        this.callback = sinon.stub().callsFake(done)
         this.lines = ['mock', 'doc', 'lines']
         this.rev = 77
         this.DocManager.checkDocExists = sinon
           .stub()
           .callsArgWith(2, null, true)
-        this.MongoManager.markDocAsDeleted = sinon.stub().callsArg(2)
+        this.MongoManager.markDocAsDeleted = sinon.stub().yields(null)
+        this.DocArchiveManager.archiveDocById = sinon.stub().yields(null)
         return this.DocManager.deleteDoc(
           this.project_id,
           this.doc_id,
@@ -449,9 +453,71 @@ describe('DocManager', function () {
           .should.equal(true)
       })
 
-      return it('should return the callback', function () {
+      it('should return the callback', function () {
         return this.callback.called.should.equal(true)
       })
+
+      describe('background flush disabled', function () {
+        beforeEach(function () {
+          this.settings.docstore.archiveOnSoftDelete = false
+        })
+
+        it('should not flush the doc out of mongo', function () {
+          this.DocArchiveManager.archiveDocById.should.not.have.been.called
+        })
+      })
+
+      describe('background flush enabled', function () {
+        beforeEach(function (done) {
+          this.settings.docstore.archiveOnSoftDelete = true
+          this.callback = sinon.stub().callsFake(done)
+          this.DocManager.deleteDoc(this.project_id, this.doc_id, this.callback)
+        })
+
+        it('should not fail the delete process', function () {
+          this.callback.should.have.been.calledWith(null)
+        })
+
+        it('should flush the doc out of mongo', function () {
+          this.DocArchiveManager.archiveDocById.should.have.been.calledWith(
+            this.project_id,
+            this.doc_id
+          )
+        })
+
+        describe('when the background flush fails', function () {
+          beforeEach(function (done) {
+            this.err = new Error('foo')
+            this.DocManager.checkDocExists = sinon.stub().yields(null, true)
+            this.MongoManager.markDocAsDeleted = sinon.stub().yields(null)
+            this.DocArchiveManager.archiveDocById = sinon
+              .stub()
+              .yields(this.err)
+            this.logger.warn = sinon.stub()
+            this.callback = sinon.stub().callsFake(done)
+            this.DocManager.deleteDoc(
+              this.project_id,
+              this.doc_id,
+              this.callback
+            )
+          })
+
+          it('should log a warning', function () {
+            this.logger.warn.should.have.been.calledWith(
+              sinon.match({
+                project_id: this.project_id,
+                doc_id: this.doc_id,
+                err: this.err
+              }),
+              'archiving a single doc in the background failed'
+            )
+          })
+
+          it('should not fail the delete process', function () {
+            this.callback.should.have.been.calledWith(null)
+          })
+        })
+      })
     })
 
     return describe('when the doc does not exist', function () {