diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index bbb4f7fecd..409668c313 100644 --- a/services/docstore/app/js/DocManager.js +++ b/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) }) } diff --git a/services/docstore/config/settings.defaults.js b/services/docstore/config/settings.defaults.js index 54dc112709..0bbf99424e 100644 --- a/services/docstore/config/settings.defaults.js +++ b/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 diff --git a/services/docstore/test/acceptance/js/DeletingDocsTests.js b/services/docstore/test/acceptance/js/DeletingDocsTests.js index 80a448cdcc..d32cd52b6b 100644 --- a/services/docstore/test/acceptance/js/DeletingDocsTests.js +++ b/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 () { diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index 55955f3704..597129b1bd 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/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 () {