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

This commit is contained in:
Jakob Ackermann 2021-01-04 13:36:19 +00:00
parent 915fa4ca67
commit 708bdfd197
4 changed files with 155 additions and 4 deletions

View file

@ -20,6 +20,7 @@ const logger = require('logger-sharelatex')
const _ = require('underscore') const _ = require('underscore')
const DocArchive = require('./DocArchiveManager') const DocArchive = require('./DocArchiveManager')
const RangeManager = require('./RangeManager') const RangeManager = require('./RangeManager')
const Settings = require('settings-sharelatex')
module.exports = DocManager = { module.exports = DocManager = {
// TODO: For historical reasons, the doc version is currently stored in the docOps // 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) return MongoManager.markDocAsDeleted(project_id, doc_id, callback)
}) })
} }

View file

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

View file

@ -17,6 +17,7 @@ const { db, ObjectId } = require('../../../app/js/mongodb')
const { expect } = chai const { expect } = chai
const DocstoreApp = require('./helpers/DocstoreApp') const DocstoreApp = require('./helpers/DocstoreApp')
const Errors = require('../../../app/js/Errors') const Errors = require('../../../app/js/Errors')
const Settings = require('settings-sharelatex')
const DocstoreClient = require('./helpers/DocstoreClient') 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) => { return db.docs.find({ _id: this.doc_id }).toArray((error, docs) => {
docs[0]._id.should.deep.equal(this.doc_id) docs[0]._id.should.deep.equal(this.doc_id)
docs[0].lines.should.deep.equal(this.lines) docs[0].lines.should.deep.equal(this.lines)
@ -94,6 +95,74 @@ describe('Deleting a doc', function () {
return done() 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 () { describe('when the doc exists in another project', function () {

View file

@ -15,6 +15,7 @@
const SandboxedModule = require('sandboxed-module') const SandboxedModule = require('sandboxed-module')
const sinon = require('sinon') const sinon = require('sinon')
const chai = require('chai') const chai = require('chai')
chai.use(require('sinon-chai'))
const { assert } = require('chai') const { assert } = require('chai')
chai.should() chai.should()
const { expect } = chai const { expect } = chai
@ -34,6 +35,7 @@ describe('DocManager', function () {
}, },
shouldUpdateRanges: sinon.stub().returns(false) shouldUpdateRanges: sinon.stub().returns(false)
}), }),
'settings-sharelatex': (this.settings = { docstore: {} }),
'logger-sharelatex': (this.logger = { 'logger-sharelatex': (this.logger = {
log: sinon.stub(), log: sinon.stub(),
warn() {}, warn() {},
@ -423,13 +425,15 @@ describe('DocManager', function () {
describe('deleteDoc', function () { describe('deleteDoc', function () {
describe('when the doc exists', function () { describe('when the doc exists', function () {
beforeEach(function () { beforeEach(function (done) {
this.callback = sinon.stub().callsFake(done)
this.lines = ['mock', 'doc', 'lines'] this.lines = ['mock', 'doc', 'lines']
this.rev = 77 this.rev = 77
this.DocManager.checkDocExists = sinon this.DocManager.checkDocExists = sinon
.stub() .stub()
.callsArgWith(2, null, true) .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( return this.DocManager.deleteDoc(
this.project_id, this.project_id,
this.doc_id, this.doc_id,
@ -449,9 +453,71 @@ describe('DocManager', function () {
.should.equal(true) .should.equal(true)
}) })
return it('should return the callback', function () { it('should return the callback', function () {
return this.callback.called.should.equal(true) 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 () { return describe('when the doc does not exist', function () {